[Kimchi-devel] [project-kimchi] [PATCH v1 2/3] Issue #276: logical pool: a quick fix for the device listing rules, back-end
Zhou Zheng Sheng
zhshzhou at linux.vnet.ibm.com
Wed Dec 18 10:20:50 UTC 2013
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.
2. Devices not listed by "blkid -o device"
Some devices are not listed by "blkid -o device", but actually they
can be used.
For example, a disk without patition table.
Disk with partitions are not listed by blkid but the partitions are
listed, this is OK. However, a disk with partition table but with
no patitions is not listed in "blkid -o device", while it can be used
actually.
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>
---
src/kimchi/disks.py | 168 ++++++++++++++++++++++++++++------------------------
1 file changed, 92 insertions(+), 76 deletions(-)
diff --git a/src/kimchi/disks.py b/src/kimchi/disks.py
index 874f669..7c7798a 100644
--- a/src/kimchi/disks.py
+++ b/src/kimchi/disks.py
@@ -23,22 +23,25 @@
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.")
+def _get_partitions_paths():
+ """ Returns all available partitions path of the host """
+ blkid = subprocess.Popen(
+ ["blkid", "-o", "device"],
+ stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+ out, err = blkid.communicate()
+ if blkid.returncode != 0:
+ raise OperationFailed('Error executing blkid: %s' % err)
+
+ dev_paths = out.rstrip("\n").split("\n")
+
return dev_paths
-def get_partition_path(name, dev_paths):
+def _get_partition_path(name, dev_paths):
""" Returns device path given a partition name """
dev_path = None
regexp = re.compile(r"^.*"+name+"$")
@@ -46,79 +49,92 @@ def get_partition_path(name, dev_paths):
if regexp.search(path) is not None:
dev_path = path
break
- if dev_path:
- return dev_path
+ return dev_path
-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.")
+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 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 _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():
+ # Will be used later to double check the partition
+ dev_paths = _get_partitions_paths()
+ 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]
+ # 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.
+ path = _get_partition_path(name, dev_paths)
+
+ # Only list unmounted and unformated partition or disk.
+ if not all([dev['type'] in ['part', 'disk'],
+ dev['fstype'] == "",
+ dev['mountpoint'] == "",
+ path is not None]):
+ continue
+
+ names.append(name)
+ return names
+
+
+def get_partition_details(name):
+ # Find device path
+ try:
+ dev_paths = _get_partitions_paths()
+ except OperationFailed as e:
+ kimchi_log.error(
+ "Error getting partition path for %s: %s", (name, e))
+ return {}
+
+ dev_path = _get_partition_path(name, dev_paths)
+ if dev_path is None:
+ return {}
+
+ 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))
+ 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
--
1.7.11.7
--
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.
More information about the Kimchi-devel
mailing list