Sorry for the late reply. LGTM, thanks for taking care of this.
Morg. On 13 March 2017 at 10:30, 'Viktor Bachraty' via ganeti-devel <[email protected]> wrote: > From: Ansgar Jazdzewski <[email protected]> > > In order to solve the missing "-p" option in "rbd showmapped" it is > removed from the showmap_cmd. To make sure we found the volume in the > showmapped (json) output, we now check also the pool in the > _ParseRbdShowmappedJson method. > > Signed-off-by: Ansgar Jazdzewski <[email protected]> > Signed-off-by: Viktor Bachraty <[email protected]> > --- > lib/storage/bdev.py | 17 +++++++++++------ > test/py/ganeti.storage.bdev_unittest.py | 18 +++++++++++------- > 2 files changed, 22 insertions(+), 13 deletions(-) > > diff --git a/lib/storage/bdev.py b/lib/storage/bdev.py > index 138defb12..c7d5f3b6d 100644 > --- a/lib/storage/bdev.py > +++ b/lib/storage/bdev.py > @@ -1020,8 +1020,6 @@ class RADOSBlockDevice(base.BlockDev): > showmap_cmd = [ > constants.RBD_CMD, > "showmapped", > - "-p", > - pool, > "--format", > "json" > ] > @@ -1032,7 +1030,7 @@ class RADOSBlockDevice(base.BlockDev): > result.fail_reason, result.output) > raise RbdShowmappedJsonError > > - return cls._ParseRbdShowmappedJson(result.output, volume_name) > + return cls._ParseRbdShowmappedJson(result.output, pool, volume_name) > except RbdShowmappedJsonError: > # For older versions of rbd, we have to parse the plain / text output > # manually. > @@ -1045,7 +1043,7 @@ class RADOSBlockDevice(base.BlockDev): > return cls._ParseRbdShowmappedPlain(result.output, volume_name) > > @staticmethod > - def _ParseRbdShowmappedJson(output, volume_name): > + def _ParseRbdShowmappedJson(output, volume_pool, volume_name): > """Parse the json output of `rbd showmapped'. > > This method parses the json output of `rbd showmapped' and returns the > rbd > @@ -1053,8 +1051,10 @@ class RADOSBlockDevice(base.BlockDev): > > @type output: string > @param output: the json output of `rbd showmapped' > + @type volume_pool: string > + @param volume_pool: name of the volume whose device we search for > @type volume_name: string > - @param volume_name: the name of the volume whose device we search for > + @param volume_name: name of the pool in which we search > @rtype: string or None > @return: block device path if the volume is mapped, else None > > @@ -1071,7 +1071,12 @@ class RADOSBlockDevice(base.BlockDev): > except KeyError: > base.ThrowError("'name' key missing from json object %s", devices) > > - if name == volume_name: > + try: > + pool = d["pool"] > + except KeyError: > + base.ThrowError("'pool' key missing from json object %s", devices) > + > + if name == volume_name and pool == volume_pool: > if rbd_dev is not None: > base.ThrowError("rbd volume %s is mapped more than once", > volume_name) > > diff --git a/test/py/ganeti.storage.bdev_unittest.py > b/test/py/ganeti.storage.bdev_unittest.py > index 2bfcb0449..0ee36bd6c 100755 > --- a/test/py/ganeti.storage.bdev_unittest.py > +++ b/test/py/ganeti.storage.bdev_unittest.py > @@ -94,6 +94,7 @@ class TestRADOSBlockDevice(testutils.GanetiTestCase): > self.output_invalid = > testutils.ReadTestData("bdev-rbd/output_invalid.txt") > > self.volume_name = "d7ab910a-4933-4ffe-88d0-faf2ce31390a.rbd.disk0" > + self.pool_name = "rbd" > self.test_unique_id = ("rbd", self.volume_name) > self.test_params = { > constants.LDP_POOL: "fake_pool" > @@ -102,16 +103,19 @@ class TestRADOSBlockDevice(testutils.GanetiTestCase): > def testParseRbdShowmappedJson(self): > parse_function = bdev.RADOSBlockDevice._ParseRbdShowmappedJson > > - self.assertEqual(parse_function(self.json_output_ok, self.volume_name), > - "/dev/rbd3") > - self.assertEqual(parse_function(self.json_output_empty, > self.volume_name), > - None) > - self.assertEqual(parse_function(self.json_output_no_matches, > + self.assertEqual(parse_function(self.json_output_ok, self.pool_name, > + self.volume_name), "/dev/rbd3") > + self.assertEqual(parse_function(self.json_output_ok, "fake_pool", > + self.volume_name), None) > + self.assertEqual(parse_function(self.json_output_empty, self.pool_name, > + self.volume_name), None) > + self.assertEqual(parse_function(self.json_output_no_matches, > self.pool_name, > self.volume_name), None) > self.assertRaises(errors.BlockDeviceError, parse_function, > - self.json_output_extra_matches, self.volume_name) > + self.json_output_extra_matches, self.pool_name, > + self.volume_name) > self.assertRaises(errors.BlockDeviceError, parse_function, > - self.output_invalid, self.volume_name) > + self.output_invalid, self.pool_name, self.volume_name) > > def testParseRbdShowmappedPlain(self): > parse_function = bdev.RADOSBlockDevice._ParseRbdShowmappedPlain > -- > 2.12.0.246.ga2ecc84866-goog >
