[Kimchi-devel] [PATCH v3 2/2] logical pool: list unused physical volume

[Sheldon]

Reviewed-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>

On 12/24/2013 01:54 PM, Zhou Zheng Sheng wrote:
> 于 2013年12月24日 13:25, Sheldon 写道:
>> Reviewed-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>> Test-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>>
>> Great, you can list a pv do not belongs to any VG.
>>
>> jiust a minor comments as follow.
>>
>> On 12/23/2013 10:44 PM, Zhou Zheng Sheng wrote:
>>> Physical volume belongs to no volume group can be considered a free
>>> block device to use.
>>>
>>> Implement a _get_vgname() function to get volume group name of a device
>>> node. If the requested block device is not a physical volume, it returns
>>> empty string, otherwise returns volume group name. Then it inspects the
>>> volume group name for all potential devices and filters out in use
>>> physical volumes.
>>>
>>> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>>> ---
>>>    src/kimchi/disks.py | 20 ++++++++++++++++++--
>>>    1 file changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/kimchi/disks.py b/src/kimchi/disks.py
>>> index 34b9e01..ceaa182 100644
>>> --- a/src/kimchi/disks.py
>>> +++ b/src/kimchi/disks.py
>>> @@ -99,6 +99,20 @@ def _parse_lsblk_output(output, keys):
>>>        return r
>>>
>>>
>>> +def _get_vgname(devNodePath):
>>> +    """ Return volume group name of a physical volume. If the device
>>> node path
>>> +    is not a physical volume, return empty string. """
>>> +    pvs = subprocess.Popen(
>>> +        ["pvs", "--unbuffered", "--nameprefixes", "--noheadings",
>>> +         "-o", "vg_name", devNodePath],
>>> +        stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>>> +    out, err = pvs.communicate()
>>> +    if pvs.returncode != 0:
>>> +        return ""
>>> +
>>> +    return re.findall(r"LVM2_VG_NAME='([^\']*)'", out)[0]
>>> +
>>> +
>>>    def get_partitions_names():
>>>        names = []
>>>        keys = ["NAME", "TYPE", "FSTYPE", "MOUNTPOINT", "MAJ:MIN"]
>>> @@ -111,10 +125,12 @@ def get_partitions_names():
>>>            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.
>>> +        # by any multipath device. Physical volume belongs to no
>>> volume group
>>> +        # is also listed.
>>>            if not (dev['type'] in ['part', 'disk'] and
>>> -                dev['fstype'] == "" and
>>> +                dev['fstype'] in ['', 'LVM2_member'] and
>> I have see a strange fstype on meina's host.
>> so
>> How can a constant at the head of this file.:
>>
>> EXCLUDE_FSTYEP = ['', 'LVM2_member']
>>
>> we can easily to add more fstype that not need to be listed.
>>
>> and here:
>>
>> +                dev['fstype'] in EXCLUDE_FSTYEP and
>>
>>
>>
> Wise idea, but you may misunderstand this list. fstype in ['',
> 'LVM2_member'] is not a exclude list, it's a white list. So unknown file
> system does not matter.
sorry.
got it. it is not exclude list.
”LVM2_member“ is used to find a pv. :-)
>>>                    dev['mountpoint'] == "" and
>>> +                _get_vgname(devNodePath) == "" and
>>>                    _is_dev_leaf(devNodePath)):
>>>                continue
>>>
>>
>


-- 
Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center