[PATCH rhel6-branch] mpath: fix bug in parseMultipathOutput (#803883)

[Will Woods]

parseMultipathOutput was assuming that each new mpath block would start
with 'create: ' or 'mpath'. But sometimes we can get input like this:

create: mpatha [...]
    `- 2:3:0:0   sda 8:0  undef ready running
  reject: mpathb [...]
    `- 2:255:1:0 sdb 8:16 undef ready running

Since the regex only matched 'create: ', it missed the start of the new
block and incorrectly puts 'sdb' into 'mpatha' with 'sda'.

Instead, match any string of lowercase letters and save that as the
'action'. When we hit the end of the block, if the action is 'create' or
None (i.e. the plain 'mpath' case) it's saved, otherwise it's ignored.

(We should only ever see 'reject' and 'create' as far as I can tell, but
better safe than sorry.)
---
 storage/devicelibs/mpath.py       | 11 ++++++-----
 tests/storage/devicelibs/mpath.py | 17 +++++++++++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/storage/devicelibs/mpath.py b/storage/devicelibs/mpath.py
index 789add1..fc09f24 100644
--- a/storage/devicelibs/mpath.py
+++ b/storage/devicelibs/mpath.py
@@ -75,12 +75,13 @@ def parseMultipathOutput(output):
     if output is None:
         return mpaths

+    action = None
     name = None
     devices = []

policy = re.compile('^[|+` -]+policy')
     device = re.compile('^[|+` -]+[0-9]+:[0-9]+:[0-9]+:[0-9]+ +([a-zA-Z0-9!/]+)')
-    create = re.compile('^(create: )?(mpath\w+|[a-f0-9]+)')
+    create = re.compile('^(?:([a-z]+): )?(mpath\w+|[a-f0-9]+)')

lines = output.split('\n')
     for line in lines:
@@ -91,11 +92,11 @@ def parseMultipathOutput(output):
         if not lexemes:
             break
         if cmatch and cmatch.group(2):
-            if name and devices:
+            if name and devices and action in (None, 'create'):
                 mpaths[name] = devices
-                name = None
-                devices = []
+            action = cmatch.group(1)
             name = cmatch.group(2)
+            devices = []
         elif lexemes[0].startswith('size='):
             pass
         elif pmatch:
@@ -103,7 +104,7 @@ def parseMultipathOutput(output):
         elif dmatch:
             devices.append(dmatch.groups()[0].replace('!','/'))

-    if name and devices:
+    if name and devices and action in (None, 'create'):
         mpaths[name] = devices

return mpaths
diff --git a/tests/storage/devicelibs/mpath.py b/tests/storage/devicelibs/mpath.py
index f83ae3e..c2ac1e2 100755
--- a/tests/storage/devicelibs/mpath.py
+++ b/tests/storage/devicelibs/mpath.py
@@ -58,6 +58,20 @@ size=100G features='0' hwhandler='1 rdac' wp=rw
   `- 2:0:0:1 sdd 8:48 active undef running
 """

+    # bug #803883
+    output5 = """\
+Apr 10 11:38:10 | sdb: alua not supported
+create: mpatha (35000039328130360) undef IBM-ESXS,MBF2600RC
+size=559G features='0' hwhandler='0' wp=undef
+`-+- policy='round-robin 0' prio=1 status=undef
+  `- 2:3:0:0   sda 8:0  undef ready running
+Apr 10 11:38:10 | sdb: alua not supported
+reject: mpathb (1IBM     IPR-0   62E954E914C7A220) undef IBM,IPR-0   62E954E9
+size=532G features='1 queue_if_no_path' hwhandler='1 alua' wp=undef
+`-+- policy='round-robin 0' prio=-1 status=undef
+  `- 2:255:1:0 sdb 8:16 undef ready running
+"""
+
     def setUp(self):
         self.setupModules(
             ['_isys', 'logging', 'anaconda_log', 'block'])
@@ -81,6 +95,9 @@ size=100G features='0' hwhandler='1 rdac' wp=rw
         self.assertEqual(topology,
                          {'3600a0b800067fabc000067694d23fe6e' : ['sdb','sdd'],
                           '3600a0b800067fcc9000001f34d23ff88' : ['sda', 'sdc']})
+        topology = mpath.parseMultipathOutput(self.output5)
+        self.assertEqual(topology, {'mpatha':['sda']})
+

def suite():
     return unittest.TestLoader().loadTestsFromTestCase(MPathTestCase)

[Brian C. Lane]

On Mon, Oct 15, 2012 at 02:46:56PM -0400, Will Woods wrote:
> parseMultipathOutput was assuming that each new mpath block would start
> with 'create: ' or 'mpath'. But sometimes we can get input like this:
>
> create: mpatha [...]
>     `- 2:3:0:0   sda 8:0  undef ready running
>   reject: mpathb [...]
>     `- 2:255:1:0 sdb 8:16 undef ready running
>
> Since the regex only matched 'create: ', it missed the start of the new
> block and incorrectly puts 'sdb' into 'mpatha' with 'sda'.
>
> Instead, match any string of lowercase letters and save that as the
> 'action'. When we hit the end of the block, if the action is 'create' or
> None (i.e. the plain 'mpath' case) it's saved, otherwise it's ignored.
>
> (We should only ever see 'reject' and 'create' as far as I can tell, but
> better safe than sorry.)
>
> storage/devicelibs/mpath.py       | 11 ++++++-----
>  tests/storage/devicelibs/mpath.py | 17 +++++++++++++++++
>  2 files changed, 23 insertions(+), 5 deletions(-)

ACK