LGTM, thanks

On Wed, 22 Apr 2015 at 11:25 Hrvoje Ribicic <[email protected]> wrote:

> Actually, I have an interdiff:
>
> diff --git a/lib/cmdlib/instance_storage.py
> b/lib/cmdlib/instance_storage.py
> index a41383f..3f609d6 100644
> --- a/lib/cmdlib/instance_storage.py
> +++ b/lib/cmdlib/instance_storage.py
> @@ -737,12 +737,13 @@ def CheckSpindlesExclusiveStorage(diskdict, es_flag,
> required):
>
>
>  def CheckDiskExtProvider(diskdict, disk_template):
> -  """Check that the given disk
> +  """Check that the given disk should or should not have the provider
> param.
>
>    @type diskdict: dict
>    @param diskdict: disk parameters
>    @type disk_template: string
>    @param disk_template: the desired template of this disk
> +  @raise errors.OpPrereqError: when the parameter is used in the wrong way
>
>    """
>    ext_provider = diskdict.get(constants.IDISK_PROVIDER, None)
>
> On Wed, Apr 22, 2015 at 11:22 AM, Helga Velroyen <[email protected]>
> wrote:
>
>> 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
>>>
>>>
> Hrvoje Ribicic
> Ganeti Engineering
> 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
> Steuernummer: 48/725/00206
> Umsatzsteueridentifikationsnummer: DE813741370
>

Reply via email to