Hi,
On 12/09/2009 04:33 PM, David Lehman wrote:
<snip>
@@ -1250,7 +1250,7 @@ class DeviceTree(object):
# The first step is to either look up or create the device
#
if udev_device_is_multipath_member(info):
- device = StorageDevice(name,
+ device = DiskDevice(name,
major=udev_device_get_major(info),
minor=udev_device_get_minor(info),
sysfsPath=sysfs_path, exists=True,
@@ -1282,14 +1282,9 @@ class DeviceTree(object):
if device is None:
device = self.addUdevOpticalDevice(info)
elif udev_device_is_biosraid(info) and udev_device_is_disk(info):
- # This is special handling to avoid the "unrecognized disklabel"
- # code since biosraid member disks won't have a disklabel. We
- # use a StorageDevice because DiskDevices need disklabels.
- # Quite lame, but it doesn't matter much since we won't use
- # the StorageDevice instances for anything.
log.debug("%s is part of a biosraid" % name)
if device is None:
- device = StorageDevice(name,
+ device = DiskDevice(name,
major=udev_device_get_major(info),
minor=udev_device_get_minor(info),
sysfsPath=sysfs_path, exists=True)
This is wrong, now handleUdevDiskLabelFormat() will get called on them
which in case of a multipath or a RAID mirror will succeed (and in
case of a RAID stripe might succeed too), resulting in the raw disk
becoming part of storage.partitioned. Maybe partionable should
become a parameter of the DiskDevice constructor and get set to
False here ?
Instantiating DiskDevice for a disk is most certainly not wrong.
Sorry I did not mean to imply the principle was wrong, merely that as
implemented it will result in a wrong outcome in some cases.
I did
forget about the disklabel showing up in some cases, though -- I may
expand the partitionable attribute into a property for this.
Ack.
>>
Even with this fixed, the raw disks now will still become part of
storage.disks, and I'm not sure we even want that.
I'm not sure why we wouldn't now that the (disk ==> disklabel) rule is
gone. However, we could certainly use those formats' new "hidden"
attribute for this if we do want to filter them out any place they are
found to be problematic.
Ah, I forgot about the hidden attribute, and I agree that in general
making these disks DiskDevices is the right thing to do. All I was
trying to say is that I'm worried this might cause issues.
(Likewise for the new status behavior of DMRaidArrayDevice / MultiPath...)
Patch 4 looks good
Patch 5, do we want to hide devices with a disklabel on them (iow normal disks)
from the main partition UI ?
Yes, because partitioned devices are handled differently than other
formatting. The "hidden" attribute only prevents the device from being
displayed in the main partitioning screen as a leaf node in the tree.
Partitioned devices, like volume groups, do not appear as leaf nodes in
the tree -- the partitions are the leaf nodes (like the logical
volumes).
Ok.
Regards,
Hans
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
