From 4c6743851dcf1e399a785482ac78ffa3df40cc7f Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 22 Nov 2023 13:19:13 +0100 Subject: [PATCH 1/7] Add "device ID" that could be used as a unique device identifier Currently device name is commonly used as a unique device identifier both in blivet internally and in Anaconda. The problem with this is that duplicate names are allowed in the system in some cases -- btrfs names doesn't need to be unique and we can also have two different devices with the same name if they use different technologies (e.g. an LVM VG and MD array can share the same name). The new "device_ID" parameter is designed to be usable as a unique identifier and to be different for devices that share the same name. The device ID looks as follows: - name for disks, partitions and loop devices - LVM- for LVM VGs and LVs - BTRFS-- for btrfs volumes and subvolumes - STRATIS- for Stratis devices - LUKS- for LUKS mapped devices - MDRAID- for MD arrays - DM- for generic DM devices --- blivet/devices/btrfs.py | 10 ++++++++++ blivet/devices/device.py | 10 ++++++++++ blivet/devices/dm.py | 5 +++++ blivet/devices/luks.py | 5 +++++ blivet/devices/lvm.py | 10 ++++++++++ blivet/devices/md.py | 4 ++++ blivet/devices/stratis.py | 10 ++++++++++ 7 files changed, 54 insertions(+) diff --git a/blivet/devices/btrfs.py b/blivet/devices/btrfs.py index 8e62165ca..259bcf82a 100644 --- a/blivet/devices/btrfs.py +++ b/blivet/devices/btrfs.py @@ -226,6 +226,11 @@ def __init__(self, *args, **kwargs): def members(self): return list(self.parents) + @property + def device_id(self): + # BTRFS- + return "BTRFS-%s" % self.uuid + def _set_level(self, value, data): """ Sets a valid level for this device and level type. @@ -599,6 +604,11 @@ def volume(self): def container(self): return self.volume + @property + def device_id(self): + # BTRFS-- + return "BTRFS-%s-%s" % (self.volume.uuid, self.name) + def setup_parents(self, orig=False): """ Run setup method of all parent devices. """ log_method_call(self, name=self.name, orig=orig) diff --git a/blivet/devices/device.py b/blivet/devices/device.py index 33814f56e..d82c23b22 100644 --- a/blivet/devices/device.py +++ b/blivet/devices/device.py @@ -262,6 +262,16 @@ def _set_name(self, value): lambda s, v: s._set_name(v), doc="This device's name") + @property + def device_id(self): + """ Unique ID for this device. + + Note: This is an internal identifier in Blivet which is designed to be unique + even for devices with the same name or label. + This ID is persistent between blivet.reset() calls for existing devices. + """ + return self.name + @property def isleaf(self): """ True if no other device depends on this one. """ diff --git a/blivet/devices/dm.py b/blivet/devices/dm.py index b48cd6140..38cfbf78b 100644 --- a/blivet/devices/dm.py +++ b/blivet/devices/dm.py @@ -99,6 +99,11 @@ def map_name(self): """ This device's device-mapper map name """ return self.name + @property + def device_id(self): + # DM- + return "DM-%s" % self.name + @property def status(self): try: diff --git a/blivet/devices/luks.py b/blivet/devices/luks.py index 59260905d..c22453756 100644 --- a/blivet/devices/luks.py +++ b/blivet/devices/luks.py @@ -70,6 +70,11 @@ def raw_device(self): return self.parents[0].parents[0] return self.parents[0] + @property + def device_id(self): + # LUKS- + return "LUKS-%s" % self.name + def _get_size(self): if not self.exists: size = self.raw_device.size - crypto.LUKS_METADATA_SIZE diff --git a/blivet/devices/lvm.py b/blivet/devices/lvm.py index c9c9293ac..158f5569c 100644 --- a/blivet/devices/lvm.py +++ b/blivet/devices/lvm.py @@ -208,6 +208,11 @@ def path(self): """ Device node representing this device. """ return "%s/%s" % (self._dev_dir, self.map_name) + @property + def device_id(self): + # LVM- + return "LVM-%s" % self.name + def update_sysfs_path(self): """ Update this device's sysfs path. """ log_method_call(self, self.name, status=self.status) @@ -1176,6 +1181,11 @@ def lvname(self): """ The LV's name (not including VG name). """ return self._name + @property + def device_id(self): + # LVM-- + return "LVM-%s" % self.name + @property def complete(self): """ Test if vg exits and if it has all pvs. """ diff --git a/blivet/devices/md.py b/blivet/devices/md.py index f6b5c46e7..9efca08b5 100644 --- a/blivet/devices/md.py +++ b/blivet/devices/md.py @@ -150,6 +150,10 @@ def __init__(self, name, level=None, major=None, minor=None, size=None, if self.parents and self.parents[0].type == "mdcontainer" and self.type != "mdbiosraidarray": raise errors.DeviceError("A device with mdcontainer member must be mdbiosraidarray.") + @property + def device_id(self): + return "MDRAID-%s" % self.name + @property def mdadm_format_uuid(self): """ This array's UUID, formatted for external use. diff --git a/blivet/devices/stratis.py b/blivet/devices/stratis.py index 4528dcd0c..6611bd60d 100644 --- a/blivet/devices/stratis.py +++ b/blivet/devices/stratis.py @@ -58,6 +58,11 @@ def __init__(self, *args, **kwargs): super(StratisPoolDevice, self).__init__(*args, **kwargs) + @property + def device_id(self): + # STRATIS- + return "STRATIS-%s" % self.name + @property def blockdevs(self): """ A list of this pool block devices """ @@ -216,6 +221,11 @@ def fsname(self): """ The Stratis filesystem name (not including pool name). """ return self._name + @property + def device_id(self): + # STRATIS-/ + return "STRATIS-%s/%s" % (self.pool.name, self.fsname) + @property def pool(self): if not self.parents: From 7f9601c0320a34027d877df8b6fd6c3805589e6d Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 29 Nov 2023 15:46:33 +0100 Subject: [PATCH 2/7] Add a function to get a device by device ID --- blivet/devicetree.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/blivet/devicetree.py b/blivet/devicetree.py index 181763a03..6f0e1eee7 100644 --- a/blivet/devicetree.py +++ b/blivet/devicetree.py @@ -585,7 +585,7 @@ def get_device_by_path(self, path, incomplete=False, hidden=False): return result def get_device_by_id(self, id_num, incomplete=False, hidden=False): - """ Return a device with specified device id. + """ Return a device with specified id. :param int id_num: the id to look for :param bool incomplete: include incomplete devices in search @@ -599,6 +599,21 @@ def get_device_by_id(self, id_num, incomplete=False, hidden=False): log_method_return(self, result) return result + def get_device_by_device_id(self, device_id, incomplete=False, hidden=False): + """ Return a device with specified device id. + + :param str device id: the device id to look for + :param bool incomplete: include incomplete devices in search + :param bool hidden: include hidden devices in search + :returns: the first matching device found + :rtype: :class:`~.devices.Device` + """ + log_method_call(self, device_id=device_id, incomplete=incomplete, hidden=hidden) + devices = self._filter_devices(incomplete=incomplete, hidden=hidden) + result = six.next((d for d in devices if d.device_id == device_id), None) + log_method_return(self, result) + return result + def resolve_device(self, devspec, blkid_tab=None, crypt_tab=None, options=None, subvolspec=None): """ Return the device matching the provided device specification. From d9350fc719f17b891fd6f643a688e0dced30fc2c Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 29 Nov 2023 15:46:51 +0100 Subject: [PATCH 3/7] Use get_device_by_device_id instead of _by_name in populator We can now internally use the device_id instead of name when getting device to be sure we will get the expected device a not a different device with the same name. --- blivet/blivet.py | 2 +- blivet/populator/helpers/luks.py | 2 +- blivet/populator/helpers/lvm.py | 27 +++++++------ blivet/populator/helpers/mdraid.py | 4 +- blivet/populator/helpers/partition.py | 6 +-- blivet/populator/populator.py | 2 +- tests/unit_tests/populator_test.py | 58 ++++++++++----------------- 7 files changed, 45 insertions(+), 56 deletions(-) diff --git a/blivet/blivet.py b/blivet/blivet.py index e1c0042d8..f290a0695 100644 --- a/blivet/blivet.py +++ b/blivet/blivet.py @@ -419,7 +419,7 @@ def remove_empty_extended_partitions(self): if extended and not logical_parts: log.debug("removing empty extended partition from %s", disk.name) extended_name = device_path_to_name(extended.getDeviceNodeName()) - extended = self.devicetree.get_device_by_name(extended_name) + extended = self.devicetree.get_device_by_device_id(extended_name) self.destroy_device(extended) def get_free_space(self, disks=None, partitions=None): diff --git a/blivet/populator/helpers/luks.py b/blivet/populator/helpers/luks.py index ad07a432e..2df18b417 100644 --- a/blivet/populator/helpers/luks.py +++ b/blivet/populator/helpers/luks.py @@ -121,7 +121,7 @@ def run(self): return # look up or create the mapped device - if not self._devicetree.get_device_by_name(self.device.format.map_name): + if not self._devicetree.get_device_by_device_id("LUKS-" + self.device.format.map_name): passphrase = luks_data.luks_devs.get(self.device.format.uuid) if self.device.format.configured: pass diff --git a/blivet/populator/helpers/lvm.py b/blivet/populator/helpers/lvm.py index 212d36445..0cf47ba43 100644 --- a/blivet/populator/helpers/lvm.py +++ b/blivet/populator/helpers/lvm.py @@ -53,9 +53,10 @@ def run(self): log_method_call(self, name=name) vg_name = udev.device_get_lv_vg_name(self.data) - device = self._devicetree.get_device_by_name(vg_name, hidden=True) + vg_id = "LVM-%s" % vg_name + device = self._devicetree.get_device_by_device_id(vg_id, hidden=True) if device and not isinstance(device, LVMVolumeGroupDevice): - log.warning("found non-vg device with name %s", vg_name) + log.warning("found non-vg device with device id %s", vg_id) device = None self._devicetree._add_parent_devices(self.data) @@ -63,11 +64,12 @@ def run(self): # LVM provides no means to resolve conflicts caused by duplicated VG # names, so we're just being optimistic here. Woo! vg_name = udev.device_get_lv_vg_name(self.data) - vg_device = self._devicetree.get_device_by_name(vg_name) + vg_id = "LVM-%s" % vg_name + vg_device = self._devicetree.get_device_by_device_id(vg_id) if not vg_device: log.error("failed to find vg '%s' after scanning pvs", vg_name) - return self._devicetree.get_device_by_name(name) + return self._devicetree.get_device_by_device_id("LVM-" + name) def _handle_rename(self): name = self.data.get("DM_LV_NAME") @@ -161,12 +163,13 @@ def add_required_lv(name, msg): :raises: :class:`~.errors.DeviceTreeError` on failure """ - vol = self._devicetree.get_device_by_name(name) + vgid = "LVM-" + name + vol = self._devicetree.get_device_by_device_id(vgid) if vol is None: new_lv = add_lv(lv_info[name]) if new_lv: all_lvs.append(new_lv) - vol = self._devicetree.get_device_by_name(name) + vol = self._devicetree.get_device_by_device_id(vgid) if vol is None: log.error("%s: %s", msg, name) @@ -184,7 +187,7 @@ def add_lv(lv): lv_kwargs = {} name = "%s-%s" % (vg_name, lv_name) - lv_device = self._devicetree.get_device_by_name(name) + lv_device = self._devicetree.get_device_by_device_id("LVM-" + name) if lv_device is not None: # some lvs may have been added on demand below log.debug("already added %s", name) @@ -215,7 +218,7 @@ def add_lv(lv): origin_device_name = "%s-%s" % (vg_name, origin_name) add_required_lv(origin_device_name, "failed to locate origin lv") - origin = self._devicetree.get_device_by_name(origin_device_name) + origin = self._devicetree.get_device_by_device_id("LVM-" + origin_device_name) lv_kwargs["origin"] = origin elif lv_attr[0] in 'IrielTCoD' and lv_name.endswith(']'): @@ -238,10 +241,10 @@ def add_lv(lv): if origin_name: origin_device_name = "%s-%s" % (vg_name, origin_name) add_required_lv(origin_device_name, "failed to locate origin lv") - origin = self._devicetree.get_device_by_name(origin_device_name) + origin = self._devicetree.get_device_by_device_id("LVM-" + origin_device_name) lv_kwargs["origin"] = origin - lv_parents = [self._devicetree.get_device_by_name(pool_device_name)] + lv_parents = [self._devicetree.get_device_by_device_id("LVM-" + pool_device_name)] elif lv_attr[0] == 'd': # vdo pool if BLOCKDEV_LVM_PLUGIN_VDO.available: @@ -259,7 +262,7 @@ def add_lv(lv): pool_device_name = "%s-%s" % (vg_name, pool_name) add_required_lv(pool_device_name, "failed to look up VDO pool") - lv_parents = [self._devicetree.get_device_by_name(pool_device_name)] + lv_parents = [self._devicetree.get_device_by_device_id("LVM-" + pool_device_name)] elif lv_name.endswith(']'): # unrecognized Internal LVM2 device return @@ -385,7 +388,7 @@ def _add_vg_device(self): vg_device.parents.append(self.device) callbacks.parent_added(device=vg_device, parent=self.device) elif vg_device is None: - same_name = self._devicetree.get_device_by_name(vg_name) + same_name = self._devicetree.get_device_by_device_id("LVM-" + vg_name) if isinstance(same_name, LVMVolumeGroupDevice): raise DuplicateVGError("multiple LVM volume groups with the same name (%s)" % vg_name) diff --git a/blivet/populator/helpers/mdraid.py b/blivet/populator/helpers/mdraid.py index 9907c8a88..6d66b0c07 100644 --- a/blivet/populator/helpers/mdraid.py +++ b/blivet/populator/helpers/mdraid.py @@ -58,7 +58,7 @@ def run(self): return None # try to get the device again now that we've got all the parents - device = self._devicetree.get_device_by_name(name, incomplete=flags.allow_imperfect_devices) + device = self._devicetree.get_device_by_device_id("MDRAID-" + name, incomplete=flags.allow_imperfect_devices) if device is None: try: @@ -175,7 +175,7 @@ def run(self): md_name = md_name[2:] if md_name: - array = self._devicetree.get_device_by_name(md_name, incomplete=True) + array = self._devicetree.get_device_by_device_id("MDRAID-" + md_name, incomplete=True) if array and array.uuid != md_uuid: log.error("found multiple devices with the name %s", md_name) diff --git a/blivet/populator/helpers/partition.py b/blivet/populator/helpers/partition.py index 9257407e6..2548f0e94 100644 --- a/blivet/populator/helpers/partition.py +++ b/blivet/populator/helpers/partition.py @@ -46,21 +46,21 @@ def run(self): log_method_call(self, name=name) sysfs_path = udev.device_get_sysfs_path(self.data) - device = self._devicetree.get_device_by_name(name) + device = self._devicetree.get_device_by_device_id(name) if device: return device disk = None disk_name = udev.device_get_partition_disk(self.data) if disk_name: - disk = self._devicetree.get_device_by_name(disk_name) + disk = self._devicetree.get_device_by_device_id(disk_name) if disk is None: # create a device instance for the disk disk_info = six.next((i for i in udev.get_devices() if udev.device_get_name(i) == disk_name), None) if disk_info is not None: self._devicetree.handle_device(disk_info) - disk = self._devicetree.get_device_by_name(disk_name) + disk = self._devicetree.get_device_by_device_id(disk_name) if disk is None: # if the disk is still not in the tree something has gone wrong diff --git a/blivet/populator/populator.py b/blivet/populator/populator.py index f52e7f09e..c842ef7c9 100644 --- a/blivet/populator/populator.py +++ b/blivet/populator/populator.py @@ -192,7 +192,7 @@ def _handle_degraded_md(self, info, device): if md_name is None: log.warning("No name for possibly degraded md array.") else: - device = self.get_device_by_name(md_name, incomplete=flags.allow_imperfect_devices) + device = self.get_device_by_device_id("MDRAID-" + md_name, incomplete=flags.allow_imperfect_devices) if device and not isinstance(device, MDRaidArrayDevice): log.warning("Found device %s, but it turns out not be an md array device after all.", device.name) diff --git a/tests/unit_tests/populator_test.py b/tests/unit_tests/populator_test.py index 8d43ac367..febdf92ce 100644 --- a/tests/unit_tests/populator_test.py +++ b/tests/unit_tests/populator_test.py @@ -236,7 +236,7 @@ def test_get_helper(self, *args): # a failure because the ordering is not complete, meaning any of several device helpers # could be the first helper class checked. - @patch.object(DeviceTree, "get_device_by_name") + @patch.object(DeviceTree, "get_device_by_device_id") @patch.object(DeviceTree, "_add_parent_devices") @patch("blivet.udev.device_get_name") @patch("blivet.udev.device_get_lv_vg_name") @@ -244,53 +244,39 @@ def test_run(self, *args): """Test lvm device populator.""" device_get_lv_vg_name = args[0] device_get_name = args[1] - get_device_by_name = args[3] + get_device_by_device_id = args[3] devicetree = DeviceTree() data = Mock() + lv_name = "lvtest" + vg_name = "vg_test" + # Add parent devices and then look up the device. - device_get_name.return_value = sentinel.lv_name - devicetree.get_device_by_name.return_value = None + device_get_name.return_value = lv_name + devicetree.get_device_by_device_id.return_value = None # pylint: disable=unused-argument - def _get_device_by_name(name, **kwargs): - if name == sentinel.lv_name: + def _get_device_by_device_id(device_id, **kwargs): + if device_id == "LVM-" + lv_name: return sentinel.lv_device - get_device_by_name.side_effect = _get_device_by_name - device_get_lv_vg_name.return_value = sentinel.vg_name + get_device_by_device_id.side_effect = _get_device_by_device_id + device_get_lv_vg_name.return_value = vg_name helper = self.helper_class(devicetree, data) self.assertEqual(helper.run(), sentinel.lv_device) - self.assertEqual(devicetree.get_device_by_name.call_count, 3) # pylint: disable=no-member - get_device_by_name.assert_has_calls( - [call(sentinel.vg_name, hidden=True), - call(sentinel.vg_name), - call(sentinel.lv_name)]) + self.assertEqual(devicetree.get_device_by_device_id.call_count, 3) # pylint: disable=no-member + get_device_by_device_id.assert_has_calls( + [call("LVM-" + vg_name, hidden=True), + call("LVM-" + vg_name), + call("LVM-" + lv_name)]) # Add parent devices, but the device is still not in the tree - get_device_by_name.side_effect = None - get_device_by_name.return_value = None + get_device_by_device_id.side_effect = None + get_device_by_device_id.return_value = None self.assertEqual(helper.run(), None) - get_device_by_name.assert_called_with(sentinel.lv_name) - - # A non-vg device with the same name as the vg is already in the tree. - # pylint: disable=unused-argument - def _get_device_by_name2(name, **kwargs): - if name == sentinel.lv_name: - return sentinel.lv_device - elif name == sentinel.vg_name: - return sentinel.non_vg_device - - get_device_by_name.side_effect = _get_device_by_name2 - if six.PY3: - with self.assertLogs('blivet', level='WARNING') as log_cm: - self.assertEqual(helper.run(), sentinel.lv_device) - log_entry = "WARNING:blivet:found non-vg device with name %s" % sentinel.vg_name - self.assertTrue(log_entry in log_cm.output) - else: - self.assertEqual(helper.run(), sentinel.lv_device) + get_device_by_device_id.assert_called_with("LVM-" + lv_name) class OpticalDevicePopulatorTestCase(PopulatorHelperTestCase): @@ -687,7 +673,7 @@ def test_get_helper(self, *args): # a failure because the ordering is not complete, meaning any of several device helpers # could be the first helper class checked. - @patch.object(DeviceTree, "get_device_by_name") + @patch.object(DeviceTree, "get_device_by_device_id") @patch.object(DeviceTree, "_add_parent_devices") @patch("blivet.udev.device_get_name") @patch("blivet.udev.device_get_md_uuid") @@ -695,7 +681,7 @@ def test_get_helper(self, *args): def test_run(self, *args): """Test md device populator.""" device_get_md_name = args[0] - get_device_by_name = args[4] + get_device_by_device_id = args[4] devicetree = DeviceTree() @@ -706,7 +692,7 @@ def test_run(self, *args): device_name = "mdtest" device_get_md_name.return_value = device_name - get_device_by_name.return_value = device + get_device_by_device_id.return_value = device helper = self.helper_class(devicetree, data) self.assertEqual(helper.run(), device) From 6a615d5d7952ba1f2ae322302c71e92ca78c0a3f Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Mon, 4 Dec 2023 14:18:14 +0100 Subject: [PATCH 4/7] tests: Add a simple test for DeviceTree.get_device_by_device_id --- tests/unit_tests/devicetree_test.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/unit_tests/devicetree_test.py b/tests/unit_tests/devicetree_test.py index 8d26d5807..77781d42f 100644 --- a/tests/unit_tests/devicetree_test.py +++ b/tests/unit_tests/devicetree_test.py @@ -300,6 +300,29 @@ def test_get_device_by_name(self): self.assertIsNone(dt.get_device_by_name("dev3")) self.assertEqual(dt.get_device_by_name("dev3", hidden=True), dev3) + def test_get_device_by_device_id(self): + dt = DeviceTree() + + # for StorageDevice, device_id is just name + dev1 = StorageDevice("dev1", exists=False, parents=[]) + dev2 = StorageDevice("dev2", exists=False, parents=[dev1]) + dt._add_device(dev1) + dt._add_device(dev2) + + self.assertIsNone(dt.get_device_by_device_id("dev3")) + self.assertEqual(dt.get_device_by_device_id("dev2"), dev2) + self.assertEqual(dt.get_device_by_device_id("dev1"), dev1) + + dev2.complete = False + self.assertEqual(dt.get_device_by_device_id("dev2"), None) + self.assertEqual(dt.get_device_by_device_id("dev2", incomplete=True), dev2) + + dev3 = StorageDevice("dev3", exists=True, parents=[]) + dt._add_device(dev3) + dt.hide(dev3) + self.assertIsNone(dt.get_device_by_device_id("dev3")) + self.assertEqual(dt.get_device_by_device_id("dev3", hidden=True), dev3) + def test_recursive_remove(self): dt = DeviceTree() dev1 = StorageDevice("dev1", exists=False, parents=[]) From 4170889170b9ca8412f17abcda618f14db65989e Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Mon, 4 Dec 2023 14:36:50 +0100 Subject: [PATCH 5/7] tests: Add basic unit tests for device_id --- tests/unit_tests/devices_test/btrfs_test.py | 10 ++++++++++ tests/unit_tests/devices_test/lvm_test.py | 10 ++++++++++ tests/unit_tests/devices_test/md_test.py | 10 ++++++++++ tests/unit_tests/devices_test/partition_test.py | 4 ++++ tests/unit_tests/devices_test/stratis_test.py | 10 ++++++++++ 5 files changed, 44 insertions(+) diff --git a/tests/unit_tests/devices_test/btrfs_test.py b/tests/unit_tests/devices_test/btrfs_test.py index df7802680..fe6692c0a 100644 --- a/tests/unit_tests/devices_test/btrfs_test.py +++ b/tests/unit_tests/devices_test/btrfs_test.py @@ -55,3 +55,13 @@ def test_new_btrfs(self): self.assertEqual(sub.format.type, "btrfs") self.assertEqual(sub.size, vol.size) self.assertEqual(sub.volume, vol) + + def test_device_id(self): + bd = StorageDevice("bd1", fmt=blivet.formats.get_format("btrfs"), + size=Size("2 GiB"), exists=False) + + vol = BTRFSVolumeDevice("testvolume", parents=[bd]) + self.assertEqual(vol.device_id, "BTRFS-" + vol.uuid) + + sub = BTRFSSubVolumeDevice("testsub", parents=[vol]) + self.assertEqual(sub.device_id, "BTRFS-" + vol.uuid + "-testsub") diff --git a/tests/unit_tests/devices_test/lvm_test.py b/tests/unit_tests/devices_test/lvm_test.py index b80335ee8..16cbd85c7 100644 --- a/tests/unit_tests/devices_test/lvm_test.py +++ b/tests/unit_tests/devices_test/lvm_test.py @@ -571,6 +571,16 @@ def test_add_remove_pv(self): vg._remove_parent(pv2) self.assertEqual(pv2.format.vg_name, None) + def test_device_id(self): + pv = StorageDevice("pv1", fmt=blivet.formats.get_format("lvmpv"), + size=Size("1 GiB")) + vg = LVMVolumeGroupDevice("testvg", parents=[pv]) + self.assertEqual(vg.device_id, "LVM-testvg") + + lv = LVMLogicalVolumeDevice("testlv", parents=[vg], + fmt=blivet.formats.get_format("xfs")) + self.assertEqual(lv.device_id, "LVM-testvg-testlv") + class TypeSpecificCallsTest(unittest.TestCase): def test_type_specific_calls(self): diff --git a/tests/unit_tests/devices_test/md_test.py b/tests/unit_tests/devices_test/md_test.py index ad6a6290a..b3e825ca8 100644 --- a/tests/unit_tests/devices_test/md_test.py +++ b/tests/unit_tests/devices_test/md_test.py @@ -85,3 +85,13 @@ def test_chunk_size2(self): with six.assertRaisesRegex(self, ValueError, "specifying chunk size is not allowed for raid1"): raid_array.chunk_size = Size("512 KiB") + + def test_device_id(self): + member1 = StorageDevice("member1", fmt=blivet.formats.get_format("mdmember"), + size=Size("1 GiB")) + member2 = StorageDevice("member2", fmt=blivet.formats.get_format("mdmember"), + size=Size("1 GiB")) + + raid_array = MDRaidArrayDevice(name="raid", level="raid0", member_devices=2, + total_devices=2, parents=[member1, member2]) + self.assertEqual(raid_array.device_id, "MDRAID-raid") diff --git a/tests/unit_tests/devices_test/partition_test.py b/tests/unit_tests/devices_test/partition_test.py index fbb29a703..b17da9dc3 100644 --- a/tests/unit_tests/devices_test/partition_test.py +++ b/tests/unit_tests/devices_test/partition_test.py @@ -190,3 +190,7 @@ def test_disk_is_empty(self): PartitionDevice("testpart1", exists=True, parents=[disk]) self.assertFalse(disk.is_empty) + + def test_device_id(self): + part = PartitionDevice("req1", exists=False) + self.assertEqual(part.device_id, "req1") diff --git a/tests/unit_tests/devices_test/stratis_test.py b/tests/unit_tests/devices_test/stratis_test.py index 2ce873843..f22372219 100644 --- a/tests/unit_tests/devices_test/stratis_test.py +++ b/tests/unit_tests/devices_test/stratis_test.py @@ -104,3 +104,13 @@ def test_new_encrypted_stratis(self): encrypted=True, passphrase="secret", key_file=None) + + def test_device_id(self): + bd = StorageDevice("bd1", fmt=blivet.formats.get_format("stratis"), + size=Size("2 GiB"), exists=False) + + pool = StratisPoolDevice("testpool", parents=[bd]) + self.assertEqual(pool.device_id, "STRATIS-testpool") + + fs = StratisFilesystemDevice("testfs", parents=[pool], size=Size("1 GiB")) + self.assertEqual(fs.device_id, "STRATIS-testpool/testfs") From 3e23729cf7250fa98220264331911c4269f72562 Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Tue, 5 Dec 2023 12:12:40 +0100 Subject: [PATCH 6/7] tests: Add a test case with multiple devices with the same name --- tests/storage_tests/devices_test/misc_test.py | 142 ++++++++++++++++++ 1 file changed, 142 insertions(+) create mode 100644 tests/storage_tests/devices_test/misc_test.py diff --git a/tests/storage_tests/devices_test/misc_test.py b/tests/storage_tests/devices_test/misc_test.py new file mode 100644 index 000000000..5791fc2ad --- /dev/null +++ b/tests/storage_tests/devices_test/misc_test.py @@ -0,0 +1,142 @@ +import os +import unittest + +try: + from unittest.mock import patch +except ImportError: + from mock import patch + +from ..storagetestcase import StorageTestCase + +import blivet +from blivet.devices.btrfs import BTRFSVolumeDevice + + +class DeviceIDTestCase(StorageTestCase): + + vgname = "blivetTestVG" + + @classmethod + def setUpClass(cls): + unavailable_deps = BTRFSVolumeDevice.unavailable_type_dependencies() + if unavailable_deps: + dep_str = ", ".join([d.name for d in unavailable_deps]) + raise unittest.SkipTest("some unavailable dependencies required for this test: %s" % dep_str) + + def setUp(self): + super().setUp() + + # allow automounting to get btrfs information + blivet.flags.flags.auto_dev_updates = True + + disks = [os.path.basename(vdev) for vdev in self.vdevs] + self.storage = blivet.Blivet() + self.storage.exclusive_disks = disks + self.storage.reset() + + # make sure only the targetcli disks are in the devicetree + for disk in self.storage.disks: + self.assertTrue(disk.path in self.vdevs) + self.assertIsNone(disk.format.type) + self.assertFalse(disk.children) + + def _clean_up(self): + self.storage.reset() + for disk in self.storage.disks: + if disk.path not in self.vdevs: + raise RuntimeError("Disk %s found in devicetree but not in disks created for tests" % disk.name) + self.storage.recursive_remove(disk) + + self.storage.do_it() + + # reset back to default + blivet.flags.flags.auto_dev_updates = False + + return super()._clean_up() + + def test_same_name_devices(self): + """ Test that we can handle multiple devices with the same name + + This test cases creates three devices named "test": two btrfs + volumes and one VG and checks that reset() and get_device_by_device_id() + work as expected. + """ + + # BTRFS volume named "test" + disk = self.storage.devicetree.get_device_by_path(self.vdevs[0]) + self.assertIsNotNone(disk) + + self.storage.initialize_disk(disk) + + part = self.storage.new_partition(size=blivet.size.Size("1 GiB"), fmt_type="btrfs", + parents=[disk]) + self.storage.create_device(part) + + blivet.partitioning.do_partitioning(self.storage) + + vol = self.storage.new_btrfs(name="test", parents=[part]) + btrfs1_uuid = vol.uuid + self.storage.create_device(vol) + self.storage.do_it() + self.storage.reset() + + # second BTRFS volume also named "test" + disk = self.storage.devicetree.get_device_by_path(self.vdevs[1]) + self.assertIsNotNone(disk) + + self.storage.initialize_disk(disk) + + part = self.storage.new_partition(size=blivet.size.Size("1 GiB"), fmt_type="btrfs", + parents=[disk]) + self.storage.create_device(part) + + blivet.partitioning.do_partitioning(self.storage) + + # XXX we actually cannot create another device with the same name + with patch("blivet.devicetree.DeviceTree.names", []): + vol = self.storage.new_btrfs(name="test", parents=[part]) + btrfs2_uuid = vol.uuid + self.storage.create_device(vol) + self.storage.do_it() + self.storage.reset() + + # LVM VG named "test" + disk = self.storage.devicetree.get_device_by_path(self.vdevs[2]) + self.assertIsNotNone(disk) + + self.storage.initialize_disk(disk) + + pv = self.storage.new_partition(size=blivet.size.Size("100 MiB"), fmt_type="lvmpv", + parents=[disk]) + self.storage.create_device(pv) + + blivet.partitioning.do_partitioning(self.storage) + + # XXX we actually cannot create another device with the same name + with patch("blivet.devicetree.DeviceTree.names", []): + vg = self.storage.new_vg(name="test", parents=[pv]) + self.storage.create_device(vg) + self.storage.do_it() + self.storage.reset() + + # we should now have three devices named test + test_devs = [dev for dev in self.storage.devices if dev.name == "test"] + self.assertEqual(len(test_devs), 3) + + vol1 = self.storage.devicetree.get_device_by_device_id("BTRFS-" + btrfs1_uuid) + self.assertIsNotNone(vol1) + self.assertEqual(vol1.type, "btrfs volume") + self.assertEqual(vol1.name, "test") + self.assertEqual(vol1.parents[0].path, self.vdevs[0] + "1") + + vol2 = self.storage.devicetree.get_device_by_device_id("BTRFS-" + btrfs2_uuid) + self.assertIsNotNone(vol2) + self.assertEqual(vol2.type, "btrfs volume") + self.assertEqual(vol2.name, "test") + self.assertEqual(vol2.parents[0].path, self.vdevs[1] + "1") + + vg = self.storage.devicetree.get_device_by_device_id("LVM-test") + self.assertIsNotNone(vg) + self.assertEqual(vg.type, "lvmvg") + self.assertEqual(vg.name, "test") + self.assertEqual(vg.parents[0].path, self.vdevs[2] + "1") From 87520a1a36e4131b72b8c2216470a57fff33716e Mon Sep 17 00:00:00 2001 From: Vojtech Trefny Date: Wed, 6 Dec 2023 14:19:17 +0100 Subject: [PATCH 7/7] tests: Wait for array resync in MD tests --- tests/storage_tests/devices_test/md_test.py | 38 +++++++++++++++++---- 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/tests/storage_tests/devices_test/md_test.py b/tests/storage_tests/devices_test/md_test.py index 0fed916dc..9975d27aa 100644 --- a/tests/storage_tests/devices_test/md_test.py +++ b/tests/storage_tests/devices_test/md_test.py @@ -1,10 +1,28 @@ import os +import time + +from contextlib import contextmanager from ..storagetestcase import StorageTestCase import blivet +@contextmanager +def wait_for_resync(): + try: + yield + finally: + time.sleep(2) + action = True + while action: + with open("/proc/mdstat", "r") as f: + action = "resync" in f.read() + if action: + print("Sleeping") + time.sleep(1) + + class MDTestCase(StorageTestCase): raidname = "blivetTestRAID" @@ -57,7 +75,8 @@ def _test_mdraid(self, raid_level, members): member_devices=members) self.storage.create_device(array) - self.storage.do_it() + with wait_for_resync(): + self.storage.do_it() self.storage.reset() array = self.storage.devicetree.get_device_by_name(self.raidname) @@ -95,7 +114,8 @@ def test_mdraid_raid0_extra(self): metadata_version="1.2") self.storage.create_device(array) - self.storage.do_it() + with wait_for_resync(): + self.storage.do_it() self.storage.reset() array = self.storage.devicetree.get_device_by_name(self.raidname) @@ -111,7 +131,8 @@ def test_mdraid_raid1_spare(self): member_devices=2) self.storage.create_device(array) - self.storage.do_it() + with wait_for_resync(): + self.storage.do_it() self.storage.reset() array = self.storage.devicetree.get_device_by_name(self.raidname) @@ -133,7 +154,8 @@ def test_mdraid_members_add_remove(self): member_devices=2) self.storage.create_device(array) - self.storage.do_it() + with wait_for_resync(): + self.storage.do_it() self.storage.reset() array = self.storage.devicetree.get_device_by_name(self.raidname) @@ -158,7 +180,9 @@ def test_mdraid_members_add_remove(self): ac = blivet.deviceaction.ActionAddMember(array, part) self.storage.devicetree.actions.add(ac) - self.storage.do_it() + + with wait_for_resync(): + self.storage.do_it() array = self.storage.devicetree.get_device_by_name(self.raidname) self.assertIsNotNone(array) @@ -171,7 +195,9 @@ def test_mdraid_members_add_remove(self): # and now remove it from the array ac = blivet.deviceaction.ActionRemoveMember(array, part) self.storage.devicetree.actions.add(ac) - self.storage.do_it() + + with wait_for_resync(): + self.storage.do_it() array = self.storage.devicetree.get_device_by_name(self.raidname) self.assertIsNotNone(array)