Re: [PATCH] Ignore disk devices with missing media (#488800).

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Chris Lumens wrote:

testing gave me 2 tracebacks, see below
There's a class of device that shows up as a disk drive, but is removable
and doesn't necessarily have media present all the time.  This class
contains things like USB CF readers, those 5-in-1 media readers on the
fronts of some computers, and so forth.

To fix this, I've added a mediaPresent method to DiskDevice that checks if
self.partedDevice was created or not.  We then need to check this throughout
DiskDevice methods - basically, do nothing if we couldn't probe the device
to begin with, but still recognize its existence.

In order to keep the rest of the code clean, we need to weed out all devices
without media present in Storage.disks.
---
 storage/__init__.py |    6 ++--
 storage/devices.py  |   63 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 50 insertions(+), 19 deletions(-)

diff --git a/storage/__init__.py b/storage/__init__.py
index 8207156..30c7824 100644
--- a/storage/__init__.py
+++ b/storage/__init__.py
@@ -239,7 +239,7 @@ class Storage(object):
     def disks(self):
         """ A list of the disks in the device tree.
- Ignored disks are not included.
+            Ignored disks are not included, as are disks with no media present.
This is based on the current state of the device tree and
             does not necessarily reflect the actual on-disk state of the
@@ -248,7 +248,7 @@ class Storage(object):
         disks = []
         devices = self.devicetree.devices
         for d in devices:
-            if isinstance(devices[d], DiskDevice):
+            if isinstance(devices[d], DiskDevice) and d.mediaPresent:
we should test devices[d].mediaPresent here
                 disks.append(devices[d])
         disks.sort(key=lambda d: d.name)
         return disks
@@ -613,7 +613,7 @@ class Storage(object):
def extendedPartitionsSupported(self):
         """ Return whether any disks support extended partitions."""
-        for disk in self.disks:
+        for disk in filter(lambda disk: disk.mediaPresent, self.disks):
             if disk.partedDisk.supportsFeature(parted.DISK_TYPE_EXTENDED):
                 return True
         return False
diff --git a/storage/devices.py b/storage/devices.py
index 8b6f7ef..c9b0ed0 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -738,28 +738,38 @@ class DiskDevice(StorageDevice):
         self.partedDisk = None
log.debug("looking up parted Device: %s" % self.path)
-        self.partedDevice = parted.Device(path=self.path)
-        if not self.partedDevice:
-            raise DeviceError("cannot find parted device instance")
+
+        # We aren't guaranteed to be able to get a device.  In particular,
+        # built-in USB flash readers show up as devices but do not always
+        # have any media present, so parted won't be able to find a device.
+        try:
+            self.partedDevice = parted.Device(path=self.path)
+        except DeviceException:
I think we need _ped.DeviceException here
+            pass
log.debug("creating parted Disk: %s" % self.path)
-        if initlabel:
-            self.partedDisk = self.freshPartedDisk()
-        else:
-            try:
-                self.partedDisk = parted.Disk(device=self.partedDevice)
-            except _ped.DiskLabelException:
-                # if we have a cb function use it. else an error.
-                if initcb is not None and initcb():
-                    self.partedDisk = self.freshPartedDisk()
-                else:
-                    raise DeviceUserDeniedFormatError("User prefered to not format.")
+        if self.partedDevice:
+            if initlabel:
+                self.partedDisk = self.fresPartedDisk()
+            else:
+                try:
+                    self.partedDisk = parted.Disk(device=self.partedDevice)
+                except _ped.DiskLabelException:
+                    # if we have a cb function use it. else an error.
+                    if initcb is not None and initcb():
+                        self.partedDisk = parted.freshDisk(device=self.partedDevice, \
+                                ty = platform.getPlatform(None).diskType)
+                    else:
+                        raise DeviceUserDeniedFormatError("User prefered to not format.")
# We save the actual state of the disk here. Before the first
         # modification (addPartition or removePartition) to the partition
         # table we reset self.partedPartition to this state so we can
         # perform the modifications one at a time.
-        self._origPartedDisk = self.partedDisk.duplicate()
+        if self.partedDisk:
+            self._origPartedDisk = self.partedDisk.duplicate()
+        else:
+            self._origPartedDisk = None
self.probe() @@ -777,8 +787,15 @@ class DiskDevice(StorageDevice):
         return parted.freshDisk(device=self.partedDevice, ty=labelType)
@property
+    def mediaPresent(self):
+        return self.partedDevice is not None
+
+    @property
     def size(self):
         """ The disk's size in MB """
+        if not self.mediaPresent:
+            return 0
+
         if not self._size:
             self._size = self.partedDisk.device.getSize()
         return self._size
@@ -790,12 +807,20 @@ class DiskDevice(StorageDevice):
def removePartition(self, device):
         log_method_call(self, self.name, part=device.name)
+        if not self.mediaPresent:
+            log.warning("disk %s has no media, can't remove partition" % self.name)
+            return
+
         partition = self.partedDisk.getPartitionByPath(device.path)
         if partition:
             self.partedDisk.removePartition(partition)
def addPartition(self, device):
         log_method_call(self, self.name, part=device.name)
+        if not self.mediaPresent:
+            log.warning("disk %s has no media, can't add partition" % self.name)
+            return
+
         for part in self.partedDisk.partitions:
             log.debug("disk %s: partition %s has geom %s" % (self.name,
                                                              part.getDeviceNodeName(),
@@ -815,7 +840,7 @@ class DiskDevice(StorageDevice):
             pyparted should be able to tell us anything we want to know.
             size, disklabel type, maybe even partition layout
         """
-        if  not 'parted' in globals().keys():
+        if not self.mediaPresent or not 'parted' in globals().keys():
             return
log_method_call(self, self.name, size=self.size, partedDevice=self.partedDevice)
@@ -829,6 +854,9 @@ class DiskDevice(StorageDevice):
     def commit(self, intf=None):
         """ Commit changes to the device. """
         log_method_call(self, self.name, status=self.status)
+        if not self.mediaPresent:
+            return
+
         self.setupParents()
         self.setup()
         self.partedDisk.commit()
@@ -836,6 +864,9 @@ class DiskDevice(StorageDevice):
     def destroy(self):
         """ Destroy the device. """
         log_method_call(self, self.name, status=self.status)
+        if not self.mediaPresent:
+            return
+
         if self.status:
             self.format.destroy()

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux