Re: [Kimchi-devel] [PATCH] Logical Storage Pool: Detect and Enumerate Multipath Block Devices

Thanks a lot for the comments on the back-end code. I will change it in v2. Maybe Hong Liang can share what he thinks on the front-end code. Then I'll update the patch. on 2014/03/25 14:15, Mark Wu wrote: > On 03/25/2014 11:52 AM, zhshzhou(a)linux.vnet.ibm.com wrote: >> From: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; >> >> When creating a logical storage pool Kimci ignores multipath block >> devices. This is on purpose because at the time the kimchi.disks was >> implemented, there was not enough time to test every kind of devices. >> >> This patch adds the missing support for multipath block devices. After >> this patch, the kimchi.disks will include multipath devices in the >> available partition list. The front-end code is also changed >> accordingly. >> >> Signed-off-by: Zhou Zheng Sheng <zhshzhou(a)linux.vnet.ibm.com&gt; >> --- >> src/kimchi/disks.py | 20 +++++++++++++++++++- >> ui/js/src/kimchi.storagepool_add_main.js | 3 ++- >> 2 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/src/kimchi/disks.py b/src/kimchi/disks.py >> index ce0c5bb..6817ab4 100644 >> --- a/src/kimchi/disks.py >> +++ b/src/kimchi/disks.py >> @@ -17,6 +17,7 @@ >> # 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 errno >> import re >> import subprocess >> >> @@ -27,8 +28,25 @@ from kimchi.exception import OperationFailed >> from kimchi.utils import kimchi_log >> >> >> +def _get_friendly_dm_path(maj_min): >> + """ Returns user friendly dm path given the device number >> 'major:min' """ >> + dm_name = "/sys/dev/block/%s/dm/name" % maj_min >> + try: >> + with open(dm_name) as dm_f: >> + content = dm_f.read().rstrip('\n') >> + except IOError as e: >> + if not e.errno == errno.ENOENT: > why not "!=" >> + raise >> + return None >> + return "/dev/mapper/" + content >> + >> + >> def _get_dev_node_path(maj_min): >> """ Returns device node path given the device number 'major:min' "" >> + dm_path = _get_friendly_dm_path(maj_min) > check if string starts with "253:" could avoid reading the dm name file > for non device mapper file. > The major device number 253 is reserved for device mapper device in > kernel. It's reliable. >> + if dm_path is not None: >> + return dm_path >> + >> uevent = "/sys/dev/block/%s/uevent" % maj_min >> with open(uevent) as ueventf: >> content = ueventf.read() >> @@ -137,7 +155,7 @@ def get_partitions_names(): >> # 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 not (dev['type'] in ['part', 'disk'] and >> + if not (dev['type'] in ['part', 'disk', 'mpath'] and >> dev['fstype'] in ['', 'LVM2_member'] and >> dev['mountpoint'] == "" and >> _get_vgname(devNodePath) == "" and >> diff --git a/ui/js/src/kimchi.storagepool_add_main.js >> b/ui/js/src/kimchi.storagepool_add_main.js >> index 10833be..f3b85d2 100644 >> --- a/ui/js/src/kimchi.storagepool_add_main.js >> +++ b/ui/js/src/kimchi.storagepool_add_main.js >> @@ -28,7 +28,8 @@ kimchi.initStorageAddPage = function() { >> var deviceHtml = $('#partitionTmpl').html(); >> var listHtml = ''; >> $.each(data, function(index, value) { >> - if (value.type === 'part' || value.type === 'disk') { >> + valid_types = {part: 'part', disk: 'disk', mpath: >> 'mpath'}; > what we need is just a list, not sure if we can have a 'in' operation > for list in js. Maybe Hongliang can help here.

>> + if (value.type in valid_types) { >> listHtml += kimchi.template(deviceHtml, value); >> } >> });