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

[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



>                   dev['mountpoint'] == "" and
> +                _get_vgname(devNodePath) == "" and
>                   _is_dev_leaf(devNodePath)):
>               continue
>


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