Mostly LGTM, however...
I am not very familiar with the system this part of code is
interfacing (Ceph/rbd) so I'll trust the committer with the logic
behind removing the pool parameter.
The py-tests are failing:
======================================================================
ERROR: testParseRbdShowmappedJson (__main__.TestRADOSBlockDevice)
----------------------------------------------------------------------
Traceback (most recent call last):
File "test/py/ganeti.storage.bdev_unittest.py", line 105, in
testParseRbdShowmappedJson
self.assertEqual(parse_function(self.json_output_ok, self.volume_name),
TypeError: _ParseRbdShowmappedJson() takes exactly 3 arguments (2 given)
Please fix these and also test the extra logic in the new code.
On 8 March 2017 at 15:46, '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 +++++++++++------
> 1 file changed, 11 insertions(+), 6 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)
>
> --
> 2.12.0.246.ga2ecc84866-goog
>
