On 12/10/2013 02:34 PM, Guido Trotter wrote:
Makes sense. I'm just worried that the change will expose bugs that
might hardcode the default values in the dict and there would be no
way to reset them to default. I guess we should address this checking
in qa.
Ack.
So, we are good and we should discuss how we can
enable changing parameters also at disk level in a
generic way. Right?
Thanks,
Constantinos
G
On 10 Dec 2013 13:31, "Constantinos Venetsanopoulos" <[email protected]
<mailto:[email protected]>> wrote:
On 12/10/2013 02:26 PM, Guido Trotter wrote:
On Tue, Dec 10, 2013 at 1:03 PM, Dimitris Aragiorgis
<[email protected] <mailto:[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.
Thanks,
Constantinos
Signed-off-by: Dimitris Aragiorgis <[email protected]
<mailto:[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?
"""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.
+ 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 <http://disk.name> =
params.get(constants.IDISK_NAME)
changes.append(("disk.name/%d
<http://disk.name/%d>" % idx, disk.name <http://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
