On Tue, Dec 10, 2013 at 5:19 PM, Michele Tartara <[email protected]> wrote: > 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.
Don't worry about the resend, I managed to apply it manually. I'm doing a bit of testing, and if eveything is fine I'll submit the patch series. 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
