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

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 >> >