From b35e8c485f73b5b65e0be29444eb06172d6df3e3 Mon Sep 17 00:00:00 2001 From: Jan Fajerski Date: Fri, 8 Nov 2019 16:36:30 +0100 Subject: [PATCH] ceph-volume: refactor get_devices, don't use os.path.realpath Fixes: https://tracker.ceph.com/issues/42777 Signed-off-by: Jan Fajerski --- src/ceph-volume/ceph_volume/util/disk.py | 186 +++++++---------------- 1 file changed, 52 insertions(+), 134 deletions(-) diff --git a/src/ceph-volume/ceph_volume/util/disk.py b/src/ceph-volume/ceph_volume/util/disk.py index f81d23621e518..31ba1b850dee9 100644 --- a/src/ceph-volume/ceph_volume/util/disk.py +++ b/src/ceph-volume/ceph_volume/util/disk.py @@ -362,92 +362,6 @@ def is_partition(dev): return False -def _map_dev_paths(_path, include_abspath=False, include_realpath=False): - """ - Go through all the items in ``_path`` and map them to their absolute path:: - - {'sda': '/dev/sda'} - - If ``include_abspath`` is set, then a reverse mapping is set as well:: - - {'sda': '/dev/sda', '/dev/sda': 'sda'} - - If ``include_realpath`` is set then the same operation is done for any - links found when listing, these are *not* reversed to avoid clashing on - existing keys, but both abspath and basename can be included. For example:: - - { - 'ceph-data': '/dev/mapper/ceph-data', - '/dev/mapper/ceph-data': 'ceph-data', - '/dev/dm-0': '/dev/mapper/ceph-data', - 'dm-0': '/dev/mapper/ceph-data' - } - - - In case of possible exceptions the mapping is returned empty, and the - exception is logged. - """ - mapping = {} - try: - dev_names = os.listdir(_path) - except (OSError, IOError): - logger.exception('unable to list block devices from: %s' % _path) - return {} - - for dev_name in dev_names: - mapping[dev_name] = os.path.join(_path, dev_name) - - if include_abspath: - for k, v in list(mapping.items()): - mapping[v] = k - - if include_realpath: - for abspath in list(mapping.values()): - if not os.path.islink(abspath): - continue - - realpath = os.path.realpath(abspath) - basename = os.path.basename(realpath) - mapping[basename] = abspath - if include_abspath: - mapping[realpath] = abspath - - return mapping - - -def get_block_devs(sys_block_path="/sys/block", skip_loop=True): - """ - Go through all the items in /sys/block and return them as a list. - - The ``sys_block_path`` argument is set for easier testing and is not - required for proper operation. - """ - devices = _map_dev_paths(sys_block_path).keys() - if skip_loop: - return [d for d in devices if not d.startswith('loop')] - return list(devices) - - -def get_dev_devs(dev_path="/dev"): - """ - Go through all the items in /dev and return them as a list. - - The ``dev_path`` argument is set for easier testing and is not - required for proper operation. - """ - return _map_dev_paths(dev_path, include_abspath=True) - - -def get_mapper_devs(mapper_path="/dev/mapper"): - """ - Go through all the items in /dev and return them as a list. - - The ``dev_path`` argument is set for easier testing and is not - required for proper operation. - """ - return _map_dev_paths(mapper_path, include_abspath=True, include_realpath=True) - - class BaseFloatUnit(float): """ Base class to support float representations of size values. Suffix is @@ -762,65 +676,66 @@ def is_locked_raw_device(disk_path): return 0 -def get_devices(_sys_block_path='/sys/block', _dev_path='/dev', _mapper_path='/dev/mapper'): +def get_block_devs_lsblk(): + ''' + This returns a list of lists with 3 items per inner list. + KNAME - reflects the kernel device name , for example /dev/sda or /dev/dm-0 + NAME - the device name, for example /dev/sda or + /dev/mapper/- + TYPE - the block device type: disk, partition, lvm and such + + ''' + cmd = ['lsblk', '-plno', 'KNAME,NAME,TYPE'] + stdout, stderr, rc = process.call(cmd) + # lsblk returns 1 on failure + if rc == 1: + raise OSError('lsblk returned failure, stderr: {}'.format(stderr)) + return [re.split(r'\s+', line) for line in stdout] + + +def get_devices(_sys_block_path='/sys/block'): """ - Captures all available devices from /sys/block/, including its partitions, - along with interesting metadata like sectors, size, vendor, - solid/rotational, etc... + Captures all available block devices as reported by lsblk. + Additional interesting metadata like sectors, size, vendor, + solid/rotational, etc. is collected from /sys/block/ Returns a dictionary, where keys are the full paths to devices. - ..note:: dmapper devices get their path updated to what they link from, if - /dev/dm-0 is linked by /dev/mapper/ceph-data, then the latter gets - used as the key. - ..note:: loop devices, removable media, and logical volumes are never included. """ - # Portions of this detection process are inspired by some of the fact - # gathering done by Ansible in module_utils/facts/hardware/linux.py. The - # processing of metadata and final outcome *is very different* and fully - # imcompatible. There are ignored devices, and paths get resolved depending - # on dm devices, loop, and removable media device_facts = {} - block_devs = get_block_devs(_sys_block_path) - dev_devs = get_dev_devs(_dev_path) - mapper_devs = get_mapper_devs(_mapper_path) + block_devs = get_block_devs_lsblk() for block in block_devs: - sysdir = os.path.join(_sys_block_path, block) - metadata = {} - - # Ensure that the diskname is an absolute path and that it never points - # to a /dev/dm-* device - diskname = mapper_devs.get(block) or dev_devs.get(block) - if not diskname: + devname = os.path.basename(block[0]) + diskname = block[1] + if block[2] != 'disk': continue + sysdir = os.path.join(_sys_block_path, devname) + metadata = {} # If the mapper device is a logical volume it gets excluded if is_mapper_device(diskname): if lvm.is_lv(diskname): continue - metadata['removable'] = get_file_contents(os.path.join(sysdir, 'removable')) - # Is the device read-only ? - metadata['ro'] = get_file_contents(os.path.join(sysdir, 'ro')) - - - for key in ['vendor', 'model', 'rev', 'sas_address', 'sas_device_handle']: - metadata[key] = get_file_contents(sysdir + "/device/" + key) - - for key in ['sectors', 'size']: - metadata[key] = get_file_contents(os.path.join(sysdir, key), 0) - - for key, _file in [('support_discard', '/queue/discard_granularity')]: - metadata[key] = get_file_contents(os.path.join(sysdir, _file)) - - metadata['partitions'] = get_partitions_facts(sysdir) - - for key in ['rotational', 'nr_requests']: - metadata[key] = get_file_contents(sysdir + "/queue/" + key) + # all facts that have no defaults + # (, ) + facts = [('removable', 'removable'), + ('ro', 'ro'), + ('vendor', 'device/vendor'), + ('model', 'device/model'), + ('rev', 'device/rev'), + ('sas_address', 'device/sas_address'), + ('sas_device_handle', 'device/sas_device_handle'), + ('support_discard', 'queue/discard_granularity'), + ('rotational', 'queue/rotational'), + ('nr_requests', 'queue/nr_requests'), + ] + for key, file_ in facts: + metadata[key] = get_file_contents(os.path.join(sysdir, file_)) metadata['scheduler_mode'] = "" scheduler = get_file_contents(sysdir + "/queue/scheduler") @@ -829,14 +744,17 @@ def get_devices(_sys_block_path='/sys/block', _dev_path='/dev', _mapper_path='/d if m: metadata['scheduler_mode'] = m.group(2) - if not metadata['sectors']: - metadata['sectors'] = 0 - size = metadata['sectors'] or metadata['size'] - metadata['sectorsize'] = get_file_contents(sysdir + "/queue/logical_block_size") - if not metadata['sectorsize']: - metadata['sectorsize'] = get_file_contents(sysdir + "/queue/hw_sector_size", 512) - metadata['human_readable_size'] = human_readable_size(float(size) * 512) + metadata['partitions'] = get_partitions_facts(sysdir) + + size = get_file_contents(os.path.join(sysdir, 'size'), 0) + + metadata['sectors'] = get_file_contents(os.path.join(sysdir, 'sectors'), 0) + fallback_sectorsize = get_file_contents(sysdir + "/queue/hw_sector_size", 512) + metadata['sectorsize'] = get_file_contents(sysdir + + "/queue/logical_block_size", + fallback_sectorsize) metadata['size'] = float(size) * 512 + metadata['human_readable_size'] = human_readable_size(metadata['size']) metadata['path'] = diskname metadata['locked'] = is_locked_raw_device(metadata['path']) -- 2.39.5