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