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:

+ 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)

+ 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'};

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

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); >> } >> });

On 03/26/2014 03:48 AM, Aline Manera wrote: > On 03/25/2014 04:56 AM, Zhou Zheng Sheng wrote: >> 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. > > > According to Adam we can do: > > list = ['part', 'disk', 'mpath'] > if list.indexOf(value.type) != -1 { Zhengsheng told me that indexOf is not supported by IE browser.

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