* 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. > > > 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? > 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 > >> > > > >
signature.asc
Description: Digital signature
