* 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..

> >>      """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.

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
> >

Attachment: signature.asc
Description: Digital signature

Reply via email to