--- pyanaconda/image.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/pyanaconda/image.py b/pyanaconda/image.py index 5fe3833..a7422ec 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -193,6 +193,10 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): # created from ISO images. for dev in set(devicetree.getDevicesByType("cdrom") + \ [d for d in devicetree.devices if d.format.type == "iso9660"]): + if not dev.sysfsPath: + # No udev information + continue + devicetree.updateDeviceFormat(dev) if not hasattr(dev.format, "mount"): # no mountable media
On Mon, 2013-09-23 at 17:31 -0400, David Shea wrote:
pyanaconda/image.py | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/pyanaconda/image.py b/pyanaconda/image.py index 5fe3833..a7422ec 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -193,6 +193,10 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): # created from ISO images. for dev in set(devicetree.getDevicesByType("cdrom") + \ [d for d in devicetree.devices if d.format.type == "iso9660"]):
if not dev.sysfsPath:
# No udev information
continue
devicetree.updateDeviceFormat(dev) if not hasattr(dev.format, "mount"): # no mountable media
I think this is papering over possibly several other problems. I'd consider two other changes instead of this one:
1. continue as above, but do so 'if not device.controllable' instead of 'if not device.sysfsPath'. That'll filter out several devices we should be ignoring. 2. add a check to DeviceTree.updateDeviceFormat so it logs an error and returns early if 'not info' just before calling self.handleUdevDeviceFormat.
David
--- blivet/devicetree.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 39e756c..0772c89 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -1701,9 +1701,11 @@ class DeviceTree(object):
udev_settle() info = udev_get_device(device.sysfsPath) - self.handleUdevDeviceFormat(info, device) - if device.format.type: - log.info("got format: %s" % device.format) + + if info: + self.handleUdevDeviceFormat(info, device) + if device.format.type: + log.info("got format: %s" % device.format)
def _handleInconsistencies(self): for vg in [d for d in self.devices if d.type == "lvmvg"]:
--- pyanaconda/image.py | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/pyanaconda/image.py b/pyanaconda/image.py index 5fe3833..9049be9 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -193,6 +193,9 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): # created from ISO images. for dev in set(devicetree.getDevicesByType("cdrom") + \ [d for d in devicetree.devices if d.format.type == "iso9660"]): + if not dev.controllable: + continue + devicetree.updateDeviceFormat(dev) if not hasattr(dev.format, "mount"): # no mountable media
None of the callers of opticalInstallMedia use the mountpoint parameter, so removed it. Changed to using a temporary directory instead of /run/install/source to mount and verify potential optical media, since /run/install/source may already be in use by media specified in inst.repo. --- pyanaconda/image.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/pyanaconda/image.py b/pyanaconda/image.py index 9049be9..82f3e4f 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -18,8 +18,8 @@ #
import isys -import os, os.path, stat -from constants import INSTALL_TREE, ISO_DIR +import os, os.path, stat, tempfile +from constants import ISO_DIR
from errors import errorHandler, ERROR_RAISE, InvalidImageSizeError, MediaMountError, MediaUnmountError, MissingImageError
@@ -185,7 +185,7 @@ def mountImage(isodir, tree):
# Return the first Device instance containing valid optical install media # for this product. -def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): +def opticalInstallMedia(devicetree): retval = None
# Search for devices identified as cdrom along with any other @@ -201,16 +201,21 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): # no mountable media continue
+ mountpoint = tempfile.mkdtemp() try: - dev.format.mount(mountpoint=mountpoint) - except FSError: - continue + try: + dev.format.mount(mountpoint=mountpoint) + except FSError: + continue + + if not verifyMedia(mountpoint): + dev.format.unmount() + continue
- if not verifyMedia(mountpoint): dev.format.unmount() - continue + finally: + os.rmdir(mountpoint)
- dev.format.unmount() retval = dev break
On Tue, 2013-09-24 at 09:37 -0400, David Shea wrote:
None of the callers of opticalInstallMedia use the mountpoint parameter, so removed it. Changed to using a temporary directory instead of /run/install/source to mount and verify potential optical media, since /run/install/source may already be in use by media specified in inst.repo.
pyanaconda/image.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/pyanaconda/image.py b/pyanaconda/image.py index 9049be9..82f3e4f 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -18,8 +18,8 @@ #
import isys -import os, os.path, stat -from constants import INSTALL_TREE, ISO_DIR +import os, os.path, stat, tempfile +from constants import ISO_DIR
from errors import errorHandler, ERROR_RAISE, InvalidImageSizeError, MediaMountError, MediaUnmountError, MissingImageError
@@ -185,7 +185,7 @@ def mountImage(isodir, tree):
# Return the first Device instance containing valid optical install media # for this product. -def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): +def opticalInstallMedia(devicetree): retval = None
# Search for devices identified as cdrom along with any other
@@ -201,16 +201,21 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): # no mountable media continue
mountpoint = tempfile.mkdtemp() try:
dev.format.mount(mountpoint=mountpoint)
except FSError:
continue
try:
dev.format.mount(mountpoint=mountpoint)
except FSError:
continue
if not verifyMedia(mountpoint):
dev.format.unmount()
continue
if not verifyMedia(mountpoint): dev.format.unmount()
continue
finally:
os.rmdir(mountpoint)
dev.format.unmount()
If verifyMedia returns True, the format won't get umounted and the finally block will try to remove its contents. Or am I missing something?
On 09/24/2013 09:48 AM, Vratislav Podzimek wrote:
On Tue, 2013-09-24 at 09:37 -0400, David Shea wrote:
None of the callers of opticalInstallMedia use the mountpoint parameter, so removed it. Changed to using a temporary directory instead of /run/install/source to mount and verify potential optical media, since /run/install/source may already be in use by media specified in inst.repo.
pyanaconda/image.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/pyanaconda/image.py b/pyanaconda/image.py index 9049be9..82f3e4f 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -18,8 +18,8 @@ #
import isys -import os, os.path, stat -from constants import INSTALL_TREE, ISO_DIR +import os, os.path, stat, tempfile +from constants import ISO_DIR
from errors import errorHandler, ERROR_RAISE, InvalidImageSizeError, MediaMountError, MediaUnmountError, MissingImageError
@@ -185,7 +185,7 @@ def mountImage(isodir, tree):
# Return the first Device instance containing valid optical install media # for this product. -def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): +def opticalInstallMedia(devicetree): retval = None
# Search for devices identified as cdrom along with any other
@@ -201,16 +201,21 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): # no mountable media continue
mountpoint = tempfile.mkdtemp() try:
dev.format.mount(mountpoint=mountpoint)
except FSError:
continue
try:
dev.format.mount(mountpoint=mountpoint)
except FSError:
continue
if not verifyMedia(mountpoint):
dev.format.unmount()
continue
if not verifyMedia(mountpoint): dev.format.unmount()
continue
finally:
os.rmdir(mountpoint)
dev.format.unmount()
If verifyMedia returns True, the format won't get umounted and the finally block will try to remove its contents. Or am I missing something?
It looks funny because I indented a block and one of the original unmounts ended up in the context of the diff. I just removed the parameter from the function definition, added the mountpoint = tempfile.mkdtemp(), and wrapped everything in this block:
try: dev.format.mount(mountpoint=mountpoint) except FSError: continue
if not verifyMedia(mountpoint): dev.format.unmount() continue
dev.format.unmount()
In a try-finally. There's still two unmount() lines.
On Tue, 2013-09-24 at 09:55 -0400, David Shea wrote:
On 09/24/2013 09:48 AM, Vratislav Podzimek wrote:
On Tue, 2013-09-24 at 09:37 -0400, David Shea wrote:
None of the callers of opticalInstallMedia use the mountpoint parameter, so removed it. Changed to using a temporary directory instead of /run/install/source to mount and verify potential optical media, since /run/install/source may already be in use by media specified in inst.repo.
pyanaconda/image.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/pyanaconda/image.py b/pyanaconda/image.py index 9049be9..82f3e4f 100644 --- a/pyanaconda/image.py +++ b/pyanaconda/image.py @@ -18,8 +18,8 @@ #
import isys -import os, os.path, stat -from constants import INSTALL_TREE, ISO_DIR +import os, os.path, stat, tempfile +from constants import ISO_DIR
from errors import errorHandler, ERROR_RAISE, InvalidImageSizeError, MediaMountError, MediaUnmountError, MissingImageError
@@ -185,7 +185,7 @@ def mountImage(isodir, tree):
# Return the first Device instance containing valid optical install media # for this product. -def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): +def opticalInstallMedia(devicetree): retval = None
# Search for devices identified as cdrom along with any other
@@ -201,16 +201,21 @@ def opticalInstallMedia(devicetree, mountpoint=INSTALL_TREE): # no mountable media continue
mountpoint = tempfile.mkdtemp() try:
dev.format.mount(mountpoint=mountpoint)
except FSError:
continue
try:
dev.format.mount(mountpoint=mountpoint)
except FSError:
continue
if not verifyMedia(mountpoint):
dev.format.unmount()
continue
if not verifyMedia(mountpoint): dev.format.unmount()
continue
finally:
os.rmdir(mountpoint)
dev.format.unmount()
If verifyMedia returns True, the format won't get umounted and the finally block will try to remove its contents. Or am I missing something?
It looks funny because I indented a block and one of the original unmounts ended up in the context of the diff. I just removed the parameter from the function definition, added the mountpoint = tempfile.mkdtemp(), and wrapped everything in this block:
try: dev.format.mount(mountpoint=mountpoint) except FSError: continue if not verifyMedia(mountpoint): dev.format.unmount() continue dev.format.unmount()
In a try-finally. There's still two unmount() lines.
Oh, I've overlooked the one context line between two removed lines. These both look good to me, then.
anaconda-patches@lists.fedorahosted.org