[Kimchi-devel] [PATCH] Moved disks.py file from kimchi plugin to gingerbase plugin
Aline Manera
alinefm at linux.vnet.ibm.com
Mon Nov 23 20:17:04 UTC 2015
Okay about moving the whole file as more functions will be required for
Ginger Base soon.
My only comment is regarding the license header.
Update the project name to "Ginger Base" and add the line:
"# Code derived from Project Kimchi"
You can check the other files in Ginger Base as examples.
Regards,
Aline Manera
On 23/11/2015 11:41, Chandra Shekhar Reddy Potula wrote:
> 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
>>
>
> _______________________________________________
> 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