LGTM, thanks

On Wed, 22 Apr 2015 at 11:20 'Hrvoje Ribicic' via ganeti-devel <
[email protected]> wrote:

> .... to make sure that we can use it later for checking disk
> modifications.
>
> Signed-off-by: Hrvoje Ribicic <[email protected]>
> ---
>  lib/cmdlib/instance_storage.py | 41
> +++++++++++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/lib/cmdlib/instance_storage.py
> b/lib/cmdlib/instance_storage.py
> index f24518d..a41383f 100644
> --- a/lib/cmdlib/instance_storage.py
> +++ b/lib/cmdlib/instance_storage.py
> @@ -365,12 +365,7 @@ def ComputeDisks(disks, disk_template, default_vg):
>        raise errors.OpPrereqError("Invalid disk size '%s'" % size,
>                                   errors.ECODE_INVAL)
>
> -    ext_provider = disk.get(constants.IDISK_PROVIDER, None)
> -    if ext_provider and 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,
> -                                  disk_template), errors.ECODE_INVAL)
> +    CheckDiskExtProvider(disk, disk_template)
>
>      data_vg = disk.get(constants.IDISK_VG, default_vg)
>      name = disk.get(constants.IDISK_NAME, None)
> @@ -400,14 +395,10 @@ def ComputeDisks(disks, disk_template, default_vg):
>      # For extstorage, demand the `provider' option and add any
>      # additional parameters (ext-params) to the dict
>      if 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)
> +      new_disk[constants.IDISK_PROVIDER] = disk[constants.IDISK_PROVIDER]
> +      for key in disk:
> +        if key not in constants.IDISK_PARAMS:
> +          new_disk[key] = disk[key]
>
>      new_disks.append(new_disk)
>
> @@ -745,6 +736,28 @@ def CheckSpindlesExclusiveStorage(diskdict, es_flag,
> required):
>                                 errors.ECODE_INVAL)
>
>
> +def CheckDiskExtProvider(diskdict, disk_template):
> +  """Check that the given disk
> +
> +  @type diskdict: dict
> +  @param diskdict: disk parameters
> +  @type disk_template: string
> +  @param disk_template: the desired template of this disk
> +
> +  """
> +  ext_provider = diskdict.get(constants.IDISK_PROVIDER, None)
> +
> +  if ext_provider and 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,
> +                                disk_template), errors.ECODE_INVAL)
> +
> +  if ext_provider is None and disk_template == constants.DT_EXT:
> +    raise errors.OpPrereqError("Missing provider for template '%s'" %
> +                               constants.DT_EXT, errors.ECODE_INVAL)
> +
> +
>  class LUInstanceRecreateDisks(LogicalUnit):
>    """Recreate an instance's missing disks.
>
> --
> 2.2.0.rc0.207.ga3a616c
>
>

Reply via email to