On Tue, Dec 10, 2013 at 5:00 PM, Dimitris Aragiorgis <[email protected]> wrote:
> Disks of ext template are allowed to have arbitrary parameters
> stored in the Disk object's params slot. Those parameters can be
> passed during creation of a new disk, either in LUInstanceCreate()
> or in LUInsanceSetParams(). Still those parameters can not be
> changed afterwards. With this patch we override this limitation.
>
> Currently, for the other disk templates we allow modifying only
> 'name' and 'mode'. Therefore, we introduce new constants
> MODIFIABLE_IDISK_PARAM* to include those params. If any other
> parameter is passed, _VerifyDiskModification() will raise an
> exception.
>
> Signed-off-by: Dimitris Aragiorgis <[email protected]>
> ---
>  lib/cmdlib/instance.py |   23 +++++++++++++++--------
>  lib/constants.py       |    6 ++++++
>  2 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index be30315..5041dcb 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -2277,8 +2277,7 @@ class LUInstanceSetParams(LogicalUnit):
>        else:
>          raise errors.ProgrammerError("Unhandled operation '%s'" % op)
>
> -  @staticmethod
> -  def _VerifyDiskModification(op, params):
> +  def _VerifyDiskModification(self, op, params):
>      """Verifies a disk modification.
>
>      """
> @@ -2308,10 +2307,12 @@ class LUInstanceSetParams(LogicalUnit):
>        if constants.IDISK_SIZE in params:
>          raise errors.OpPrereqError("Disk size change not possible, use"
>                                     " grow-disk", errors.ECODE_INVAL)
> -      if len(params) > 2:
> -        raise errors.OpPrereqError("Disk modification doesn't support"
> -                                   " additional arbitrary parameters",
> -                                   errors.ECODE_INVAL)
> +
> +      # Disk modification supports changing only the disk name and mode.
> +      # Changing arbitrary parameters is allowed only for ext disk template",
> +      if self.instance.disk_template != constants.DT_EXT:
> +        utils.ForceDictType(params, constants.MODIFIABLE_IDISK_PARAMS_TYPES)
> +
>        name = params.get(constants.IDISK_NAME, None)
>        if name is not None and name.lower() == constants.VALUE_NONE:
>          params[constants.IDISK_NAME] = None
> @@ -3182,8 +3183,7 @@ class LUInstanceSetParams(LogicalUnit):
>        ("disk/%d" % idx, "add:size=%s,mode=%s" % (disk.size, disk.mode)),
>        ])
>
> -  @staticmethod
> -  def _ModifyDisk(idx, disk, params, _):
> +  def _ModifyDisk(self, idx, disk, params, _):
>      """Modifies a disk.
>
>      """
> @@ -3196,6 +3196,13 @@ class LUInstanceSetParams(LogicalUnit):
>        disk.name = params.get(constants.IDISK_NAME)
>        changes.append(("disk.name/%d" % idx, disk.name))
>
> +    # Modify arbitrary params in case instance template is ext
> +    for key, value in params.iteritems():
> +      if (key not in constants.MODIFIABLE_IDISK_PARAMS and
> +          self.instance.disk_template == constants.DT_EXT):
> +        disk.params[key] = value
> +        changes.append(("disk.params:%s/%d" % (key, idx), value))
> +
>      return changes
>
>    def _RemoveDisk(self, idx, root, _):
> diff --git a/lib/constants.py b/lib/constants.py
> index d8b6063..ee178f8 100644
> --- a/lib/constants.py
> +++ b/lib/constants.py
> @@ -1354,6 +1354,12 @@ IDISK_PARAMS_TYPES = {
>    }
>  IDISK_PARAMS = frozenset(IDISK_PARAMS_TYPES.keys())
>
> +MODIFIABLE_IDISK_PARAMS_TYPES = {
> +  IDISK_MODE: VTYPE_STRING,
> +  IDISK_NAME: VTYPE_STRING,
> +  }
> +MODIFIABLE_IDISK_PARAMS = frozenset(MODIFIABLE_IDISK_PARAMS_TYPES.keys())
> +
>  # INIC_* constants are used in opcodes, to create/change nics
>  INIC_MAC = "mac"
>  INIC_IP = "ip"
> --
> 1.7.10.4
>

LGTM.

Could you please also resend patch 3/3?
The previous one cannot be applied anymore, on top of this one.

Thanks,
Michele

-- 
Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

Reply via email to