From e74f62910ed46353096fa2f1f0e6b04b1cca3d73 Mon Sep 17 00:00:00 2001
From: Constantinos Venetsanopoulos <cven@grnet.gr>
Date: Wed, 28 Mar 2012 12:29:22 +0300
Subject: [PATCH] Multiple ExtStorage Providers and ext-params

Add support for passing parameters to the ext template (ext-params).
Take advantage of disk-params, that don't seem to make much sense in
this template (ExtStorage Providers are not predefined and we don't
know their needs) and use them to pass the ext-params dynamically to
the template.

ext-params are correlated with gnt-os-interface's os-params.
All ext-params are exported to the ExtStorage Provider through it's
environment, with variables prefixed with 'EXTP_' (similarly to the
OS interface's 'OSP_' params).

ext-params are passed through the --disk option. If the disk template
is of type `ext' during instance add, then any additional options that
are not in IDISK_PARAMS given to --disk are considered ext-params
e.g.:

 gnt-instance add -t ext --disk=0:size=2G,param1=value1,param2=value2

Finally, we introduce a new IDISK_PARAM called IDISK_PROVIDER, that is
mandatory for template `ext' and is used to select the desired
ExtStorage Provider. This parameter is not valid for other template
types.

The IDISK_PROVIDER parameter becomes the first element of the
disk's unique_id tuple e.g.:

 unique_id = ('sample_provider1', 'UUID.ext.diskX')

Example selecting different ExtStorage Providers for each disk and
passing different ext-params to them:

 -t ext --disk=0:size=2G,provider=sample_provider1,param1=value1
        --disk=1:size=3G,provider=sample_provider2,param2=value2

Signed-off-by: Constantinos Venetsanopoulos <cven@grnet.gr>
---
 lib/bdev.py      | 47 +++++++++++++++++++++++++++++++---------
 lib/cmdlib.py    | 45 +++++++++++++++++++++++++++++++++++---
 lib/constants.py |  9 +++++++-
 lib/objects.py   |  5 +++++
 lib/opcodes.py   | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 148 insertions(+), 14 deletions(-)

diff --git a/lib/bdev.py b/lib/bdev.py
index 17b0faee0..40c75c31b 100644
--- a/lib/bdev.py
+++ b/lib/bdev.py
@@ -2653,6 +2653,7 @@ class ExtStorageDevice(BlockDev):
       raise ValueError("Invalid configuration data %s" % str(unique_id))
 
     self.driver, self.vol_name = unique_id
+    self.ext_params = params
 
     self.major = self.minor = None
     self.Attach()
@@ -2668,10 +2669,12 @@ class ExtStorageDevice(BlockDev):
     if not isinstance(unique_id, (tuple, list)) or len(unique_id) != 2:
       raise errors.ProgrammerError("Invalid configuration data %s" %
                                    str(unique_id))
+    ext_params = params
 
     # Call the External Storage's create script,
     # to provision a new Volume inside the External Storage
-    _ExtStorageAction(constants.ES_ACTION_CREATE, unique_id, str(size))
+    _ExtStorageAction(constants.ES_ACTION_CREATE, unique_id,
+                      ext_params, str(size))
 
     return ExtStorageDevice(unique_id, children, size, params)
 
@@ -2688,7 +2691,8 @@ class ExtStorageDevice(BlockDev):
 
     # Call the External Storage's remove script,
     # to remove the Volume from the External Storage
-    _ExtStorageAction(constants.ES_ACTION_REMOVE, self.unique_id)
+    _ExtStorageAction(constants.ES_ACTION_REMOVE, self.unique_id,
+                      self.ext_params)
 
   def Rename(self, new_id):
     """Rename this device.
@@ -2708,7 +2712,7 @@ class ExtStorageDevice(BlockDev):
     # Call the External Storage's attach script,
     # to attach an existing Volume to a block device under /dev
     self.dev_path = _ExtStorageAction(constants.ES_ACTION_ATTACH,
-                                      self.unique_id)
+                                      self.unique_id, self.ext_params)
 
     try:
       st = os.stat(self.dev_path)
@@ -2742,7 +2746,8 @@ class ExtStorageDevice(BlockDev):
 
     # Call the External Storage's detach script,
     # to detach an existing Volume from it's block device under /dev
-    _ExtStorageAction(constants.ES_ACTION_DETACH, self.unique_id)
+    _ExtStorageAction(constants.ES_ACTION_DETACH, self.unique_id,
+                      self.ext_params)
 
     self.minor = None
     self.dev_path = None
@@ -2781,10 +2786,10 @@ class ExtStorageDevice(BlockDev):
     # Call the External Storage's grow script,
     # to grow an existing Volume inside the External Storage
     _ExtStorageAction(constants.ES_ACTION_GROW, self.unique_id,
-                      str(self.size), grow=str(new_size))
+                      self.ext_params, str(self.size), grow=str(new_size))
 
 
-def _ExtStorageAction(action, unique_id, size=None, grow=None):
+def _ExtStorageAction(action, unique_id, ext_params, size=None, grow=None):
   """Take an External Storage action.
 
   Take an External Storage action concerning or affecting
@@ -2796,6 +2801,8 @@ def _ExtStorageAction(action, unique_id, size=None, grow=None):
   @type unique_id: tuple (driver, vol_name)
   @param unique_id: a tuple containing the type of ExtStorage (driver)
                     and the Volume name
+  @type ext_params: dict
+  @type ext_params: ExtStorage parameters
   @type size: integer
   @param size: the size of the Volume in mebibytes
   @type grow: integer
@@ -2811,7 +2818,7 @@ def _ExtStorageAction(action, unique_id, size=None, grow=None):
     _ThrowError("%s" % inst_es)
 
   # Create the basic environment for the driver's scripts
-  create_env = _ExtStorageEnvironment(unique_id, size, grow)
+  create_env = _ExtStorageEnvironment(unique_id, ext_params, size, grow)
 
   # Do not use log file for action `attach' as we need
   # to get the outpout from RunResult
@@ -2870,7 +2877,9 @@ def ExtStorageFromDisk(name, base_dir=None):
   # an optional one
   es_files = dict.fromkeys(constants.ES_SCRIPTS, True)
 
-  for filename in es_files:
+  es_files[constants.ES_PARAMETERS_FILE] = True
+
+  for (filename, required) in es_files.items():
     es_files[filename] = utils.PathJoin(es_dir, filename)
 
     try:
@@ -2888,21 +2897,35 @@ def ExtStorageFromDisk(name, base_dir=None):
         return False, ("File '%s' under path '%s' is not executable" %
                        (filename, es_dir))
 
+  parameters = []
+  if constants.ES_PARAMETERS_FILE in es_files:
+    parameters_file = es_files[constants.ES_PARAMETERS_FILE]
+    try:
+      parameters = utils.ReadFile(parameters_file).splitlines()
+    except EnvironmentError, err:
+      return False, ("Error while reading the EXT parameters file at %s: %s" %
+                     (parameters_file, utils.ErrnoOrStr(err)))
+    parameters = [v.split(None, 1) for v in parameters]
+
   es_obj = \
     objects.ExtStorage(name=name, path=es_dir,
                        create_script=es_files[constants.ES_SCRIPT_CREATE],
                        remove_script=es_files[constants.ES_SCRIPT_REMOVE],
                        grow_script=es_files[constants.ES_SCRIPT_GROW],
                        attach_script=es_files[constants.ES_SCRIPT_ATTACH],
-                       detach_script=es_files[constants.ES_SCRIPT_DETACH])
+                       detach_script=es_files[constants.ES_SCRIPT_DETACH],
+                       verify_script=es_files[constants.ES_SCRIPT_VERIFY],
+                       supported_parameters=parameters)
   return True, es_obj
 
 
-def _ExtStorageEnvironment(unique_id, size=None, grow=None):
+def _ExtStorageEnvironment(unique_id, ext_params, size=None, grow=None):
   """Calculate the environment for an External Storage script.
 
   @type unique_id: tuple (driver, vol_name)
   @param unique_id: ExtStorage pool and name of the Volume
+  @type ext_params: dict
+  @param ext_params: the EXT parameters
   @type size: integer
   @param size: size of the Volume in mebibytes
   @rtype: dict
@@ -2914,6 +2937,10 @@ def _ExtStorageEnvironment(unique_id, size=None, grow=None):
   result = {}
   result['VOL_NAME'] = vol_name
 
+  # EXT params
+  for pname, pvalue in ext_params.items():
+    result["EXTP_%s" % pname.upper()] = str(pvalue)
+
   if size is not None:
     result['VOL_SIZE'] = size
 
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index 320e855c7..e05aef0c8 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -7058,6 +7058,7 @@ class LUInstanceRecreateDisks(LogicalUnit):
     # TODO: Implement support changing VG while recreating
     constants.IDISK_VG,
     constants.IDISK_METAVG,
+    constants.IDISK_PROVIDER,
     ]))
 
   def CheckArguments(self):
@@ -8830,13 +8831,26 @@ def _GenerateDiskTemplate(lu, template_name, instance_name, primary_node,
     elif template_name == constants.DT_RBD:
       logical_id_fn = lambda idx, _, disk: ("rbd", names[idx])
     elif template_name == constants.DT_EXT:
-      logical_id_fn = lambda idx, _, disk: ("ext", names[idx])
+      def logical_id_fn(idx, _, disk):
+        provider = disk.get(constants.IDISK_PROVIDER, None)
+        if provider is None:
+          raise errors.ProgrammerError("Disk template is %s, but '%s' is"
+                                       " not found", constants.DT_EXT,
+                                       constants.IDISK_PROVIDER)
+        return (provider, names[idx])
     else:
       raise errors.ProgrammerError("Unknown disk template '%s'" % template_name)
 
     dev_type = _DISK_TEMPLATE_DEVICE_TYPE[template_name]
 
     for idx, disk in enumerate(disk_info):
+      params={}
+      # Only for the Ext template add disk_info to params
+      if template_name == constants.DT_EXT:
+        params[constants.IDISK_PROVIDER] = disk[constants.IDISK_PROVIDER]
+        for key in disk:
+          if key not in constants.IDISK_PARAMS:
+            params[key] = disk[key]
       disk_index = idx + base_index
       size = disk[constants.IDISK_SIZE]
       feedback_fn("* disk %s, size %s" %
@@ -8845,7 +8859,7 @@ def _GenerateDiskTemplate(lu, template_name, instance_name, primary_node,
                                 logical_id=logical_id_fn(idx, disk_index, disk),
                                 iv_name="disk/%d" % disk_index,
                                 mode=disk[constants.IDISK_MODE],
-                                params={}))
+                                params=params))
 
   return disks
 
@@ -9219,7 +9233,8 @@ class LUInstanceCreate(LogicalUnit):
     # check disks. parameter names and consistent adopt/no-adopt strategy
     has_adopt = has_no_adopt = False
     for disk in self.op.disks:
-      utils.ForceDictType(disk, constants.IDISK_PARAMS_TYPES)
+      if self.op.disk_template != constants.DT_EXT:
+        utils.ForceDictType(disk, constants.IDISK_PARAMS_TYPES)
       if constants.IDISK_ADOPT in disk:
         has_adopt = True
       else:
@@ -9813,16 +9828,37 @@ class LUInstanceCreate(LogicalUnit):
         raise errors.OpPrereqError("Invalid disk size '%s'" % size,
                                    errors.ECODE_INVAL)
 
+      ext_provider = disk.get(constants.IDISK_PROVIDER, None)
+      if ext_provider and self.op.disk_template != constants.DT_EXT:
+        raise errors.OpPrereqError("The '%s' option is only valid for the %s"
+                                   " disk template, not %s" %
+                                   (constants.IDISK_PROVIDER, constants.DT_EXT,
+                                   self.op.disk_template), errors.ECODE_INVAL)
+
       data_vg = disk.get(constants.IDISK_VG, default_vg)
       new_disk = {
         constants.IDISK_SIZE: size,
         constants.IDISK_MODE: mode,
         constants.IDISK_VG: data_vg,
         }
+
       if constants.IDISK_METAVG in disk:
         new_disk[constants.IDISK_METAVG] = disk[constants.IDISK_METAVG]
       if constants.IDISK_ADOPT in disk:
         new_disk[constants.IDISK_ADOPT] = disk[constants.IDISK_ADOPT]
+
+      # For extstorage, demand the `provider' option and add any
+      # additional parameters (ext-params) to the dict
+      if self.op.disk_template == constants.DT_EXT:
+        if ext_provider:
+          new_disk[constants.IDISK_PROVIDER] = ext_provider
+          for key in disk:
+            if key not in constants.IDISK_PARAMS:
+              new_disk[key] = disk[key]
+        else:
+          raise errors.OpPrereqError("Missing provider for template '%s'" %
+                                     constants.DT_EXT, errors.ECODE_INVAL)
+
       self.disks.append(new_disk)
 
     if self.op.mode == constants.INSTANCE_IMPORT:
@@ -10028,6 +10064,9 @@ class LUInstanceCreate(LogicalUnit):
 
     _CheckNicsBridgesExist(self, self.nics, self.pnode.name)
 
+    #TODO: _CheckExtParams (remotely)
+    # Check parameters for extstorage
+
     # memory check on primary node
     #TODO(dynmem): use MINMEM for checking
     if self.op.start:
diff --git a/lib/constants.py b/lib/constants.py
index b39b4bb57..49bff3ade 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -687,20 +687,25 @@ ES_ACTION_REMOVE = "remove"
 ES_ACTION_GROW = "grow"
 ES_ACTION_ATTACH = "attach"
 ES_ACTION_DETACH = "detach"
+ES_ACTION_VERIFY = "verify"
 
 ES_SCRIPT_CREATE = ES_ACTION_CREATE
 ES_SCRIPT_REMOVE = ES_ACTION_REMOVE
 ES_SCRIPT_GROW = ES_ACTION_GROW
 ES_SCRIPT_ATTACH = ES_ACTION_ATTACH
 ES_SCRIPT_DETACH = ES_ACTION_DETACH
+ES_SCRIPT_VERIFY = ES_ACTION_VERIFY
 ES_SCRIPTS = frozenset([
   ES_SCRIPT_CREATE,
   ES_SCRIPT_REMOVE,
   ES_SCRIPT_GROW,
   ES_SCRIPT_ATTACH,
-  ES_SCRIPT_DETACH
+  ES_SCRIPT_DETACH,
+  ES_SCRIPT_VERIFY
   ])
 
+ES_PARAMETERS_FILE = "parameters.list"
+
 # ssh constants
 SSH_CONFIG_DIR = _autoconf.SSH_CONFIG_DIR
 SSH_HOST_DSA_PRIV = SSH_CONFIG_DIR + "/ssh_host_dsa_key"
@@ -1132,12 +1137,14 @@ IDISK_MODE = "mode"
 IDISK_ADOPT = "adopt"
 IDISK_VG = "vg"
 IDISK_METAVG = "metavg"
+IDISK_PROVIDER = "provider"
 IDISK_PARAMS_TYPES = {
   IDISK_SIZE: VTYPE_SIZE,
   IDISK_MODE: VTYPE_STRING,
   IDISK_ADOPT: VTYPE_STRING,
   IDISK_VG: VTYPE_STRING,
   IDISK_METAVG: VTYPE_STRING,
+  IDISK_PROVIDER: VTYPE_STRING,
   }
 IDISK_PARAMS = frozenset(IDISK_PARAMS_TYPES.keys())
 
diff --git a/lib/objects.py b/lib/objects.py
index c5b8ce8e5..a9559318a 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -935,6 +935,9 @@ class Disk(ConfigObject):
                  params)
       result.append(params)
 
+    elif disk_template == constants.DT_EXT:
+      result.append(constants.DISK_LD_DEFAULTS[constants.LD_EXT])
+
     return result
 
 
@@ -1274,6 +1277,8 @@ class ExtStorage(ConfigObject):
     "grow_script",
     "attach_script",
     "detach_script",
+    "verify_script",
+    "supported_parameters",
     ]
 
 
diff --git a/lib/opcodes.py b/lib/opcodes.py
index aa7cce1e0..b91b1533f 100644
--- a/lib/opcodes.py
+++ b/lib/opcodes.py
@@ -196,6 +196,12 @@ _TDiskParams = \
   ht.Comment("Disk parameters")(ht.TDictOf(ht.TElemOf(constants.IDISK_PARAMS),
                                            ht.TOr(ht.TNonEmptyString, ht.TInt)))
 
+#: Same as _TDiskParams but with NonEmptyString in the place of IDISK_PARAMS
+_TExtDiskParams = \
+  ht.Comment("ExtStorage Disk parameters")(ht.TDictOf(ht.TNonEmptyString,
+                                                      ht.TOr(ht.TNonEmptyString,
+                                                             ht.TInt)))
+
 _TQueryRow = \
   ht.TListOf(ht.TAnd(ht.TIsLength(2),
                      ht.TItems([ht.TElemOf(constants.RS_ALL),
@@ -1225,6 +1231,56 @@ class OpInstanceCreate(OpCode):
     ]
   OP_RESULT = ht.Comment("instance nodes")(ht.TListOf(ht.TNonEmptyString))
 
+  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
+    is_ext = False
+    for (attr_name, _, _, _) in self.GetAllParams():
+      if hasattr(self, attr_name):
+        if attr_name == "disk_template" and \
+           getattr(self, attr_name) == constants.DT_EXT:
+          is_ext = 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 the template is DT_EXT and attr_name = disks
+      # set a new test method that allows passing of unknown parameters
+      if is_ext and attr_name == "disks":
+        test = ht.TListOf(_TExtDiskParams)
+
+      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 OpInstanceReinstall(OpCode):
   """Reinstall an instance's OS."""
-- 
GitLab