From 01112ba6894834287092d3b672ec548cb3a4d676 Mon Sep 17 00:00:00 2001 From: Guillaume Abrioux Date: Fri, 8 Jul 2022 13:47:00 +0200 Subject: [PATCH] ceph-volume: drop self.abspath in Device() seems useless to have both self.path and self.abspath attributes. Signed-off-by: Guillaume Abrioux --- .../ceph_volume/devices/lvm/zap.py | 28 +++++++++---------- .../tests/devices/lvm/test_migrate.py | 12 ++++---- .../ceph_volume/tests/devices/lvm/test_zap.py | 6 ++-- .../tests/util/test_arg_validators.py | 2 +- .../ceph_volume/tests/util/test_device.py | 2 +- src/ceph-volume/ceph_volume/util/device.py | 28 +++++++++---------- src/ceph-volume/ceph_volume/util/disk.py | 4 +-- .../ceph_volume/util/encryption.py | 2 +- 8 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/ceph-volume/ceph_volume/devices/lvm/zap.py b/src/ceph-volume/ceph_volume/devices/lvm/zap.py index 9f8141d5406ce..d6d778d165efe 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/zap.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/zap.py @@ -171,8 +171,8 @@ class Zap(object): pv = api.get_single_pv(filters={'lv_uuid': lv.lv_uuid}) self.unmount_lv(lv) - wipefs(device.abspath) - zap_data(device.abspath) + wipefs(device.path) + zap_data(device.path) if self.args.destroy: lvs = api.get_lvs(filters={'vg_name': device.vg_name}) @@ -188,8 +188,8 @@ class Zap(object): mlogger.info('More than 1 LV left in VG, will proceed to ' 'destroy LV only') mlogger.info('Removing LV because --destroy was given: %s', - device.abspath) - api.remove_lv(device.abspath) + device.path) + api.remove_lv(device.path) elif lv: # just remove all lvm metadata, leaving the LV around lv.clear_tags() @@ -209,15 +209,15 @@ class Zap(object): if os.path.realpath(mapper_path) in holders: self.dmcrypt_close(mapper_uuid) - if system.device_is_mounted(device.abspath): - mlogger.info("Unmounting %s", device.abspath) - system.unmount(device.abspath) + if system.device_is_mounted(device.path): + mlogger.info("Unmounting %s", device.path) + system.unmount(device.path) - wipefs(device.abspath) - zap_data(device.abspath) + wipefs(device.path) + zap_data(device.path) if self.args.destroy: - mlogger.info("Destroying partition since --destroy was used: %s" % device.abspath) + mlogger.info("Destroying partition since --destroy was used: %s" % device.path) disk.remove_partition(device) def zap_lvm_member(self, device): @@ -230,7 +230,7 @@ class Zap(object): """ for lv in device.lvs: if lv.lv_name: - mlogger.info('Zapping lvm member {}. lv_path is {}'.format(device.abspath, lv.lv_path)) + mlogger.info('Zapping lvm member {}. lv_path is {}'.format(device.path, lv.lv_path)) self.zap_lv(Device(lv.lv_path)) else: vg = api.get_single_vg(filters={'vg_name': lv.vg_name}) @@ -259,15 +259,15 @@ class Zap(object): for part_name in device.sys_api.get('partitions', {}).keys(): self.zap_partition(Device('/dev/%s' % part_name)) - wipefs(device.abspath) - zap_data(device.abspath) + wipefs(device.path) + zap_data(device.path) @decorators.needs_root def zap(self, devices=None): devices = devices or self.args.devices for device in devices: - mlogger.info("Zapping: %s", device.abspath) + mlogger.info("Zapping: %s", device.path) if device.is_mapper and not device.is_mpath: terminal.error("Refusing to zap the mapper device: {}".format(device)) raise SystemExit(1) diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py index 1faaee05e6dda..4c86d0ca1c474 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_migrate.py @@ -69,7 +69,7 @@ class TestFindAssociatedDevices(object): result = migrate.find_associated_devices(osd_id='0', osd_fsid='1234') assert len(result) == 1 - assert result[0][0].abspath == '/dev/VolGroup/lv1' + assert result[0][0].path == '/dev/VolGroup/lv1' assert result[0][0].lvs == [vol] assert result[0][1] == 'block' @@ -96,10 +96,10 @@ class TestFindAssociatedDevices(object): assert len(result) == 2 for d in result: if d[1] == 'block': - assert d[0].abspath == '/dev/VolGroup/lv1' + assert d[0].path == '/dev/VolGroup/lv1' assert d[0].lvs == [vol] elif d[1] == 'wal': - assert d[0].abspath == '/dev/VolGroup/lv2' + assert d[0].path == '/dev/VolGroup/lv2' assert d[0].lvs == [vol2] else: assert False @@ -133,13 +133,13 @@ class TestFindAssociatedDevices(object): assert len(result) == 3 for d in result: if d[1] == 'block': - assert d[0].abspath == '/dev/VolGroup/lv1' + assert d[0].path == '/dev/VolGroup/lv1' assert d[0].lvs == [vol] elif d[1] == 'wal': - assert d[0].abspath == '/dev/VolGroup/lv2' + assert d[0].path == '/dev/VolGroup/lv2' assert d[0].lvs == [vol2] elif d[1] == 'db': - assert d[0].abspath == '/dev/VolGroup/lv3' + assert d[0].path == '/dev/VolGroup/lv3' assert d[0].lvs == [vol3] else: assert False diff --git a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py index 53c69463317a6..64016111c2631 100644 --- a/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py +++ b/src/ceph-volume/ceph_volume/tests/devices/lvm/test_zap.py @@ -69,7 +69,7 @@ class TestFindAssociatedDevices(object): monkeypatch.setattr(process, 'call', lambda x, **kw: ('', '', 0)) result = zap.find_associated_devices(osd_id='0') - assert result[0].abspath == '/dev/VolGroup/lv' + assert result[0].path == '/dev/VolGroup/lv' def test_lv_is_matched_fsid(self, monkeypatch): tags = 'ceph.osd_id=0,ceph.osd_fsid=asdf-lkjh,ceph.journal_uuid=x,' +\ @@ -82,7 +82,7 @@ class TestFindAssociatedDevices(object): monkeypatch.setattr(process, 'call', lambda x, **kw: ('', '', 0)) result = zap.find_associated_devices(osd_fsid='asdf-lkjh') - assert result[0].abspath == '/dev/VolGroup/lv' + assert result[0].path == '/dev/VolGroup/lv' def test_lv_is_matched_id_fsid(self, monkeypatch): tags = 'ceph.osd_id=0,ceph.osd_fsid=asdf-lkjh,ceph.journal_uuid=x,' +\ @@ -95,7 +95,7 @@ class TestFindAssociatedDevices(object): monkeypatch.setattr(process, 'call', lambda x, **kw: ('', '', 0)) result = zap.find_associated_devices(osd_id='0', osd_fsid='asdf-lkjh') - assert result[0].abspath == '/dev/VolGroup/lv' + assert result[0].path == '/dev/VolGroup/lv' class TestEnsureAssociatedLVs(object): diff --git a/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py b/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py index 85c4bebc049a7..7e0abd1c3c77b 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_arg_validators.py @@ -88,7 +88,7 @@ class TestValidDevice(object): lsblk = {"TYPE": "disk", "NAME": "sda"} device_info(lsblk=lsblk) result = self.validator('/dev/sda') - assert result.abspath == '/dev/sda' + assert result.path == '/dev/sda' @patch('ceph_volume.util.arg_validators.disk.has_bluestore_label', return_value=False) def test_path_is_invalid(self, m_has_bs_label, diff --git a/src/ceph-volume/ceph_volume/tests/util/test_device.py b/src/ceph-volume/ceph_volume/tests/util/test_device.py index 6e49e41ffbc50..9bca71b275478 100644 --- a/src/ceph-volume/ceph_volume/tests/util/test_device.py +++ b/src/ceph-volume/ceph_volume/tests/util/test_device.py @@ -420,7 +420,7 @@ class TestDevice(object): # low-level behavior of has_bluestore_label with patch.object(device.Device, "__init__", lambda self, path, with_lsm=False: None): disk = device.Device("/dev/sda") - disk.abspath = "/dev/sda" + disk.path = "/dev/sda" with patch('builtins.open', mock_open(read_data=b'bluestore block device\n')): assert disk.has_bluestore_label with patch('builtins.open', mock_open(read_data=b'not a bluestore block device\n')): diff --git a/src/ceph-volume/ceph_volume/util/device.py b/src/ceph-volume/ceph_volume/util/device.py index a68377169ff93..0edc3bd7190c9 100644 --- a/src/ceph-volume/ceph_volume/util/device.py +++ b/src/ceph-volume/ceph_volume/util/device.py @@ -99,11 +99,9 @@ class Device(object): def __init__(self, path, with_lsm=False, lvs=None, lsblk_all=None, all_devices_vgs=None): self.path = path - # LVs can have a vg/lv path, while disks will have /dev/sda - self.abspath = path if not sys_info.devices: sys_info.devices = disk.get_devices() - self.sys_api = sys_info.devices.get(self.abspath, {}) + self.sys_api = sys_info.devices.get(self.path, {}) self.partitions = self._get_partitions() self.lv_api = None self.lvs = [] if not lvs else lvs @@ -169,7 +167,7 @@ class Device(object): lv = None if not self.sys_api: # if no device was found check if we are a partition - partname = self.abspath.split('/')[-1] + partname = self.path.split('/')[-1] for device, info in sys_info.devices.items(): part = info['partitions'].get(partname, {}) if part: @@ -200,7 +198,7 @@ class Device(object): if lv: self.lv_api = lv self.lvs = [lv] - self.abspath = lv.lv_path + self.path = lv.lv_path self.vg_name = lv.vg_name self.lv_name = lv.name self.ceph_device = lvm.is_ceph_device(lv) @@ -231,7 +229,7 @@ class Device(object): prefix = 'Partition' elif self.is_device: prefix = 'Raw Device' - return '<%s: %s>' % (prefix, self.abspath) + return '<%s: %s>' % (prefix, self.path) def pretty_report(self): def format_value(v): @@ -263,7 +261,7 @@ class Device(object): def report(self): return report_template.format( - dev=self.abspath, + dev=self.path, size=self.size_human, rot=self.rotational, available=self.available, @@ -283,7 +281,7 @@ class Device(object): """ props = ['ID_VENDOR', 'ID_MODEL', 'ID_MODEL_ENC', 'ID_SERIAL_SHORT', 'ID_SERIAL', 'ID_SCSI_SERIAL'] - p = disk.udevadm_property(self.abspath, props) + p = disk.udevadm_property(self.path, props) if p.get('ID_MODEL','').startswith('LVM PV '): p['ID_MODEL'] = p.get('ID_MODEL_ENC', '').replace('\\x20', ' ').strip() if 'ID_VENDOR' in p and 'ID_MODEL' in p and 'ID_SCSI_SERIAL' in p: @@ -312,7 +310,7 @@ class Device(object): # VGs, should we consider it as part of LVM? We choose not to # here, because most likely, we need to use VGs from this PV. self._is_lvm_member = False - device_to_check = [self.abspath] + device_to_check = [self.path] device_to_check.extend(self.partitions) # a pv can only be in one vg, so this should be safe @@ -338,14 +336,14 @@ class Device(object): partition. Return a list of paths to be checked for a pv. """ partitions = [] - path_dir = os.path.dirname(self.abspath) + path_dir = os.path.dirname(self.path) for partition in self.sys_api.get('partitions', {}).keys(): partitions.append(os.path.join(path_dir, partition)) return partitions @property def exists(self): - return os.path.exists(self.abspath) + return os.path.exists(self.path) @property def has_fs(self): @@ -422,7 +420,7 @@ class Device(object): @property def has_bluestore_label(self): - return disk.has_bluestore_label(self.abspath) + return disk.has_bluestore_label(self.path) @property def is_mapper(self): @@ -493,7 +491,7 @@ class Device(object): elif self.is_partition: return 'crypto_LUKS' in crypt_reports elif self.is_mapper: - active_mapper = encryption_status(self.abspath) + active_mapper = encryption_status(self.path) if active_mapper: # normalize a bit to ensure same values regardless of source encryption_type = active_mapper['type'].lower().strip('12') # turn LUKS1 or LUKS2 into luks @@ -578,7 +576,7 @@ class Device(object): except OSError as e: # likely failed to open the device. assuming it is BlueStore is the safest option # so that a possibly-already-existing OSD doesn't get overwritten - logger.error('failed to determine if device {} is BlueStore. device should not be used to avoid false negatives. err: {}'.format(self.abspath, e)) + logger.error('failed to determine if device {} is BlueStore. device should not be used to avoid false negatives. err: {}'.format(self.path, e)) rejected.append('Failed to determine if device is BlueStore') if self.is_partition: @@ -588,7 +586,7 @@ class Device(object): except OSError as e: # likely failed to open the device. assuming the parent is BlueStore is the safest # option so that a possibly-already-existing OSD doesn't get overwritten - logger.error('failed to determine if partition {} (parent: {}) has a BlueStore parent. partition should not be used to avoid false negatives. err: {}'.format(self.abspath, self.parent_device, e)) + logger.error('failed to determine if partition {} (parent: {}) has a BlueStore parent. partition should not be used to avoid false negatives. err: {}'.format(self.path, self.parent_device, e)) rejected.append('Failed to determine if parent device is BlueStore') if self.has_gpt_headers: diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py index 88c43b429aa3e..0a02fbc781363 100644 --- a/src/ceph-volume/ceph_volume/util/disk.py +++ b/src/ceph-volume/ceph_volume/util/disk.py @@ -134,10 +134,10 @@ def remove_partition(device): :param device: A ``Device()`` object """ - udev_info = udevadm_property(device.abspath) + udev_info = udevadm_property(device.path) partition_number = udev_info.get('ID_PART_ENTRY_NUMBER') if not partition_number: - raise RuntimeError('Unable to detect the partition number for device: %s' % device.abspath) + raise RuntimeError('Unable to detect the partition number for device: %s' % device.path) process.run( ['parted', device.parent_device, '--script', '--', 'rm', partition_number] diff --git a/src/ceph-volume/ceph_volume/util/encryption.py b/src/ceph-volume/ceph_volume/util/encryption.py index 4ad8d16b35c37..b23c0f79b3d3b 100644 --- a/src/ceph-volume/ceph_volume/util/encryption.py +++ b/src/ceph-volume/ceph_volume/util/encryption.py @@ -273,6 +273,6 @@ def legacy_encrypted(device): devices = [Device(i['NAME']) for i in device_family(parent_device)] for d in devices: if d.ceph_disk.type == 'lockbox': - metadata['lockbox'] = d.abspath + metadata['lockbox'] = d.path break return metadata -- 2.39.5