On Tue, Dec 10, 2013 at 10:56 AM, Guido Trotter <[email protected]> wrote:
> On Tue, Dec 10, 2013 at 10:48 AM, Michele Tartara <[email protected]> wrote:
>> On Tue, Dec 10, 2013 at 10:14 AM, Dimitris Aragiorgis <[email protected]> 
>> wrote:
>>> Disks of ext template are allowed to have arbitrary parameters
>>> stored in 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.
>>>
>>> Signed-off-by: Dimitris Aragiorgis <[email protected]>
>>> ---
>>>  lib/cmdlib/instance.py |   13 +++++++------
>>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
>>> index be30315..e01dd13 100644
>>> --- a/lib/cmdlib/instance.py
>>> +++ b/lib/cmdlib/instance.py
>>> @@ -2308,10 +2308,6 @@ 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)
>>
>> The OpPrereqError exception used to be raised for every template. Now,
>> this patch wants to prevent it to be raised for the ext template
>> (which is fine) but it actually removes it completely from all the
>> template.
>> Shouldn't it be something like (pseudocode):
>>
>> if len(params) > 2 and template != EXT:
>>    error "Disk modification supports additional arbitrary parameters
>> only for the EXT template"
>>
>
> Also the params dict in upgradeconfig is allowed to stay for all
> templates, not just for ext.
> Shouldn't we allow here to modify *all* parameters?
>
> Thanks,
>
> Guido
>


Fine for me, but in that case I also suggest renaming the patch without a
reference to the "ext" template.

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