[PATCH v1 1/3] PEP 8: cleanup src/kimchi/disks.py

22 views
Skip to first unread message

Zhou Zheng Sheng

unread,
Dec 18, 2013, 5:20:49 AM12/18/13
to project...@googlegroups.com, zhou...@linux.vnet.ibm.com, shu...@linux.vnet.ibm.com, dani...@linux.vnet.ibm.com, sha...@linux.vnet.ibm.com, ali...@br.ibm.com, Zhou Zheng Sheng
Make this file PEP 8 clean

Signed-off-by: Zhou Zheng Sheng <zhsh...@linux.vnet.ibm.com>
---
Makefile.am | 1 +
src/kimchi/disks.py | 37 ++++++++++++++++++++-----------------
2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index b79919c..e57d3b6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -40,6 +40,7 @@ EXTRA_DIST = \
PEP8_WHITELIST = \
src/kimchi/asynctask.py \
src/kimchi/config.py.in \
+ src/kimchi/disks.py \
src/kimchi/server.py \
plugins/__init__.py \
plugins/sample/__init__.py \
diff --git a/src/kimchi/disks.py b/src/kimchi/disks.py
index 93d5dbc..874f669 100644
--- a/src/kimchi/disks.py
+++ b/src/kimchi/disks.py
@@ -18,26 +18,26 @@
#
# 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
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

-import collections
-import os
import re
import subprocess
-import sys

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)
+ 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):
""" Returns device path given a partition name """
dev_path = None
@@ -49,11 +49,13 @@ def get_partition_path(name, dev_paths):
if 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)
+ 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
@@ -67,11 +69,12 @@ def get_partitions_names():
expression = r"%s=\".*?\"" % key
match = re.search(expression, line)
field = match.group()
- k, v = field.split('=',1)
+ k, v = field.split('=', 1)
d[k.lower()] = v[1:-1]
- if d['type'] not in ['part','lvm']:
+ if d['type'] not in ['part', 'lvm']:
continue
- # split()[0] to avoid the second part of the name, after the whiteline
+ # 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
@@ -84,18 +87,18 @@ def get_partitions_names():
except:
raise OperationFailed("Unable to retrieve partitions' full path.")

+
def get_partition_details(name):
try:
# Find device path
- dev_path = get_partition_path(name,
- get_partitions_paths())
+ 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)
+ 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,
@@ -106,7 +109,7 @@ def get_partition_details(name):
expression = r"%s=\".*?\"" % key
match = re.search(expression, output)
field = match.group()
- k, v = field.split('=',1)
+ k, v = field.split('=', 1)
d[k.lower()] = v[1:-1]
if d['mountpoint']:
# Sometimes the mountpoint comes with [SWAP] or other
--
1.7.11.7

Zhou Zheng Sheng

unread,
Dec 18, 2013, 5:20:50 AM12/18/13
to project...@googlegroups.com, zhou...@linux.vnet.ibm.com, shu...@linux.vnet.ibm.com, dani...@linux.vnet.ibm.com, sha...@linux.vnet.ibm.com, ali...@br.ibm.com, Zhou Zheng Sheng
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 <zhsh...@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

Zhou Zheng Sheng

unread,
Dec 18, 2013, 5:20:51 AM12/18/13
to project...@googlegroups.com, zhou...@linux.vnet.ibm.com, shu...@linux.vnet.ibm.com, dani...@linux.vnet.ibm.com, sha...@linux.vnet.ibm.com, ali...@br.ibm.com, Zhou Zheng Sheng
The current front-end implementation filter out disks from the free
block device listings. This is not needed after the previous back-end
pat correctly listing free disks.

Signed-off-by: Zhou Zheng Sheng <zhsh...@linux.vnet.ibm.com>
---
ui/js/src/kimchi.storagepool_add_main.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/js/src/kimchi.storagepool_add_main.js b/ui/js/src/kimchi.storagepool_add_main.js
index 7efbfa0..02323f5 100644
--- a/ui/js/src/kimchi.storagepool_add_main.js
+++ b/ui/js/src/kimchi.storagepool_add_main.js
@@ -45,7 +45,7 @@ kimchi.initStorageAddPage = function() {
var deviceHtml = $('#partitionTmpl').html();
var listHtml = '';
$.each(data, function(index, value) {
- if (value.type === 'part') {
+ if (value.type === 'part' || value.type === 'disk') {
listHtml += kimchi.template(deviceHtml, value);
}
});
--
1.7.11.7

Sheldon

unread,
Dec 18, 2013, 7:42:51 AM12/18/13
to Zhou Zheng Sheng, project...@googlegroups.com, zhou...@linux.vnet.ibm.com, shu...@linux.vnet.ibm.com, dani...@linux.vnet.ibm.com, ali...@br.ibm.com

Reviewed-by: ShaoHe Feng <sha...@linux.vnet.ibm.com>

       
Sheldon Feng(冯少合)
IBM Linux Technology Center

Sheldon

unread,
Dec 18, 2013, 7:58:17 AM12/18/13
to Zhou Zheng Sheng, project...@googlegroups.com, zhou...@linux.vnet.ibm.com, shu...@linux.vnet.ibm.com, dani...@linux.vnet.ibm.com, ali...@br.ibm.com
Reviewed-by: ShaoHe Feng <sha...@linux.vnet.ibm.com>
       
On 12/18/2013 06:20 PM, Zhou Zheng Sheng wrote:

Aline Manera

unread,
Dec 18, 2013, 12:22:14 PM12/18/13
to Project Kimchi
From: Zhou Zheng Sheng <zhsh...@linux.vnet.ibm.com>

Make this file PEP 8 clean

Signed-off-by: Zhou Zheng Sheng <zhsh...@linux.vnet.ibm.com>
---
--
1.7.10.4

Aline Manera

unread,
Dec 18, 2013, 12:22:13 PM12/18/13
to Project Kimchi
From: Aline Manera <ali...@br.ibm.com>

V1 -> V2:
- Get device name from sysfs uevent file and assume its path is /dev/<devname>

Zhou Zheng Sheng (3):
PEP 8: cleanup src/kimchi/disks.py
Issue #276: logical pool: a quick fix for the device listing rules,
back-end
Issue #276, logical pool: a quick fix for the device listing rules,
front-end

Makefile.am | 1 +
src/kimchi/disks.py | 177 ++++++++++++++++--------------
ui/js/src/kimchi.storagepool_add_main.js | 2 +-
3 files changed, 94 insertions(+), 86 deletions(-)

--
1.7.10.4

Aline Manera

unread,
Dec 18, 2013, 12:22:15 PM12/18/13
to Project Kimchi
From: Zhou Zheng Sheng <zhsh...@linux.vnet.ibm.com>

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 <zhsh...@linux.vnet.ibm.com>
Signed-off-by: Aline Manera <ali...@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)

-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])
+ 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))
+ 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.10.4

Aline Manera

unread,
Dec 18, 2013, 12:22:16 PM12/18/13
to Project Kimchi
From: Zhou Zheng Sheng <zhsh...@linux.vnet.ibm.com>

The current front-end implementation filter out disks from the free
block device listings. This is not needed after the previous back-end
pat correctly listing free disks.

Signed-off-by: Zhou Zheng Sheng <zhsh...@linux.vnet.ibm.com>
---
ui/js/src/kimchi.storagepool_add_main.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/js/src/kimchi.storagepool_add_main.js b/ui/js/src/kimchi.storagepool_add_main.js
index 66500f3..b31610a 100644
--- a/ui/js/src/kimchi.storagepool_add_main.js
+++ b/ui/js/src/kimchi.storagepool_add_main.js
@@ -45,7 +45,7 @@ kimchi.initStorageAddPage = function() {
var deviceHtml = $('#partitionTmpl').html();
var listHtml = '';
$.each(data, function(index, value) {
- if (value.type === 'part') {
+ if (value.type === 'part' || value.type === 'disk') {
listHtml += kimchi.template(deviceHtml, value);
}
});
--
1.7.10.4

Crístian Viana

unread,
Dec 18, 2013, 2:41:16 PM12/18/13
to Aline Manera, Project Kimchi
Reviewed-by: Cr�stian Viana <via...@linux.vnet.ibm.com>

Crístian Viana

unread,
Dec 18, 2013, 2:41:31 PM12/18/13
to Aline Manera, Project Kimchi
Reviewed-by: Cr�stian Viana <via...@linux.vnet.ibm.com>

On 18-12-2013 15:22, Aline Manera wrote:

Crístian Viana

unread,
Dec 18, 2013, 2:41:48 PM12/18/13
to Aline Manera, Project Kimchi
Reviewed-by: Cr�stian Viana <via...@linux.vnet.ibm.com>

On 18-12-2013 15:22, Aline Manera wrote:

Aline Manera

unread,
Dec 18, 2013, 2:52:22 PM12/18/13
to Aline Manera, Project Kimchi
Applied. Thanks.

Regards,

Aline Manera

Sheldon

unread,
Dec 19, 2013, 1:41:48 AM12/19/13
to Aline Manera, Project Kimchi, Zhou Zheng Sheng

       
On 12/19/2013 01:22 AM, Aline Manera wrote:
From: Zhou Zheng Sheng <zhsh...@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
+with open("/sys/dev/block/%s/uevent" % maj_min) as f:
+    out = f.read()
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

Zhou Zheng Sheng

unread,
Dec 19, 2013, 5:50:35 AM12/19/13
to Sheldon, Aline Manera, Project Kimchi
on 2013/12/19 14:41, Sheldon wrote:
>
> On 12/19/2013 01:22 AM, Aline Manera wrote:
>> From: Zhou Zheng Sheng <zhsh...@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

Sheldon, I sent a fix. See email titled 'logical pool fixes: only list
leaf devices, and read file instead of run "cat"'

Thanks for report this bug!
> Sheldon Feng(???)
> IBM Linux Technology Center
>


--
Thanks and best regards!

Zhou Zheng Sheng / 周征晟
E-mail: zhsh...@linux.vnet.ibm.com
Telephone: 86-10-82454397

Reply all
Reply to author
Forward
0 new messages