The patch set looks ok, except for my comment about possibly leaking a
file descriptor. With that said, I would like a third person to look at
the first patch [1/6]. Its been a long time since I have visited this
part of the code and what is being done does not seem trivial.
Regards.
On Tue, Sep 15, 2009 at 01:30:10PM +0200, Hans de Goede wrote:
> Ok, so the purpose of this patch is too write mdadm.conf lines for
> mdraid container formats (imsm), but what it does is it cleans up
> handling of mdraid container formats in general, with the
> writing of mdadm.conf writing as a bonus effect really.
>
> We were adding mdraid container members (raidsets inside the container)
> to the device tree as DiskDevices. Which works, but is not completely
> correct. This patch introduce a PartitionableMDRaidArrayDevice and
> uses that for mdraid container members instead. This means we now also
> correctly tear them down / set them up when asked. In the future (mostly
> needs UI work to export the functionality) PartitionableMDRaidArrayDevice
> can be used to create partitionable normal (native metadata) mdraid sets too.
> ---
> storage/__init__.py | 2 ++
> storage/devices.py | 41 ++++++++++++++++++++++++++++++++++++-----
> storage/devicetree.py | 15 +++++++++++----
> storage/udev.py | 3 +++
> 4 files changed, 52 insertions(+), 9 deletions(-)
>
> diff --git a/storage/__init__.py b/storage/__init__.py
> index 200fd66..e8b7b52 100644
> --- a/storage/__init__.py
> +++ b/storage/__init__.py
> @@ -1918,6 +1918,8 @@ class FSSet(object):
> def mdadmConf(self):
> """ Return the contents of mdadm.conf. """
> arrays = self.devicetree.getDevicesByType("mdarray")
> + arrays.extend(self.devicetree.getDevicesByType("partitionable mdarray"))
> + arrays.extend(self.devicetree.getDevicesByType("mdcontainer"))
> conf = "# mdadm.conf written out by anaconda\n"
> conf += "MAILADDR root\n"
> devices = self.mountpoints.values() + self.swapDevices
> diff --git a/storage/devices.py b/storage/devices.py
> index fa34d91..756b9b2 100644
> --- a/storage/devices.py
> +++ b/storage/devices.py
> @@ -2112,12 +2112,12 @@ class LVMLogicalVolumeDevice(DMDevice):
> class MDRaidArrayDevice(StorageDevice):
> """ An mdraid (Linux RAID) device.
>
> - Since this is derived from StorageDevice, not PartitionDevice, it
> - can be used to represent a partitionable device.
> + Since this is derived from StorageDevice, not DiskDevice, it
> + can NOT be used to represent a partitionable device.
> """
> _type = "mdarray"
>
> - def __init__(self, name, level=None, minor=None, size=None,
> + def __init__(self, name, level=None, major=None, minor=None, size=None,
> memberDevices=None, totalDevices=None, bitmap=False,
> uuid=None, format=None, exists=None,
> parents=None, sysfsPath=''):
> @@ -2140,7 +2140,7 @@ class MDRaidArrayDevice(StorageDevice):
> exists -- indicates whether this is an existing device
> """
> StorageDevice.__init__(self, name, format=format, exists=exists,
> - minor=minor, size=size,
> + major=major, minor=minor, size=size,
> parents=parents, sysfsPath=sysfsPath)
>
> self.level = level
> @@ -2156,6 +2156,10 @@ class MDRaidArrayDevice(StorageDevice):
> self.chunkSize = 64.0 / 1024.0 # chunk size in MB
> self.superBlockSize = 128.0 / 1024.0 # superblock size in MB
>
> + # For container members probe size now, as we cannot determine it
> + # when teared down.
> + if self.parents and self.parents[0].type == "mdcontainer":
> + self._size = self.currentSize
>
> # FIXME: Bitmap is more complicated than this.
> # It can be internal or external. External requires a filename.
> @@ -2184,6 +2188,11 @@ class MDRaidArrayDevice(StorageDevice):
> if not self.devices:
> return 0
>
> + # For container members return probed size, as we cannot determine it
> + # when teared down.
> + if self.devices[0].type == "mdcontainer":
> + return self._size
> +
> size = 0
> smallestMemberSize = self.smallestMember.size - self.superBlockSize
> if not self.exists or not self.partedDevice:
> @@ -2345,6 +2354,8 @@ class MDRaidArrayDevice(StorageDevice):
> udev_settle(timeout=10)
> try:
> mdraid.mdadd(device.path, len(self.devices) < self.memberDevices)
> + # mdadd causes udev events
> + udev_settle(timeout=10)
> except MDRaidError as e:
> log.warning("failed to add member %s to md array %s: %s"
> % (device.path, self.path, e))
> @@ -2387,7 +2398,10 @@ class MDRaidArrayDevice(StorageDevice):
> if os.access(state_file, os.R_OK):
> state = open(state_file).read().strip()
> log.debug("%s state is %s" % (self.name, state))
> - if state in ("clean", "active", "active-idle"):
> + if state in ("clean", "active", "active-idle", "readonly", "read-auto"):
> + status = True
> + # mdcontainers have state inactive when started (clear if stopped)
> + if self._type == "mdcontainer" and state == "inactive":
> status = True
>
> return status
> @@ -2502,6 +2516,23 @@ class MDRaidArrayDevice(StorageDevice):
> self.exists = False
>
>
> +class PartitionableMDRaidArrayDevice(MDRaidArrayDevice, DiskDevice):
> + """ A partitionable mdraid (Linux RAID) device.
> +
> + Since this is derived from DiskDevice, not StorageDevice, it
> + can be used to represent a partitionable device.
> + """
> + _type = "partitionable mdarray"
> +
> + @property
> + def mediaPresent(self):
> + # Even if stopped/deactivated we still want to show up in storage.disks
> + return True
> +
> + @property
> + def model(self):
> + return "RAID%d Array" % self.level
> +
> class DMRaidArrayDevice(DiskDevice):
> """ A dmraid (device-mapper RAID) device """
> _type = "dm-raid array"
> diff --git a/storage/devicetree.py b/storage/devicetree.py
> index 3a832f4..afbe475 100644
> --- a/storage/devicetree.py
> +++ b/storage/devicetree.py
> @@ -1154,6 +1154,15 @@ class DeviceTree(object):
> kwargs["nic"] = udev_device_get_fcoe_nic(info)
> kwargs["identifier"] = udev_device_get_fcoe_identifier(info)
> log.debug("%s is an fcoe disk" % name)
> + elif udev_device_get_md_container(info):
> + diskType = PartitionableMDRaidArrayDevice
> + parentName = devicePathToName(udev_device_get_md_container(info))
> + kwargs["parents"] = [ self.getDeviceByName(parentName) ]
> + kwargs["level"] = udev_device_get_md_level(info)
> + kwargs["memberDevices"] = int(udev_device_get_md_devices(info))
> + # This needs bug #523314 fixed
> + kwargs["uuid"] = udev_device_get_md_uuid(info)
> + kwargs["exists"] = True
> else:
> diskType = DiskDevice
> log.debug("%s is a disk" % name)
> @@ -1259,9 +1268,7 @@ class DeviceTree(object):
> # Now, if the device is a disk, see if there is a usable disklabel.
> # If not, see if the user would like to create one.
> # XXX this is the bit that forces disklabels on disks. Lame.
> - if isinstance(device, DiskDevice) or \
> - isinstance(device, DMRaidArrayDevice) or \
> - isinstance(device, MultipathDevice):
> + if isinstance(device, DiskDevice):
> self.handleUdevDiskLabelFormat(info, device)
> return
>
> @@ -1918,7 +1925,7 @@ class DeviceTree(object):
> for device in self.leaves:
> try:
> device.teardown(recursive=True)
> - except (DeviceError, DeviceFormatError, LVMError) as e:
> + except (DeviceError, DeviceFormatError, LVMError, MDRaidError) as e:
> log.info("teardown of %s failed: %s" % (device.name, e))
>
> def setupAll(self):
> diff --git a/storage/udev.py b/storage/udev.py
> index bb6310b..6dba824 100644
> --- a/storage/udev.py
> +++ b/storage/udev.py
> @@ -186,6 +186,9 @@ def udev_device_get_md_devices(info):
> def udev_device_get_md_uuid(info):
> return info["MD_UUID"]
>
> +def udev_device_get_md_container(info):
> + return info.get("MD_CONTAINER")
> +
> def udev_device_get_vg_name(info):
> return info['LVM2_VG_NAME']
>
> --
> 1.6.4.2
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
--
Joel Andres Granados
Brno, Czech Republic, Red Hat.
Attachment:
pgpx6tdHcMe0I.pgp
Description: PGP signature
_______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
