[Kimchi-devel] [PATCH v3 1/2] logical pool fixes: only list leaf devices, and read file instead of run "cat"
Zhou Zheng Sheng
zhshzhou at linux.vnet.ibm.com
Fri Dec 27 10:26:59 UTC 2013
on 2013/12/26 23:43, Aline Manera wrote:
> On 12/23/2013 12:44 PM, Zhou Zheng Sheng wrote:
>> Some devices are parent of other devices. We should just list the leaf
>> devices but not parent devices. For example, if a disk contains
>> partitions, we should only list the partitions. If a disk is held by
>> multipath device, we should not list the disk.
>>
>> Currently we strip the last charactter from "vda1" to get "vda" and
>> ignore the parent "vda" device. This may fails on disk contains lots of
>> logical partitions, for example "vda10"'s parent is not "vda1". Another
>> problem is this method can not find the parent-child relation ship
>> between a multipath device and its slaves.
>>
>> The most accurate information is on sysfs, and lsblk lists all children
>> device of the requested device based on sysfs. So when "lsblk -P
>> devices" prints only one line, it's the device itself, when it prints
>> more lines, they are the children devices. This patch use this method to
>> detect if a device a leaf one.
>>
>> This patch also avoids start new "cat" process to read uevent file, it
>> opens the uevent file and read it directly.
>>
>> v2:
>> Assign string literal to constants thus avoid breaking the line.
>>
>> v3:
>> Save N lsblk invocations when list all devices by collecting major:min
>> and device node path in one run. N is number of block devices.
>> Use short-circuiting operator "and" instead of all to save extra lsblk
>> invocation.
>>
>> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>> ---
>> src/kimchi/disks.py | 83
>> ++++++++++++++++++++++++++++++++---------------------
>> 1 file changed, 50 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/kimchi/disks.py b/src/kimchi/disks.py
>> index a054961..34b9e01 100644
>> --- a/src/kimchi/disks.py
>> +++ b/src/kimchi/disks.py
>> @@ -27,27 +27,13 @@ from kimchi.exception import OperationFailed
>> from kimchi.utils import kimchi_log
>>
>>
>> -def _get_partition_path(name):
>> - """ Returns device path given a partition name """
>> - dev_path = None
>> - maj_min = None
>> -
>> - keys = ["NAME", "MAJ:MIN"]
>> - dev_list = _get_lsblk_devs(keys)
>> -
>> - for dev in dev_list:
>> - if dev['name'] == name:
>> - maj_min = dev['maj:min']
>> - break
>> +def _get_dev_node_path(maj_min):
>> + """ Returns device node path given the device number 'major:min' """
>> + uevent = "/sys/dev/block/%s/uevent" % maj_min
>> + with open(uevent) as ueventf:
>> + content = ueventf.read()
>>
>> - uevent_cmd = "cat /sys/dev/block/%s/uevent" % maj_min
>> - uevent = subprocess.Popen(uevent_cmd, stdout=subprocess.PIPE,
>> - stderr=subprocess.PIPE, shell=True)
>> - out, err = uevent.communicate()
>> - if uevent.returncode != 0:
>> - raise OperationFailed("Error getting partition path for %s",
>> name)
>> -
>> - data = dict(re.findall(r'(\S+)=(".*?"|\S+)', out.replace("\n", "
>> ")))
>> + data = dict(re.findall(r'(\S+)=(".*?"|\S+)',
>> content.replace("\n", " ")))
>>
>> return "/dev/%s" % data["DEVNAME"]
>>
>> @@ -63,6 +49,39 @@ def _get_lsblk_devs(keys, devs=[]):
>> return _parse_lsblk_output(out, keys)
>>
>>
>> +def _get_dev_major_min(name):
>> + maj_min = None
>> +
>> + keys = ["NAME", "MAJ:MIN"]
>> + dev_list = _get_lsblk_devs(keys)
>> +
>> + for dev in dev_list:
>> + if dev['name'] == name:
>> + maj_min = dev['maj:min']
>> + break
>> + else:
>> + msg = "Failed to find major and minor number for %s" % name
>> + raise OperationFailed(msg)
>> +
>> + return maj_min
>> +
>> +
>> +def _is_dev_leaf(devNodePath):
>> + try:
>> + # By default, lsblk prints a device information followed by
>> children
>> + # device information
>> + childrenCount = len(
>> + _get_lsblk_devs(["NAME"], [devNodePath])) - 1
>> + except OperationFailed as e:
>> + # lsblk is known to fail on multipath devices
>> + # Assume these devices contain children
>> + kimchi_log.error(
>> + "Error getting device info for %s: %s", devNodePath, e)
>> + return False
>> + else:
>
> As I commented before the 'else' isn't needed as it is the normal code flow
>
Oops. I actual tend to remove this 'else' in v3. It seems I overlook it.
My fault. Let me correct it in v4. Thanks!
>> + return childrenCount == 0
>> +
>> +
>> def _parse_lsblk_output(output, keys):
>> # output is on format key="value",
>> # where key can be NAME, TYPE, FSTYPE, SIZE, MOUNTPOINT, etc
>> @@ -82,32 +101,30 @@ def _parse_lsblk_output(output, keys):
>>
>> def get_partitions_names():
>> names = []
>> - ignore_names = []
>> - keys = ["NAME", "TYPE", "FSTYPE", "MOUNTPOINT"]
>> + keys = ["NAME", "TYPE", "FSTYPE", "MOUNTPOINT", "MAJ:MIN"]
>> # output is on format key="value",
>> # where key can be NAME, TYPE, FSTYPE, MOUNTPOINT
>> for dev in _get_lsblk_devs(keys):
>> # split()[0] to avoid the second part of the name, after the
>> # whiteline
>> name = dev['name'].split()[0]
>> - # Only list unmounted and unformated partition or disk.
>> - if not all([dev['type'] in ['part', 'disk'],
>> - dev['fstype'] == "",
>> - dev['mountpoint'] == ""]):
>> -
>> - # the whole disk must be ignored in it has at least one
>> - # mounted/formatted partition
>> - if dev['type'] == 'part':
>> - ignore_names.append(name[:-1])
>> + devNodePath = _get_dev_node_path(dev['maj:min'])
>> + # Only list unmounted and unformated and leaf and (partition
>> or disk)
>> + # leaf means a partition, a disk has no partition, or a disk
>> not held
>> + # by any multipath device.
>> + if not (dev['type'] in ['part', 'disk'] and
>> + dev['fstype'] == "" and
>> + dev['mountpoint'] == "" and
>> + _is_dev_leaf(devNodePath)):
>> continue
>>
>> names.append(name)
>>
>> - return list(set(names) - set(ignore_names))
>> + return names
>>
>>
>> def get_partition_details(name):
>> - dev_path = _get_partition_path(name)
>> + dev_path = _get_dev_node_path(_get_dev_major_min(name))
>>
>> keys = ["TYPE", "FSTYPE", "SIZE", "MOUNTPOINT"]
>> try:
>
--
Thanks and best regards!
Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou at linux.vnet.ibm.com
Telephone: 86-10-82454397
More information about the Kimchi-devel
mailing list