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

I would suggest to move the entire file instead individual parts. Since there are new patches lined up use other methods of the disks.py Chandra On 23/11/15 6:41 PM, Daniel Henrique Barboza wrote: > 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 > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel