On Tue, Dec 10, 2013 at 1:53 PM, Dimitris Aragiorgis <[email protected]> wrote:
> * Constantinos Venetsanopoulos <[email protected]> [2013-12-10 14:31:53 +0200]:
>
>> On 12/10/2013 02:26 PM, Guido Trotter wrote:
>> >On Tue, Dec 10, 2013 at 1:03 PM, 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.
>> >>
>> >>Additionally, for other disk templates we allow modifying only
>> >>name and mode. If any other parameter is passed
>> >>_VerifyDiskModification() will raise an exception.
>> >>
>> >Why not allowing it also for others?
>>
>> Which ones exactly? I don't think I'm following here.
>>
>> Do you mean the "vgname"? If yes, I think this belongs
>> to a different patch that will introduce such functionality.
>>
>
> +1
>
> Currently the only valid changes for disk modification is name and mode.
> Params such as adopt, vg, metavg, provider are only used during disk creation.
>
>> Thanks,
>> Constantinos
>>
>> >>Signed-off-by: Dimitris Aragiorgis <[email protected]>
>> >>---
>> >> lib/cmdlib/instance.py | 23 +++++++++++++++--------
>> >> 1 file changed, 15 insertions(+), 8 deletions(-)
>> >>
>> >>diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
>> >>index be30315..0a0d29b 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):
>> >Can't you leave this as static and pass the instance in?
>> >
>
> This would make the patch to intrusive. CheckMods should change along
> with _VerifyNicModification..
>
Ack, if you think it's too intrusive no problem.
>> >> """Verifies a disk modification.
>> >>
>> >> """
>> >>@@ -2308,10 +2307,13 @@ 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:
>> >>+ allowed_mods = [constants.IDISK_MODE, constants.IDISK_NAME]
>> >this belongs to constants, not here.
>> >
>
> You mean something like:
>
> MODIFIABLE_IDISK_PARAMS = frozenset([IDISK_MODE, IDISK_NAME])
>
> defined in lib/constants.py
>
> If yes upon merging 2.8 to latest branches this should be defined in the
> haskell part related to constants.
>
Yes. No problem, we'll take care of the merge.
Thanks,
Guido
> Thanks,
> dimara
>
>> >>+ utils.ForceDictType(params, allowed_mods)
>> >>+
>> >> 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 +3184,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 +3197,12 @@ class LUInstanceSetParams(LogicalUnit):
>> >> disk.name = params.get(constants.IDISK_NAME)
>> >> changes.append(("disk.name/%d" % idx, disk.name))
>> >>
>> >>+ for key, value in params.iteritems():
>> >>+ if (key not in [constants.IDISK_MODE, constants.IDISK_NAME] 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, _):
>> >Thanks,
>> >
>> >Guido
>> >
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQEcBAEBAgAGBQJSpw7eAAoJEHFDHex6CBG9WhUH/iqIflGIRphx7osD3QU/n49q
> On58KEYYGkapIO56j4sMHZcGR4vRGrN/1kJv5/j5yXmeGKNFRkgrrGLQRMi70S+t
> b6mvpQvb13zGkeya1MxKdQKZNovH5lm1CCJYX9YeCeWn3owfrhDBzhYHnvjAu1Ny
> lz4UmWFNO1aP+SkvzWSkFynxUNRFyNuh88WR3IdwcmdgSmU7mTM0qASy23c3HUb8
> aA1WCTXKhak86Ayq5DrEII7TPAfNnnTTyEubIqKLNawRedzgxaHDHTAE/qKlfqtD
> XFEU+PUv+ZwZJLkYP3a1mG0j1kG6dIbsZ9171R5YZ9uhCtMq5rbteyrmbFklLc0=
> =dwcC
> -----END PGP SIGNATURE-----
>
--
Guido Trotter
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
