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:
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
On 11/19/2015 06:04 PM, Rodrigo Trujillo wrote: 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@linux.vnet.ibm.com wrote:
From: Pooja Kulkarni <pkulkark@linux.vnet.ibm.com>
Signed-off-by: Pooja Kulkarni <pkulkark@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel