[Kimchi-devel] [PATCH] Moved disks.py file from kimchi plugin to gingerbase plugin
Chandra Shekhar Reddy Potula
chandra at linux.vnet.ibm.com
Mon Nov 23 13:41:17 UTC 2015
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 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
>
> _______________________________________________
> 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