From 9eda62b42c08076741a3591077f093682322ef29 Mon Sep 17 00:00:00 2001
From: David Lehman <dlehman@redhat.com>
Date: Fri, 7 Mar 2014 16:44:16 -0600
Subject: [PATCH 2/6] Add a ContainerDevice class to consolidate member
 management code.

---
 blivet/devices.py | 410 ++++++++++++++++++------------------------------------
 1 file changed, 136 insertions(+), 274 deletions(-)

diff --git a/blivet/devices.py b/blivet/devices.py
index 8ff82ee..d3263a4 100644
--- a/blivet/devices.py
+++ b/blivet/devices.py
@@ -2030,7 +2030,92 @@ class LUKSDevice(DMCryptDevice):
         data.encrypted = True
         super(LUKSDevice, self).populateKSData(data)
 
-class LVMVolumeGroupDevice(DMDevice):
+class ContainerDevice(StorageDevice):
+    _formatClassName = None
+    _formatUUIDAttr = None
+
+    def __init__(self, *args, **kwargs):
+        self.formatClass = get_device_format_class(self._formatClassName)
+        if not self.formatClass:
+            raise StorageError("cannot find '%s' class" % self._formatClassName)
+
+        parents = kwargs.pop("parents", [])
+        super(ContainerDevice, self).__init__(*args, **kwargs)
+
+        # could be None
+        for parent in parents or []:
+            self._addMember(parent)
+
+        # restore kwargs in case another class wants to use them further
+        kwargs["parents"] = parents
+
+    def _addMember(self, member):
+        log_method_call(self,
+                        self.name,
+                        member=member.name,
+                        status=self.status)
+        if not isinstance(member.format, self.formatClass):
+            raise ValueError("cannot add member with wrong format")
+
+        if member in self.parents:
+            raise ValueError("member is already part of this container")
+
+        if member.format.exists and self.uuid and self._formatUUIDAttr and \
+           getattr(member.format, self._formatUUIDAttr) != self.uuid:
+            raise DeviceError("cannot add member with mismatched UUID")
+
+        self.parents.append(member)
+        member.addChild()
+
+    def _removeMember(self, member):
+        log_method_call(self,
+                        self.name,
+                        member=member.name,
+                        status=self.status)
+        if member not in self.parents:
+            raise ValueError("specified pv is not part of this vg")
+
+        self.parents.remove(member)
+        member.removeChild()
+
+    def _add(self, member):
+        pass
+
+    def add(self, member):
+        """ Add the specified member to this container. """
+        if not self.exists:
+            raise DeviceError("device has not been created", self.name)
+
+        if member.format.exists and self.uuid and self._formatUUIDAttr and \
+           getattr(member.format, self._formatUUIDAttr) == self.uuid:
+            log.error("cannot re-add member: %s" % member)
+            raise DeviceError("cannot add members that are already part of the container")
+
+        self._add(self.name, member.path)
+
+        if member not in self.parents:
+            self._addMember(member)
+
+    def _remove(self, member):
+        pass
+
+    def remove(self, member):
+        """ Remove the specified member from this container. """
+        log_method_call(self, self.name, status=self.status)
+        if not self.exists:
+            raise DeviceError("device has not been created", self.name)
+
+        if self._formatUUIDAttr and self.uuid and \
+           getattr(member.originalFormat, self._formatUUIDAttr) != self.uuid:
+            log.error("cannot remove non-member: %s" % member)
+            raise DeviceError("cannot remove members that are not part of the container")
+
+        self._remove(self.name, member.path)
+
+        if member in self.parents:
+            self._removeMember(member)
+
+class LVMVolumeGroupDevice(ContainerDevice):
     """ An LVM Volume Group
 
         XXX Maybe this should inherit from StorageDevice instead of
@@ -2038,6 +2123,8 @@ class LVMVolumeGroupDevice(DMDevice):
     """
     _type = "lvmvg"
     _packages = ["lvm2"]
+    _formatClassName = "lvmpv"
+    _formatUUIDAttr = "vgUuid"
 
     def __init__(self, name, parents=None, size=None, free=None,
                  peSize=None, peCount=None, peFree=None, pvCount=None,
@@ -2069,19 +2156,8 @@ class LVMVolumeGroupDevice(DMDevice):
             :keyword uuid: the VG UUID
             :type uuid: str
         """
-        self.pvClass = get_device_format_class("lvmpv")
-        if not self.pvClass:
-            raise StorageError("cannot find 'lvmpv' class")
-
-        if isinstance(parents, list):
-            for dev in parents:
-                if not isinstance(dev.format, self.pvClass):
-                    raise ValueError("constructor requires a list of PVs")
-        elif not isinstance(parents.format, self.pvClass):
-            raise ValueError("constructor requires a list of PVs")
-
-        DMDevice.__init__(self, name, parents=parents,
-                          exists=exists, sysfsPath=sysfsPath)
+        super(LVMVolumeGroupDevice, self).__init__(name, parents=parents,
+                                            exists=exists, sysfsPath=sysfsPath)
 
         self.uuid = uuid
         self.free = util.numeric_type(free)
@@ -2196,56 +2272,6 @@ class LVMVolumeGroupDevice(DMDevice):
 
         return True
 
-    def _addDevice(self, device):
-        """ Add a new physical volume device to the volume group.
-
-            XXX This is for use by device probing routines and is not
-                intended for modification of the VG.
-        """
-        log_method_call(self,
-                        self.name,
-                        device=device.name,
-                        status=self.status)
-        if not self.exists:
-            raise DeviceError("device does not exist", self.name)
-
-        if not isinstance(device.format, self.pvClass):
-            raise ValueError("addDevice requires a PV arg")
-
-        if self.uuid and device.format.vgUuid != self.uuid:
-            # this means there is another vg with the same name on the system
-            # set hasDuplicate which will make complete return False
-            # and let devicetree._handleInconsistencies() further handle this.
-            # Note we still add the device to our parents for use by
-            # devicetree._handleInconsistencies()
-            self.hasDuplicate = True
-
-        if device in self.pvs:
-            raise ValueError("device is already a member of this VG")
-
-        self.parents.append(device)
-        device.addChild()
-
-        # now see if the VG can be activated
-        if self.complete and flags.installer_mode:
-            self.setup()
-
-    def _removeDevice(self, device):
-        """ Remove a physical volume from the volume group.
-
-            This is for cases like clearing of preexisting partitions.
-        """
-        log_method_call(self,
-                        self.name,
-                        device=device.name,
-                        status=self.status)
-        try:
-            self.parents.remove(device)
-        except ValueError:
-            raise ValueError("cannot remove non-member PV device from VG")
-
-        device.removeChild()
-
     def _preSetup(self, orig=False):
         if self.exists and not self.complete:
             raise DeviceError("cannot activate VG with missing PV(s)", self.name)
@@ -2319,42 +2345,24 @@ class LVMVolumeGroupDevice(DMDevice):
         if self.poolMetaData and not self.thinpools:
             self.poolMetaData = 0
 
-    def _addPV(self, pv):
-        """ Add a PV to this VG. """
-        if pv in self.pvs:
-            raise ValueError("pv is already part of this vg")
+    def _addMember(self, member):
+        super(LVMVolumeGroupDevice, self)._addMember(member)
 
-        # for the time being we will not allow vgextend
-        if self.exists:
-            raise DeviceError("cannot add pv to existing vg", self.name)
-
-        self.parents.append(pv)
-        pv.addChild()
+        # now see if the VG can be activated
+        ## XXX TODO: remove this activation code
+        if self.exists and member.format.exists and self.complete and \
+           flags.installer_mode:
+            self.setup()
 
         # and update our pv count
         self.pvCount = len(self.parents)
 
-    def addMember(self, member):
-        self._addPV(member)
-
-    def _removePV(self, pv):
-        """ Remove an PV from this VG. """
-        if not pv in self.pvs:
-            raise ValueError("specified pv is not part of this vg")
-
-        # for the time being we will not allow vgreduce
-        if self.exists:
-            raise DeviceError("cannot remove pv from existing vg", self.name)
-
-        self.parents.remove(pv)
-        pv.removeChild()
+    def _removeMember(self, member):
+        super(LVMVolumeGroupDevice, self)._removeMember(member)
 
         # and update our pv count
         self.pvCount = len(self.parents)
 
-    def removeMember(self, member):
-        self._removePV(member)
-
     # We can't rely on lvm to tell us about our size, free space, &c
     # since we could have modifications queued, unless the VG and all of
     # its PVs already exist.
@@ -2961,11 +2969,13 @@ class LVMThinLogicalVolumeDevice(LVMLogicalVolumeDevice):
         data.thin_volume = True
         data.pool_name = self.pool.lvname
 
-class MDRaidArrayDevice(StorageDevice):
+class MDRaidArrayDevice(ContainerDevice):
     """ An mdraid (Linux RAID) device. """
     _type = "mdarray"
     _packages = ["mdadm"]
     _devDir = "/dev/md"
+    _formatClassName = "mdmember"
+    _formatUUIDAttr = "mdUuid"
 
     def __init__(self, name, level=None, major=None, minor=None, size=None,
                  memberDevices=None, totalDevices=None,
@@ -2994,9 +3004,12 @@ class MDRaidArrayDevice(StorageDevice):
             :keyword minor: the device minor (obsolete?)
             :type minor: int
         """
-        StorageDevice.__init__(self, name, format=format, exists=exists,
-                               major=major, minor=minor, size=size,
-                               parents=parents, sysfsPath=sysfsPath)
+        self._memberDevices = 0
+        self._totalDevices = 0
+        super(MDRaidArrayDevice, self).__init__(name, format=format,
+                                                exists=exists, size=size,
+                                                parents=parents,
+                                                sysfsPath=sysfsPath)
 
         if level == "container":
             self._type = "mdcontainer"
@@ -3028,12 +3041,6 @@ class MDRaidArrayDevice(StorageDevice):
             self._size = self.currentSize
             self._type = "mdbiosraidarray"
 
-        self.formatClass = get_device_format_class("mdmember")
-        if not self.formatClass:
-            for dev in self.parents:
-                dev.removeChild()
-            raise DeviceError("cannot find class for 'mdmember'", self.name)
-
         if self.exists and self.uuid and not flags.testing:
             # this is a hack to work around mdadm's insistence on giving
             # really high minors to arrays it has no config entry for
@@ -3228,83 +3235,35 @@ class MDRaidArrayDevice(StorageDevice):
         else:
             self.sysfsPath = ''
 
-    def _addDevice(self, device):
-        """ Add a new member device to the array.
-
-            XXX This is for use when probing devices, not for modification
-                of arrays.
-        """
-        log_method_call(self,
-                        self.name,
-                        device=device.name,
-                        status=self.status)
-        if not self.exists:
-            raise DeviceError("device has not been created", self.name)
-
-        if not isinstance(device.format, self.formatClass):
-            raise ValueError("invalid device format for mdraid member")
-
-        if self.uuid and device.format.mdUuid != self.uuid:
-            raise ValueError("cannot add member with non-matching UUID")
+    def _addMember(self, member):
+        super(MDRaidArrayDevice, self)._addMember(member)
 
-        if device in self.devices:
-            raise ValueError("device is already a member of this array")
-
-        # we added it, so now set up the relations
-        self.devices.append(device)
-        device.addChild()
-
-        if flags.installer_mode:
-            device.setup()
+        ## XXX TODO: remove this whole block of activation code
+        if self.exists and member.format.exists and flags.installer_mode:
+            member.setup()
             udev_settle()
 
-            if self.spares > 0:
-                # mdadm doesn't like it when you try to incrementally add spares
-                return
-
-            try:
-                mdraid.mdadd(device.path)
-                # mdadd causes udev events
-                udev_settle()
-            except MDRaidError as e:
-                log.warning("failed to add member %s to md array %s: %s"
-                            % (device.path, self.path, e))
-
-        if self.status:
+            if self.spares <= 0:
+                try:
+                    mdraid.mdadd(None, [member.path], incremental=True)
+                    # mdadd causes udev events
+                    udev_settle()
+                except MDRaidError as e:
+                    log.warning("failed to add member %s to md array %s: %s"
+                                % (member.path, self.path, e))
+
+        if self.status and member.format.exists:
             # we always probe since the device may not be set up when we want
             # information about it
             self._size = self.currentSize
 
-    def _removeDevice(self, device):
-        """ Remove a component device from the array.
-
-            XXX This is for use by clearpart, not for reconfiguration.
-        """
-        log_method_call(self,
-                        self.name,
-                        device=device.name,
-                        status=self.status)
-
-        if device not in self.devices:
-            raise ValueError("cannot remove non-member device from array")
-
-        self.devices.remove(device)
-        device.removeChild()
-
-    def addMember(self, member):
-        if member in self.parents:
-            raise ValueError("member is already part of this array")
-
-        # for the time being we will not allow adding members to existing arrays
-        if self.exists:
-            raise DeviceError("cannot add member to existing array", self.name)
+        if not self.exists:
+            self._totalDevices += 1
 
-        self.parents.append(member)
-        member.addChild()
         self.memberDevices += 1
 
-    def removeMember(self, member):
-        self._removeDevice(member)
+    def _removeMember(self, member):
+        super(MDRaidArrayDevice, self)._removeMember(member)
         self.memberDevices -= 1
 
     @property
@@ -3501,12 +3460,13 @@ class MDRaidArrayDevice(StorageDevice):
         data.preexist = self.exists
         data.device = self.name
 
-class DMRaidArrayDevice(DMDevice):
+class DMRaidArrayDevice(DMDevice, ContainerDevice):
     """ A dmraid (device-mapper RAID) device """
     _type = "dm-raid array"
     _packages = ["dmraid"]
     _partitionable = True
     _isDisk = True
+    _formatClassName = "dmraidmember"
 
     def __init__(self, name, raidSet=None, format=None,
                  size=None, parents=None, sysfsPath=''):
@@ -3527,16 +3487,9 @@ class DMRaidArrayDevice(DMDevice):
             DMRaidArrayDevices always exist. Blivet cannot create or destroy
             them.
         """
-        if isinstance(parents, list):
-            for parent in parents:
-                if not parent.format or parent.format.type != "dmraidmember":
-                    raise ValueError("parent devices must contain dmraidmember format")
-        DMDevice.__init__(self, name, format=format, size=size,
-                          parents=parents, sysfsPath=sysfsPath, exists=True)
-
-        self.formatClass = get_device_format_class("dmraidmember")
-        if not self.formatClass:
-            raise StorageError("cannot find class for 'dmraidmember'")
+        super(DMRaidArrayDevice, self).__init__(name, format=format, size=size,
+                                                parents=parents, exists=True,
+                                                sysfsPath=sysfsPath)
 
         self._raidSet = raidSet
 
@@ -3544,27 +3497,6 @@ class DMRaidArrayDevice(DMDevice):
     def raidSet(self):
         return self._raidSet
 
-    def _addDevice(self, device):
-        """ Add a new member device to the array.
-
-            XXX This is for use when probing devices, not for modification
-                of arrays.
-        """
-        log_method_call(self, self.name, device=device.name, status=self.status)
-
-        if not self.exists:
-            raise DeviceError("device has not been created", self.name)
-
-        if not isinstance(device.format, self.formatClass):
-            raise ValueError("invalid device format for dmraid member")
-
-        if device in self.members:
-            raise ValueError("device is already a member of this array")
-
-        # we added it, so now set up the relations
-        self.devices.append(device)
-        device.addChild()
-
     @property
     def members(self):
         return self.parents
@@ -4438,7 +4370,7 @@ class BTRFSDevice(StorageDevice):
 
     @property
     def path(self):
-        return self.parents[0].path
+        return self.parents[0].path if self.parents else None
 
     @property
     def fstabSpec(self):
@@ -4448,9 +4380,11 @@ class BTRFSDevice(StorageDevice):
             spec = super(BTRFSDevice, self).fstabSpec
         return spec
 
-class BTRFSVolumeDevice(BTRFSDevice):
+class BTRFSVolumeDevice(BTRFSDevice, ContainerDevice):
     _type = "btrfs volume"
     vol_id = btrfs.MAIN_VOLUME_ID
+    _formatClassName = "btrfs"
+    _formatUUIDAttr = "volUUID"
 
     def __init__(self, *args, **kwargs):
         self.dataLevel = kwargs.pop("dataLevel", None)
@@ -4461,16 +4395,6 @@ class BTRFSVolumeDevice(BTRFSDevice):
         self.subvolumes = []
         self.size_policy = self.size
 
-        for parent in self.parents:
-            if parent.format.type != "btrfs":
-                raise ValueError("member device %s is not BTRFS" % parent.name)
-
-            if parent.format.exists and self.exists and \
-               parent.format.volUUID != self.uuid:
-                raise ValueError("BTRFS member device %s UUID %s does not "
-                                 "match volume UUID %s" % (parent.name,
-                                 parent.format.volUUID, self.uuid))
-
         if self.parents and not self.format.type:
             label = getattr(self.parents[0].format, "label", None)
             self.format = getFormat("btrfs",
@@ -4498,68 +4422,6 @@ class BTRFSVolumeDevice(BTRFSDevice):
 
         return size
 
-    def _addDevice(self, device):
-        """ Add a new device to this volume.
-
-            XXX This is for use by device probing routines and is not
-                intended for modification of the volume.
-        """
-        log_method_call(self,
-                        self.name,
-                        device=device.name,
-                        status=self.status)
-        if not self.exists:
-            raise DeviceError("device does not exist", self.name)
-
-        if device.format.type != "btrfs":
-            raise ValueError("addDevice requires a btrfs device as sole arg")
-
-        if device.format.volUUID != self.uuid:
-            raise ValueError("device UUID does not match the volume UUID")
-
-        if device in self.parents:
-            raise ValueError("device is already a member of this volume")
-
-        self.parents.append(device)
-        device.addChild()
-
-    def _removeDevice(self, device):
-        """ Remove a device from the volume.
-
-            This is for cases like clearing of preexisting partitions.
-        """
-        log_method_call(self,
-                        self.name,
-                        device=device.name,
-                        status=self.status)
-        try:
-            self.parents.remove(device)
-        except ValueError:
-            raise ValueError("cannot remove non-member device from volume")
-
-        device.removeChild()
-
-    def addMember(self, member):
-        if member in self.parents:
-            raise ValueError("member is already part of this volume")
-
-        # for the time being we will not allow adding members to existing vols
-        if self.exists:
-            raise DeviceError("cannot add member to existing volume", self.name)
-
-        self.parents.append(member)
-        member.addChild()
-
-    def removeMember(self, member):
-        if member not in self.parents:
-            raise ValueError("member is not part of this volume")
-
-        if self.exists:
-            raise DeviceError("cannot remove member from an existing volume")
-
-        self.parents.remove(member)
-        member.removeChild()
-
     def _addSubVolume(self, vol):
         if vol.name in [v.name for v in self.subvolumes]:
             raise ValueError("subvolume %s already exists" % vol.name)
-- 
1.8.5.3

