From 17957d9beb42a04b8f180ccb7ba07d43179a41d3 Mon Sep 17 00:00:00 2001 From: Rishabh Dave Date: Fri, 3 Jan 2020 15:44:04 +0530 Subject: [PATCH] ceph-volume: add helper methods to get only first LVM devs These convenience methods shortens following phrase to "lv = get_first_lv()" - lvs = get_lvs() if len(lvs) >= 1: lvs = lv[0] These methods do the same things as above phrase internall. Rewrite listing.py to use these new helper methods. Signed-off-by: Rishabh Dave --- src/ceph-volume/ceph_volume/api/lvm.py | 30 +++++++++++ .../ceph_volume/devices/lvm/listing.py | 50 +++++++------------ 2 files changed, 47 insertions(+), 33 deletions(-) diff --git a/src/ceph-volume/ceph_volume/api/lvm.py b/src/ceph-volume/ceph_volume/api/lvm.py index db8666d821a1a..f17a71d7da63f 100644 --- a/src/ceph-volume/ceph_volume/api/lvm.py +++ b/src/ceph-volume/ceph_volume/api/lvm.py @@ -1421,6 +1421,16 @@ def get_pvs(fields=PV_FIELDS, sep='";"', filters='', tags=None): pvs_report = _output_parser(stdout, fields) return [PVolume(**pv_report) for pv_report in pvs_report] +def get_first_pv(fields=PV_FIELDS, sep='";"', filters=None, tags=None): + """ + Wrapper of get_pv meant to be a convenience method to avoid the phrase:: + pvs = get_pvs() + if len(pvs) >= 1: + pv = pvs[0] + """ + pvs = get_pvs(fields=fields, sep=sep, filters=filters, tags=tags) + return pvs[0] if len(pvs) > 0 else [] + def get_vgs(fields=VG_FIELDS, sep='";"', filters='', tags=None): """ Return a list of VGs that are available on the system and match the @@ -1445,6 +1455,16 @@ def get_vgs(fields=VG_FIELDS, sep='";"', filters='', tags=None): vgs_report =_output_parser(stdout, fields) return [VolumeGroup(**vg_report) for vg_report in vgs_report] +def get_first_vg(fields=VG_FIELDS, sep='";"', filters=None, tags=None): + """ + Wrapper of get_vg meant to be a convenience method to avoid the phrase:: + vgs = get_vgs() + if len(vgs) >= 1: + vg = vgs[0] + """ + vgs = get_vgs(fields=fields, sep=sep, filters=filters, tags=tags) + return vgs[0] if len(vgs) > 0 else [] + def get_lvs(fields=LV_FIELDS, sep='";"', filters='', tags=None): """ Return a list of LVs that are available on the system and match the @@ -1468,3 +1488,13 @@ def get_lvs(fields=LV_FIELDS, sep='";"', filters='', tags=None): stdout, stderr, returncode = process.call(args, verbose_on_failure=False) lvs_report = _output_parser(stdout, fields) return [Volume(**lv_report) for lv_report in lvs_report] + +def get_first_lv(fields=LV_FIELDS, sep='";"', filters=None, tags=None): + """ + Wrapper of get_lv meant to be a convenience method to avoid the phrase:: + lvs = get_lvs() + if len(lvs) >= 1: + lv = lvs[0] + """ + lvs = get_lvs(fields=fields, sep=sep, filters=filters, tags=tags) + return lvs[0] if len(lvs) > 0 else [] diff --git a/src/ceph-volume/ceph_volume/devices/lvm/listing.py b/src/ceph-volume/ceph_volume/devices/lvm/listing.py index 1bb50c1d592a6..a3824423c35ec 100644 --- a/src/ceph-volume/ceph_volume/devices/lvm/listing.py +++ b/src/ceph-volume/ceph_volume/devices/lvm/listing.py @@ -180,21 +180,18 @@ def single_report(self, device, lvs=None): volume in the form of vg/lv or a device with an absolute path like /dev/sda1 or /dev/sda """ - if lvs is None: - lvs = api.Volumes() report = {} - lv = api.get_lv_from_argument(device) + lv = api.get_first_lv(filters={'lv_path': device}) # check if there was a pv created with the # name of device - pv = api.get_pv(pv_name=device) - if pv and not lv: - try: - lv = api.get_lv(vg_name=pv.vg_name) - except MultipleLVsError: - lvs.filter(vg_name=pv.vg_name) - return self.full_report(lvs=lvs) + if not lv: + pv = api.get_first_pv(filters={'pv_name': device}) + if pv: + return self.full_report(lvs=api.get_lv(filters={'vg_name': + pv.vg_name})) + # TODO: a call to full_report just might work. if lv: try: _id = lv.tags['ceph.osd_id'] @@ -206,28 +203,14 @@ def single_report(self, device, lvs=None): lv_report = lv.as_dict() lv_report['devices'] = self.match_devices(lv.lv_uuid) report[_id].append(lv_report) - else: - # this has to be a journal/wal/db device (not a logical volume) so try - # to find the PARTUUID that should be stored in the OSD logical - # volume - for device_type in ['journal', 'block', 'wal', 'db']: - device_tag_name = 'ceph.%s_device' % device_type - device_tag_uuid = 'ceph.%s_uuid' % device_type - associated_lv = lvs.get(lv_tags={device_tag_name: device}) - if associated_lv: - _id = associated_lv.tags['ceph.osd_id'] - uuid = associated_lv.tags[device_tag_uuid] - - report.setdefault(_id, []) - report[_id].append( - { - 'tags': {'PARTUUID': uuid}, - 'type': device_type, - 'path': device, - } - ) - return report + # this has to be a journal/wal/db device (not a logical volume) + # so try to find the PARTUUID that should be stored in the OSD + # logical volume + vg_name = os.path.basename(device) + lvs = api.get_lvs(filters={'vg_name': vg_name}) + if lvs: + return self.full_report(lvs=lvs) def full_report(self, lvs=None): """ @@ -235,7 +218,7 @@ def full_report(self, lvs=None): that have been previously prepared by Ceph """ if lvs is None: - lvs = api.Volumes() + lvs = api.get_lvs() report = {} for lv in lvs: @@ -257,7 +240,8 @@ def full_report(self, lvs=None): # bluestore will not have a journal, filestore will not have # a block/wal/db, so we must skip if not present continue - if not api.get_lv(lv_uuid=device_uuid, lvs=lvs): + if not api.get_lvs(filters={'lv_uuid': device_uuid}): + # if not api.get_lv(lv_uuid=device_uuid, lvs=lvs): # means we have a regular device, so query blkid disk_device = disk.get_device_from_partuuid(device_uuid) if disk_device: