On 12/09/2013 05:22 PM, Guido Trotter wrote:
On Mon, Dec 9, 2013 at 2:35 PM, Dimitris Aragiorgis <[email protected]> wrote:
* Constantinos Venetsanopoulos <[email protected]> [2013-12-02 17:23:38 +0200]:
Hello Guido,
On 12/02/2013 05:07 PM, Guido Trotter wrote:
The title of the patch is misleading: it seems you're fixing something
just for "ext" while you're actually making a change that affects all
disk templates, due to a bug that affects ext. Problem here is that
doing that you're reverting a bugfix from last year, that says that
some parameter that weren't supposed to be there showed up there.
I totally agree with you regarding the wording.
This should change.
Ack, thanks.
A
proper fix would include making sure that the parameters disk stays
empty for other disk templates where overriding is not supported.
However, if I understand correctly, the bug was introduced
in UpgradeConfig and was fixed wrong twice. The initial code
in UpgradeConfig actually filled the dict. If that's right, doesn't
this fix suffice?
Any comment on that?
As long as you confirm (I haven't tried) that by gnt-instance modify
it's always possible to update the change *and* to reset them to
default (aka remove an overridal from this disk) then the fix is
perfect. If that's not possible then fixing gnt-instance modify to
allow it is also required.
Ack.
Dimitris will send the appropriate patches right away.
Thanks,
Constantinos
Thanks a lot,
Guido
Thanks,
Constantinos
This should be done at instance save time, but not at UpgradeConfig
time of course, so the core of this patch is OK, provided the extra
work is added for non-ext disk templates.
Thanks a lot,
Guido
On Mon, Dec 2, 2013 at 3:44 PM, Dimitris Aragiorgis <[email protected]> wrote:
Commits 5dbee5e and cce4616 fix disk upgrades concerning params
slot. Since 2.7 params slot should be empty and gets filled
any time needed.
Still ext template allows passing arbitrary params per disk.
These params should be saved in config file for future use.
For instance if we have the shared-filer provider and we
specify shared_dir param during instance create, this param
is needed when we want to attach the disk e.g., during
retrieving instance info. If it gets overridden during a daemon
restart or a config reload we fail to get the instance's info.
To avoid such a failure, we set params slot to an empty dict
only if params not found in the first place.
Signed-off-by: Dimitris Aragiorgis <[email protected]>
---
lib/objects.py | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/lib/objects.py b/lib/objects.py
index ad9f1d7..f437fb8 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -828,7 +828,12 @@ class Disk(ConfigObject):
child.UpgradeConfig()
# FIXME: Make this configurable in Ganeti 2.7
- self.params = {}
+ # Params should be an empty dict that gets filled any time needed
+ # In case of ext template we allow arbitrary params that should not
+ # be overrided during a config reload/upgrade.
+ if not self.params or not isinstance(self.params, dict):
+ self.params = {}
+
# add here config upgrade for this disk
@staticmethod
--
1.7.10.4
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
iQEcBAEBAgAGBQJSpccYAAoJEHFDHex6CBG9ZZEIAKjzZOS7MHRGaB5IdQEI/UD9
yfh71l7NS+j0M2BQbtJHh9Tj9//qR8GLD+6/BsyPsk3J6+8ksaEKNM6TSeO6C6E1
cvYQR0cgMGdTVQxZjw2MUplDOJwq5/bSInXE4yqX3RezfzfbclL0G6PSNBvPwjI9
BB2GFH+1LXIxdNzar24DYUnUKjNo/J0qcp6qNcLfuNWb+qm7W9SGcHbq97z1B3eU
cc9kl2R2ZeL1/CdUWMWE/rpdiTGHwAV0oupD7bFgtf6Ir5WYla71oxwvZ9fp3C5S
DyOmjcOx8nhYyp4i97/UtfOvAERy114UQZpLMU1nyH4SsNs7xz/swSAL4eOJhXw=
=nytf
-----END PGP SIGNATURE-----
