[Kimchi-devel] [PATCH] Moved disks.py file from kimchi plugin to gingerbase plugin

Daniel Henrique Barboza dhbarboza82 at gmail.com
Mon Nov 23 13:11:34 UTC 2015


What's the status of this patch?

If there are pending features in Ginger that might depend on other 
functions of
disks.py I see no reason to not simply move the entire module to gingerbase.

Otherwise, for each feature being developed, we'll have to cherry-pick each
disks.py method and patch them to gingerbase beforehand.


Daniel

On 11/20/2015 06:14 AM, Pooja Kulkarni wrote:
> Hi Aline,
>
> Since the next features in ginger like disk partition management would 
> require most of the functions in the file, I felt it would be better 
> to move the entire disks.py file to gingerbase, to avoid code 
> duplication.
>
> On 11/19/2015 11:26 PM, Aline Manera wrote:
>>
>> Hi all,
>>
>> As per discussion in bug 
>> https://github.com/kimchi-project/ginger/issues/39, I recommend to 
>> only move the common code to gingerbase.
>> The common code as described by Chandra are only 2 functions:
>>
>> from wok.plugins.kimchi.disks import _get_dev_major_min
>> from wok.plugins.kimchi.disks import _get_dev_node_path
>>
>> All the other functions in kimchi/utils.py are only used by Kimchi so 
>> they keep there.
>>
>> Please, move only those 2 common functions: _get_dev_major_min() and 
>> _get_dev_node_path() to gingerbase/utils.py and update all the 
>> references to them accordingly.
>>
>> Regards,
>> Aline Manera
>>
>> On 19/11/2015 11:09, Suresh Babu Angadi wrote:
>>>
>>>
>>> On 11/19/2015 06:04 PM, Rodrigo Trujillo wrote:
>>>> Why is this change necessary ??
>>>> This is going to make Kimchi requires ginger-base when projects 
>>>> splits.
>>> Going by the community decision in mail thread  Re: [Kimchi-devel] 
>>> [RFC] The new ginger-basic plugin
>>> point 3 states : 3) Add ginger-basic plugin as a Kimchi dependency.
>>>
>>> As per my knowledge, gingerbase was formed since some of the host 
>>> functionality
>>> was part of kimchi, which should have also been in ginger.
>>> So I suppose it is okay to move disks functionality to gingerbase 
>>> since it is used by both kimchi and ginger.
>>>
>>>> I think that this was not expected.
>>>>
>>>> Rodrigo Trujillo
>>>>
>>>> On 11/19/2015 08:22 AM, pkulkark at linux.vnet.ibm.com wrote:
>>>>> From: Pooja Kulkarni <pkulkark at linux.vnet.ibm.com>
>>>>>
>>>>> Signed-off-by: Pooja Kulkarni <pkulkark at linux.vnet.ibm.com>
>>>>> ---
>>>>>   src/wok/plugins/gingerbase/disks.py  | 196 
>>>>> +++++++++++++++++++++++++++++++++++
>>>>>   src/wok/plugins/kimchi/model/host.py |   2 +-
>>>>>   2 files changed, 197 insertions(+), 1 deletion(-)
>>>>>   create mode 100644 src/wok/plugins/gingerbase/disks.py
>>>>>
>>>>> diff --git a/src/wok/plugins/gingerbase/disks.py 
>>>>> b/src/wok/plugins/gingerbase/disks.py
>>>>> new file mode 100644
>>>>> index 0000000..eb40e3a
>>>>> --- /dev/null
>>>>> +++ b/src/wok/plugins/gingerbase/disks.py
>>>>> @@ -0,0 +1,196 @@
>>>>> +#
>>>>> +# Project Kimchi
>>>>> +#
>>>>> +# Copyright IBM, Corp. 2013-2015
>>>>> +#
>>>>> +# This library is free software; you can redistribute it and/or
>>>>> +# modify it under the terms of the GNU Lesser General Public
>>>>> +# License as published by the Free Software Foundation; either
>>>>> +# version 2.1 of the License, or (at your option) any later version.
>>>>> +#
>>>>> +# This library is distributed in the hope that it will be useful,
>>>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>>>>> +# Lesser General Public License for more details.
>>>>> +#
>>>>> +# You should have received a copy of the GNU Lesser General Public
>>>>> +# License along with this library; if not, write to the Free 
>>>>> Software
>>>>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  
>>>>> 02110-1301 USA
>>>>> +
>>>>> +import os.path
>>>>> +import re
>>>>> +import subprocess
>>>>> +from parted import Device as PDevice
>>>>> +from parted import Disk as PDisk
>>>>> +
>>>>> +from wok.exception import OperationFailed
>>>>> +from wok.utils import wok_log
>>>>> +
>>>>> +
>>>>> +def _get_dev_node_path(maj_min):
>>>>> +    """ Returns device node path given the device number 
>>>>> 'major:min' """
>>>>> +
>>>>> +    dm_name = "/sys/dev/block/%s/dm/name" % maj_min
>>>>> +    if os.path.exists(dm_name):
>>>>> +        with open(dm_name) as dm_f:
>>>>> +            content = dm_f.read().rstrip('\n')
>>>>> +        return "/dev/mapper/" + content
>>>>> +
>>>>> +    uevent = "/sys/dev/block/%s/uevent" % maj_min
>>>>> +    with open(uevent) as ueventf:
>>>>> +        content = ueventf.read()
>>>>> +
>>>>> +    data = dict(re.findall(r'(\S+)=(".*?"|\S+)', 
>>>>> content.replace("\n", " ")))
>>>>> +
>>>>> +    return "/dev/%s" % data["DEVNAME"]
>>>>> +
>>>>> +
>>>>> +def _get_lsblk_devs(keys, devs=[]):
>>>>> +    lsblk = subprocess.Popen(
>>>>> +        ["lsblk", "-Pbo"] + [','.join(keys)] + devs,
>>>>> +        stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>>>>> +    out, err = lsblk.communicate()
>>>>> +    if lsblk.returncode != 0:
>>>>> +        raise OperationFailed("KCHDISKS0001E", {'err': err})
>>>>> +
>>>>> +    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'].split()[0] == name:
>>>>> +            maj_min = dev['maj:min']
>>>>> +            break
>>>>> +    else:
>>>>> +        raise OperationFailed("KCHDISKS0002E", {'device': name})
>>>>> +
>>>>> +    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
>>>>> +        wok_log.error(
>>>>> +            "Error getting device info for %s: %s", devNodePath, e)
>>>>> +        return False
>>>>> +
>>>>> +    return childrenCount == 0
>>>>> +
>>>>> +
>>>>> +def _is_dev_extended_partition(devType, devNodePath):
>>>>> +    if devType != 'part':
>>>>> +        return False
>>>>> +    diskPath = devNodePath.rstrip('0123456789')
>>>>> +    device = PDevice(diskPath)
>>>>> +    try:
>>>>> +        extended_part = PDisk(device).getExtendedPartition()
>>>>> +    except NotImplementedError as e:
>>>>> +        wok_log.warning(
>>>>> +            "Error getting extended partition info for dev %s 
>>>>> type %s: %s",
>>>>> +            devNodePath, devType, e.message)
>>>>> +        # Treate disk with unsupported partiton table as if it 
>>>>> does not
>>>>> +        # contain extended partitions.
>>>>> +        return False
>>>>> +    if extended_part and extended_part.path == devNodePath:
>>>>> +        return True
>>>>> +    return False
>>>>> +
>>>>> +
>>>>> +def _parse_lsblk_output(output, keys):
>>>>> +    # output is on format key="value",
>>>>> +    # where key can be NAME, TYPE, FSTYPE, SIZE, MOUNTPOINT, etc
>>>>> +    lines = output.rstrip("\n").split("\n")
>>>>> +    r = []
>>>>> +    for line in lines:
>>>>> +        d = {}
>>>>> +        for key in keys:
>>>>> +            expression = r"%s=\".*?\"" % key
>>>>> +            match = re.search(expression, line)
>>>>> +            field = match.group()
>>>>> +            k, v = field.split('=', 1)
>>>>> +            d[k.lower()] = v[1:-1]
>>>>> +        r.append(d)
>>>>> +    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 _is_available(name, devtype, fstype, mountpoint, majmin):
>>>>> +    devNodePath = _get_dev_node_path(majmin)
>>>>> +    # 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. Physical volume belongs to no 
>>>>> volume group
>>>>> +    # is also listed. Extended partitions should not be listed.
>>>>> +    if (devtype in ['part', 'disk', 'mpath'] and
>>>>> +            fstype in ['', 'LVM2_member'] and
>>>>> +            mountpoint == "" and
>>>>> +            _get_vgname(devNodePath) == "" and
>>>>> +            _is_dev_leaf(devNodePath) and
>>>>> +            not _is_dev_extended_partition(devtype, devNodePath)):
>>>>> +        return True
>>>>> +    return False
>>>>> +
>>>>> +
>>>>> +def get_partitions_names(check=False):
>>>>> +    names = set()
>>>>> +    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]
>>>>> +        if check and not _is_available(name, dev['type'], 
>>>>> dev['fstype'],
>>>>> +                                       dev['mountpoint'], 
>>>>> dev['maj:min']):
>>>>> +            continue
>>>>> +        names.add(name)
>>>>> +
>>>>> +    return list(names)
>>>>> +
>>>>> +
>>>>> +def get_partition_details(name):
>>>>> +    majmin = _get_dev_major_min(name)
>>>>> +    dev_path = _get_dev_node_path(majmin)
>>>>> +
>>>>> +    keys = ["TYPE", "FSTYPE", "SIZE", "MOUNTPOINT"]
>>>>> +    try:
>>>>> +        dev = _get_lsblk_devs(keys, [dev_path])[0]
>>>>> +    except OperationFailed as e:
>>>>> +        wok_log.error(
>>>>> +            "Error getting partition info for %s: %s", name, e)
>>>>> +        return {}
>>>>> +
>>>>> +    dev['available'] = _is_available(name, dev['type'], 
>>>>> dev['fstype'],
>>>>> +                                     dev['mountpoint'], majmin)
>>>>> +    if dev['mountpoint']:
>>>>> +        # Sometimes the mountpoint comes with [SWAP] or other
>>>>> +        # info which is not an actual mount point. Filtering it
>>>>> +        regexp = re.compile(r"\[.*\]")
>>>>> +        if regexp.search(dev['mountpoint']) is not None:
>>>>> +            dev['mountpoint'] = ''
>>>>> +    dev['path'] = dev_path
>>>>> +    dev['name'] = name
>>>>> +    return dev
>>>>> diff --git a/src/wok/plugins/kimchi/model/host.py 
>>>>> b/src/wok/plugins/kimchi/model/host.py
>>>>> index 96f4bea..4a1cff1 100644
>>>>> --- a/src/wok/plugins/kimchi/model/host.py
>>>>> +++ b/src/wok/plugins/kimchi/model/host.py
>>>>> @@ -24,7 +24,7 @@ from wok.exception import InvalidParameter
>>>>>   from wok.exception import NotFoundError
>>>>>   from wok.xmlutils.utils import xpath_get_text
>>>>>
>>>>> -from wok.plugins.kimchi import disks
>>>>> +from wok.plugins.gingerbase import disks
>>>>>   from wok.plugins.kimchi.model import hostdev
>>>>>   from wok.plugins.kimchi.model.config import CapabilitiesModel
>>>>>   from wok.plugins.kimchi.model.vms import VMModel, VMsModel
>>>>
>>>> _______________________________________________
>>>> Kimchi-devel mailing list
>>>> Kimchi-devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>>>
>>>
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel




More information about the Kimchi-devel mailing list