Prior to this commit, removing a disk from a live instance without using hotplug succeeded incorrectly leaving stale disks behind. This is due to the fact that the hypervisor holds the device open while Ganeti tries to shut it down. The operation fails but Ganeti continues anyway, removing the disk from the config and leaving the disk stale.
This patch adds an appropriate check before removing a disk from a live instance. Signed-off-by: Yiannis Tsiouris <[email protected]> Signed-off-by: Filippos Giannakos <[email protected]> --- lib/cmdlib/instance_set_params.py | 17 ++++++++++++----- test/py/cmdlib/instance_unittest.py | 8 ++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/cmdlib/instance_set_params.py b/lib/cmdlib/instance_set_params.py index 9ffbfdf0a..6dc4c3792 100644 --- a/lib/cmdlib/instance_set_params.py +++ b/lib/cmdlib/instance_set_params.py @@ -162,16 +162,18 @@ class LUInstanceSetParams(LogicalUnit): utils.ForceDictType(params, (key_types if op != constants.DDM_ATTACH else key_types_attach)) + if op not in (constants.DDM_ADD, constants.DDM_ATTACH, + constants.DDM_MODIFY, constants.DDM_REMOVE, + constants.DDM_DETACH): + raise errors.ProgrammerError("Unhandled operation '%s'" % op) + if op in (constants.DDM_REMOVE, constants.DDM_DETACH): if params: raise errors.OpPrereqError("No settings should be passed when" " removing or detaching a %s" % kind, errors.ECODE_INVAL) - elif op in (constants.DDM_ADD, constants.DDM_ATTACH, - constants.DDM_MODIFY): - item_fn(op, params) - else: - raise errors.ProgrammerError("Unhandled operation '%s'" % op) + + item_fn(op, params) def _VerifyDiskModification(self, op, params, excl_stor, group_access_types): """Verifies a disk modification. @@ -263,6 +265,11 @@ class LUInstanceSetParams(LogicalUnit): if name is not None and name.lower() == constants.VALUE_NONE: params[constants.IDISK_NAME] = None + if op == constants.DDM_REMOVE and not self.op.hotplug: + CheckInstanceState(self, self.instance, INSTANCE_NOT_RUNNING, + msg="can't remove volume from a running instance" + " without using hotplug") + @staticmethod def _VerifyNicModification(op, params): """Verifies a network interface modification. diff --git a/test/py/cmdlib/instance_unittest.py b/test/py/cmdlib/instance_unittest.py index 1a4a45b3a..74d48721b 100644 --- a/test/py/cmdlib/instance_unittest.py +++ b/test/py/cmdlib/instance_unittest.py @@ -3041,6 +3041,10 @@ class TestLUInstanceSetParams(CmdlibTestCase): instance_name=inst.name, disks=[[constants.DDM_REMOVE, -1, {}]]) + self.rpc.call_instance_info.side_effect = [ + self.RpcResultsBuilder() \ + .CreateSuccessfulNodeResult(self.master) + ] self.rpc.call_file_storage_dir_remove.return_value = \ self.RpcResultsBuilder() \ .CreateSuccessfulNodeResult(self.master) @@ -3055,6 +3059,10 @@ class TestLUInstanceSetParams(CmdlibTestCase): instance_name=inst.name, disks=[[constants.DDM_REMOVE, -1, {}]]) + self.rpc.call_instance_info.side_effect = [ + self.RpcResultsBuilder() + .CreateSuccessfulNodeResult(self.master) + ] self.rpc.call_file_storage_dir_remove.return_value = \ self.RpcResultsBuilder() \ .CreateSuccessfulNodeResult(self.master) -- 2.11.0
