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

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(a)linux.vnet.ibm.com wrote: >>>>> From: Pooja Kulkarni <pkulkark(a)linux.vnet.ibm.com&gt; >>>>> >>>>> Signed-off-by: Pooja Kulkarni <pkulkark(a)linux.vnet.ibm.com&gt; >>>>> --- >>>>> 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(a)ovirt.org >>>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >>>> >>> >> > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel