[Kimchi-devel] [project-kimchi] [PATCH 2/3] Issue #276: logical pool: a quick fix for the device listing rules, back-end
Sheldon
shaohef at linux.vnet.ibm.com
Thu Dec 19 06:41:48 UTC 2013
On 12/19/2013 01:22 AM, Aline Manera wrote:
> From: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
zhengsheng, it can not work well for mpath.
I have comment on issue 276
https://github.com/kimchi-project/kimchi/issues/276
>
> The current logical pool implementation has some problems. It lists ext4
> patitions but in fact partitions with FS should not be listed, because
> a dual boot system setup can make use of those unmounted partitions. It
> also fails to run lsblk for some devices.
>
> According to the discussion on github issue page[1]. We decide to list
> only unused partitions and disks, which means, PV and LV will not be
> listed, mounted partitions and partition with FS will not be listed.
>
> When lsblk fails to run on a purticular device, it swallows the error
> and return an empty dict.
>
> Some kinds of free block device still can not be listed by the current
> quick fix, for this is only a *quick* fix. The following cases are not
> covered, they should be improved in future patches.
> 1. PV that doesn't belong to any VG.
> blkid and lsblk do not provide enough information to detect if a PV
> belongs to an VG.
>
> To get the device path, it uses sysfs uevent file to get the device name and
> then assume the device path is /dev/<devname>
>
> This patch also drops the defensive coding style of the current
> implementation. Defensive coding is not Pythonic. It makes the code
> harder to debug, delay the discovery of code error and makes the main
> logic unclear. This patch also extract a function to run and parse
> lsblk to avoid repeating similar code.
>
> For a long term solution, backend should just list all block devices,
> along with information describing if they are in use. The front end
> provides a checkbox "filter in use devices" and let users decide what
> they want.
>
> [1] https://github.com/kimchi-project/kimchi/issues/276
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
> Signed-off-by: Aline Manera <alinefm at br.ibm.com>
>
> fix
> ---
> src/kimchi/disks.py | 170 ++++++++++++++++++++++++++-------------------------
> 1 file changed, 87 insertions(+), 83 deletions(-)
>
> diff --git a/src/kimchi/disks.py b/src/kimchi/disks.py
> index 874f669..0ae715c 100644
> --- a/src/kimchi/disks.py
> +++ b/src/kimchi/disks.py
> @@ -23,102 +23,106 @@
> import re
> import subprocess
>
> +from kimchi.utils import kimchi_log
> from kimchi.exception import OperationFailed
>
>
> -def get_partitions_paths():
> - try:
> - """ Returns all available partitions path of the host """
> - blkid = subprocess.Popen(
> - ["blkid", "-o", "device"],
> - stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> - dev_paths = blkid.communicate()[0].rstrip("\n").split("\n")
> - except:
> - raise OperationFailed("Unable to retrieve partitions' full path.")
> - return dev_paths
> -
> -
> -def get_partition_path(name, dev_paths):
> +def _get_partition_path(name):
> """ Returns device path given a partition name """
> dev_path = None
> - regexp = re.compile(r"^.*"+name+"$")
> - for path in dev_paths:
> - if regexp.search(path) is not None:
> - dev_path = path
> + maj_min = None
> +
> + keys = ["NAME", "MAJ:MIN"]
> + dev_list = _get_lsblk_devs(keys)
> +
> + for dev in dev_list:
> + if dev['name'] == name:
> + maj_min = dev['maj:min']
> break
> - if dev_path:
> - return dev_path
>
> + uevent_cmd = "cat /sys/dev/block/%s/uevent" % maj_min
> + uevent = subprocess.Popen(uevent_cmd, stdout=subprocess.PIPE,
> + stderr=subprocess.PIPE, shell=True)
> + out, err = uevent.communicate()
> + if uevent.returncode != 0:
> + raise OperationFailed("Error getting partition path for %s", name)
+with open("/sys/dev/block/%s/uevent" % maj_min) as f:
+ out = f.read()
>
> -def get_partitions_names():
> - try:
> - """ Returns all the names of available partitions """
> - lsblk = subprocess.Popen(
> - ["lsblk", "-Pbo", "NAME,TYPE"],
> - stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> - output = lsblk.communicate()[0]
> - lines_output = output.rstrip("\n").split("\n")
> - # Will be used later to double check the partition
> - dev_paths = get_partitions_paths()
> - names = []
> - keys = ["NAME", "TYPE"]
> - # output is on format key="value", where key = NAME, TYPE
> - for line in lines_output:
> - 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]
> - if d['type'] not in ['part', 'lvm']:
> - continue
> - # split()[0] to avoid the second part of the name,
> - # after the whiteline
> - name = d['name'].split()[0]
> - # There are special cases where lsblk reports
> - # a partition that does not exist in blkid and fdisk (Extended
> - # partitions), which can't be used for pool creation. We
> - # need to filter these cases as well.
> - if not get_partition_path(name, dev_paths):
> - continue
> - names.append(name)
> - return names
> - except:
> - raise OperationFailed("Unable to retrieve partitions' full path.")
> + data = dict(re.findall(r'(\S+)=(".*?"|\S+)', out.replace("\n", " ")))
>
> + return "/dev/%s" % data["DEVNAME"]
>
> -def get_partition_details(name):
> - try:
> - # Find device path
> - dev_path = get_partition_path(name, get_partitions_paths())
> - # Couldn't find dev_path.
> - if not dev_path:
> - return
> - # Executing lsblk to get partition/disk info
> - lsblk = subprocess.Popen(
> - ["lsblk", "-Pbo", "TYPE,FSTYPE,SIZE,MOUNTPOINT", dev_path],
> - stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> - # single line output
> - output = lsblk.communicate()[0].rstrip("\n")
> - # output is on format key="value", where key = NAME, TYPE, FSTYPE,
> - # SIZE, MOUNTPOINT
> - keys = ["TYPE", "FSTYPE", "SIZE", "MOUNTPOINT"]
> +
> +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('Error executing lsblk: %s' % err)
> +
> + return _parse_lsblk_output(out, keys)
> +
> +
> +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, output)
> + match = re.search(expression, line)
> field = match.group()
> k, v = field.split('=', 1)
> d[k.lower()] = v[1:-1]
> - if d['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(d['mountpoint']) is not None:
> - d['mountpoint'] = ''
> - d['path'] = dev_path
> - d['name'] = name
> - return d
> - except:
> - raise OperationFailed("Unable to retrieve partitions' data.")
> + r.append(d)
> + return r
> +
> +
> +def get_partitions_names():
> + names = []
> + ignore_names = []
> + keys = ["NAME", "TYPE", "FSTYPE", "MOUNTPOINT"]
> + # 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]
> + # Only list unmounted and unformated partition or disk.
> + if not all([dev['type'] in ['part', 'disk'],
> + dev['fstype'] == "",
> + dev['mountpoint'] == ""]):
> +
> + # the whole disk must be ignored in it has at least one
> + # mounted/formatted partition
> + if dev['type'] == 'part':
> + ignore_names.append(name[:-1])
it is OK, if partitions less 10
but what about: sda10?
"sda10"[:-1].
> + continue
> +
> + names.append(name)
> +
> + return list(set(names) - set(ignore_names))
> +
> +
> +def get_partition_details(name):
> + dev_path = _get_partition_path(name)
> +
> + keys = ["TYPE", "FSTYPE", "SIZE", "MOUNTPOINT"]
> + try:
> + dev = _get_lsblk_devs(keys, [dev_path])[0]
> + except OperationFailed as e:
> + kimchi_log.error(
> + "Error getting partition info for %s: %s", (name, e))
should be:
+ kimchi_log.error(
+ "Error getting partition info for %s: %s", name, e)
> + return {}
> +
> + 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
Sheldon Feng(???)
IBM Linux Technology Center
--
project-kimchi mailing list <project-kimchi at googlegroups.com>
https://groups.google.com/forum/#!forum/project-kimchi
---
You received this message because you are subscribed to the Google Groups "project-kimchi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-kimchi+unsubscribe at googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ovirt.org/pipermail/kimchi-devel/attachments/20131219/8d16d3e9/attachment.html>
More information about the Kimchi-devel
mailing list