From 47aaeaffe58e48f5b0b691bb51585257b9b94cb8 Mon Sep 17 00:00:00 2001
From: Constantinos Venetsanopoulos <cven@grnet.gr>
Date: Fri, 20 Jul 2012 18:38:10 +0300
Subject: [PATCH] Add --allow-arbit-params to gnt-instance modify

Signed-off-by: Constantinos Venetsanopoulos <cven@grnet.gr>
---
 lib/cli.py                 |  8 +++++
 lib/client/gnt_instance.py |  5 +--
 lib/cmdlib.py              | 64 ++++++++++++++++++++++++++++++++------
 lib/opcodes.py             | 52 +++++++++++++++++++++++++++++++
 4 files changed, 118 insertions(+), 11 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index feb0fe41a..9ba84d430 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -200,6 +200,7 @@ __all__ = [
   "HV_STATE_OPT",
   "IGNORE_IPOLICY_OPT",
   "INSTANCE_POLICY_OPTS",
+  "ALLOW_ARBITPARAMS_OPT",
   # Generic functions for CLI programs
   "ConfirmOperation",
   "CreateIPolicyFromOpts",
@@ -1431,6 +1432,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 parameters 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 c59817b5d..040b72f12 100644
--- a/lib/client/gnt_instance.py
+++ b/lib/client/gnt_instance.py
@@ -1417,7 +1417,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)
@@ -1603,7 +1604,7 @@ 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 e05aef0c8..3ffe608b9 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -12166,7 +12166,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:
@@ -12202,9 +12205,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):
@@ -12255,16 +12267,26 @@ class LUInstanceSetParams(LogicalUnit):
     if self.op.hvparams:
       _CheckGlobalHvParams(self.op.hvparams)
 
-    self.op.disks = \
-      self._UpgradeDiskNicMods("disk", self.op.disks,
-        opcodes.OpInstanceSetParams.TestDiskModifications)
+    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"
@@ -12418,6 +12440,30 @@ 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 b91b1533f..0c14507d1 100644
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -1548,6 +1548,7 @@ class OpInstanceSetParams(OpCode):
   """
   TestNicModifications = _TestInstSetParamsModList(_TestNicDef)
   TestDiskModifications = _TestInstSetParamsModList(_TDiskParams)
+  TestExtDiskModifications = _TestInstSetParamsModList(_TExtDiskParams)
 
   OP_DSC_FIELD = "instance_name"
   OP_PARAMS = [
@@ -1581,9 +1582,60 @@ 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
+
+    """
+    # 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 the 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."""
-- 
GitLab