The option is added to allow us the passing of arbitrary ext-params during disk addition (gnt-instance modify --disk add: ...), same as we do during instance add.
The option is needed because during gnt-instance modify parameters' validation, we do not know the type of the disk template (whereas in gnt-instance add we find it from the -t option). The option is called 'arbit' and not 'ext' params because it may be useful in other contexts too, in the future. The corresponding prereq checks for the existence of the 'provider' parameter in case of an 'ext' template are also taken care off in this commit. It serves as a general introduction of the 'ext' template in gnt-instance modify. Signed-off-by: Constantinos Venetsanopoulos <[email protected]> Conflicts: lib/cmdlib.py Signed-off-by: Constantinos Venetsanopoulos <[email protected]> --- lib/cli.py | 8 +++++ lib/client/gnt_instance.py | 6 ++- lib/cmdlib.py | 71 +++++++++++++++++++++++++++++++++++++------ lib/opcodes.py | 55 ++++++++++++++++++++++++++++++++++ 4 files changed, 128 insertions(+), 12 deletions(-) diff --git a/lib/cli.py b/lib/cli.py index 91ea9f6..5f003d1 100644 --- a/lib/cli.py +++ b/lib/cli.py @@ -202,6 +202,7 @@ __all__ = [ "HV_STATE_OPT", "IGNORE_IPOLICY_OPT", "INSTANCE_POLICY_OPTS", + "ALLOW_ARBITPARAMS_OPT", # Generic functions for CLI programs "ConfirmOperation", "CreateIPolicyFromOpts", @@ -1440,6 +1441,13 @@ ABSOLUTE_OPT = cli_option("--absolute", dest="absolute", help="Marks the grow as absolute instead of the" " (default) relative mode") +ALLOW_ARBITPARAMS_OPT = cli_option("--allow-arbit-params", + dest="allow_arbit_params", + action="store_true", default=None, + help="Allow arbitrary params to be passed" + " to --disk(s) option (used by ExtStorage)") + + #: Options provided by all commands COMMON_OPTS = [DEBUG_OPT] diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py index fceb359..841ccc7 100644 --- a/lib/client/gnt_instance.py +++ b/lib/client/gnt_instance.py @@ -1422,7 +1422,8 @@ def SetInstanceParams(opts, args): force=opts.force, wait_for_sync=opts.wait_for_sync, offline=offline, - ignore_ipolicy=opts.ignore_ipolicy) + ignore_ipolicy=opts.ignore_ipolicy, + allow_arbit_params=opts.allow_arbit_params) # even if here we process the result, we allow submit only result = SubmitOrSend(op, opts) @@ -1608,7 +1609,8 @@ commands = { [BACKEND_OPT, DISK_OPT, FORCE_OPT, HVOPTS_OPT, NET_OPT, SUBMIT_OPT, DISK_TEMPLATE_OPT, SINGLE_NODE_OPT, OS_OPT, FORCE_VARIANT_OPT, OSPARAMS_OPT, DRY_RUN_OPT, PRIORITY_OPT, NWSYNC_OPT, OFFLINE_INST_OPT, - ONLINE_INST_OPT, IGNORE_IPOLICY_OPT, RUNTIME_MEM_OPT], + ONLINE_INST_OPT, IGNORE_IPOLICY_OPT, RUNTIME_MEM_OPT, + ALLOW_ARBITPARAMS_OPT], "<instance>", "Alters the parameters of an instance"), "shutdown": ( GenericManyOps("shutdown", _ShutdownInstance), [ArgInstance()], diff --git a/lib/cmdlib.py b/lib/cmdlib.py index 406ae2f..4554752 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -12241,7 +12241,10 @@ class LUInstanceSetParams(LogicalUnit): for (op, _, params) in mods: assert ht.TDict(params) - utils.ForceDictType(params, key_types) + # If key_types is an empty dict, we assume we have an 'ext' template + # and thus do not ForceDictType + if key_types: + utils.ForceDictType(params, key_types) if op == constants.DDM_REMOVE: if params: @@ -12277,9 +12280,18 @@ class LUInstanceSetParams(LogicalUnit): params[constants.IDISK_SIZE] = size - elif op == constants.DDM_MODIFY and constants.IDISK_SIZE in params: - raise errors.OpPrereqError("Disk size change not possible, use" - " grow-disk", errors.ECODE_INVAL) + elif op == constants.DDM_MODIFY: + if constants.IDISK_SIZE in params: + raise errors.OpPrereqError("Disk size change not possible, use" + " grow-disk", errors.ECODE_INVAL) + if constants.IDISK_MODE not in params: + raise errors.OpPrereqError("Disk 'mode' is the only kind of" + " modification supported, but missing", + errors.ECODE_NOENT) + if len(params) > 1: + raise errors.OpPrereqError("Disk modification doesn't support" + " additional arbitrary parameters", + errors.ECODE_INVAL) @staticmethod def _VerifyNicModification(op, params): @@ -12330,14 +12342,26 @@ class LUInstanceSetParams(LogicalUnit): if self.op.hvparams: _CheckGlobalHvParams(self.op.hvparams) - self.op.disks = self._UpgradeDiskNicMods( - "disk", self.op.disks, opcodes.OpInstanceSetParams.TestDiskModifications) - self.op.nics = self._UpgradeDiskNicMods( - "NIC", self.op.nics, opcodes.OpInstanceSetParams.TestNicModifications) + if self.op.allow_arbit_params: + self.op.disks = \ + self._UpgradeDiskNicMods("disk", self.op.disks, + opcodes.OpInstanceSetParams.TestExtDiskModifications) + else: + self.op.disks = \ + self._UpgradeDiskNicMods("disk", self.op.disks, + opcodes.OpInstanceSetParams.TestDiskModifications) + + self.op.nics = \ + self._UpgradeDiskNicMods("NIC", self.op.nics, + opcodes.OpInstanceSetParams.TestNicModifications) # Check disk modifications - self._CheckMods("disk", self.op.disks, constants.IDISK_PARAMS_TYPES, - self._VerifyDiskModification) + if self.op.allow_arbit_params: + self._CheckMods("disk", self.op.disks, {}, + self._VerifyDiskModification) + else: + self._CheckMods("disk", self.op.disks, constants.IDISK_PARAMS_TYPES, + self._VerifyDiskModification) if self.op.disks and self.op.disk_template is not None: raise errors.OpPrereqError("Disk template conversion and other disk" @@ -12491,6 +12515,33 @@ class LUInstanceSetParams(LogicalUnit): self.diskmod = PrepareContainerMods(self.op.disks, None) self.nicmod = PrepareContainerMods(self.op.nics, _InstNicModPrivate) + # Check the validity of the `provider' parameter + if instance.disk_template in constants.DT_EXT: + for mod in self.diskmod: + ext_provider = mod[2].get(constants.IDISK_PROVIDER, None) + if mod[0] == constants.DDM_ADD: + if ext_provider is None: + raise errors.OpPrereqError("Instance template is '%s' and parameter" + " '%s' missing, during disk add" % + (constants.DT_EXT, + constants.IDISK_PROVIDER), + errors.ECODE_NOENT) + elif mod[0] == constants.DDM_MODIFY: + if ext_provider: + raise errors.OpPrereqError("Parameter '%s' is invalid during disk" + " modification" % + constants.IDISK_PROVIDER, + errors.ECODE_INVAL) + else: + for mod in self.diskmod: + ext_provider = mod[2].get(constants.IDISK_PROVIDER, None) + if ext_provider is not None: + raise errors.OpPrereqError("Parameter '%s' is only valid for" + " instances of type '%s'" % + (constants.IDISK_PROVIDER, + constants.DT_EXT), + errors.ECODE_INVAL) + # OS change if self.op.os_name and not self.op.force: _CheckNodeHasOS(self, instance.primary_node, self.op.os_name, diff --git a/lib/opcodes.py b/lib/opcodes.py index 961192f..92e7798 100644 --- a/lib/opcodes.py +++ b/lib/opcodes.py @@ -1536,6 +1536,7 @@ class OpInstanceSetParams(OpCode): """ TestNicModifications = _TestInstSetParamsModList(_TestNicDef) TestDiskModifications = _TestInstSetParamsModList(_TDiskParams) + TestExtDiskModifications = _TestInstSetParamsModList(_TExtDiskParams) OP_DSC_FIELD = "instance_name" OP_PARAMS = [ @@ -1569,9 +1570,63 @@ class OpInstanceSetParams(OpCode): ("wait_for_sync", True, ht.TBool, "Whether to wait for the disk to synchronize, when changing template"), ("offline", None, ht.TMaybeBool, "Whether to mark instance as offline"), + ("allow_arbit_params", None, ht.TMaybeBool, + "Whether to allow the passing of arbitrary parameters to --disk(s)"), ] OP_RESULT = _TSetParamsResult + def Validate(self, set_defaults): + """Validate opcode parameters, optionally setting default values. + + @type set_defaults: bool + @param set_defaults: Whether to set default values + @raise errors.OpPrereqError: When a parameter value doesn't match + requirements + + """ + # pylint: disable=E1101 + # as OP_ID has been dynamically defined + + # Check if the template is DT_EXT + allow_arbitrary_params = False + for (attr_name, _, _, _) in self.GetAllParams(): + if hasattr(self, attr_name): + if attr_name == "allow_arbit_params" and \ + getattr(self, attr_name) == True: + allow_arbitrary_params = True + + for (attr_name, default, test, _) in self.GetAllParams(): + assert test == ht.NoType or callable(test) + + if not hasattr(self, attr_name): + if default == ht.NoDefault: + raise errors.OpPrereqError("Required parameter '%s.%s' missing" % + (self.OP_ID, attr_name), + errors.ECODE_INVAL) + elif set_defaults: + if callable(default): + dval = default() + else: + dval = default + setattr(self, attr_name, dval) + + # If `allow_arbit_params' is set, use ExtStorage's test method for disks + if allow_arbitrary_params and attr_name == "disks": + test = OpInstanceSetParams.TestExtDiskModifications + + if test == ht.NoType: + # no tests here + continue + + if set_defaults or hasattr(self, attr_name): + attr_val = getattr(self, attr_name) + if not test(attr_val): + logging.error("OpCode %s, parameter %s, has invalid type %s/value %s", + self.OP_ID, attr_name, type(attr_val), attr_val) + raise errors.OpPrereqError("Parameter '%s.%s' fails validation" % + (self.OP_ID, attr_name), + errors.ECODE_INVAL) + class OpInstanceGrowDisk(OpCode): """Grow a disk of an instance.""" -- 1.7.2.5
