On Mon, Apr 14, 2014 at 04:42PM, Jose A. Lopes wrote: > On Apr 14 14:29, Ilias Tsitsimpis wrote: > > Signed-off-by: Ilias Tsitsimpis <[email protected]> > > --- > > lib/bootstrap.py | 1 + > > lib/objects.py | 10 +++++++--- > > src/Ganeti/Objects.hs | 1 + > > test/hs/Test/Ganeti/Objects.hs | 3 ++- > > 4 files changed, 11 insertions(+), 4 deletions(-) > > > > diff --git a/lib/bootstrap.py b/lib/bootstrap.py > > index d77f13e..04c8043 100644 > > --- a/lib/bootstrap.py > > +++ b/lib/bootstrap.py > > @@ -868,6 +868,7 @@ def InitConfig(version, cluster_config, > > master_node_config, > > nodes=nodes, > > instances={}, > > networks={}, > > + disks={}, > > serial_no=1, > > ctime=now, mtime=now) > > utils.WriteFile(cfg_file, > > diff --git a/lib/objects.py b/lib/objects.py > > index 6d10135..874951e 100644 > > --- a/lib/objects.py > > +++ b/lib/objects.py > > @@ -405,19 +405,20 @@ class ConfigData(ConfigObject): > > "nodegroups", > > "instances", > > "networks", > > + "disks", > > "serial_no", > > ] + _TIMESTAMPS > > > > def ToDict(self, _with_private=False): > > """Custom function for top-level config data. > > > > - This just replaces the list of instances, nodes and the cluster > > - with standard python types. > > + This just replaces the list of nodes, instances, nodegroups, > > + networks, disks and the cluster with standard python types. > > > > """ > > mydict = super(ConfigData, self).ToDict(_with_private=_with_private) > > mydict["cluster"] = mydict["cluster"].ToDict() > > - for key in "nodes", "instances", "nodegroups", "networks": > > + for key in "nodes", "instances", "nodegroups", "networks", "disks": > > mydict[key] = outils.ContainerToDicts(mydict[key]) > > > > return mydict > > @@ -435,6 +436,7 @@ class ConfigData(ConfigObject): > > obj.nodegroups = \ > > outils.ContainerFromDicts(obj.nodegroups, dict, NodeGroup) > > obj.networks = outils.ContainerFromDicts(obj.networks, dict, Network) > > + obj.disks = outils.ContainerFromDicts(obj.disks, dict, Disk) > > return obj > > > > def HasAnyDiskOfType(self, dev_type): > > @@ -475,6 +477,8 @@ class ConfigData(ConfigObject): > > self.networks = {} > > for network in self.networks.values(): > > network.UpgradeConfig() > > + for disk in self.disks.values(): > > + disk.UpgradeConfig() > > If noticed some items first test if the attribute is 'None', others don't. > Isn't it necessary to do this first: > > if self.disks is None: > self.disks = {} > > Can you explain the difference?
It seems to me that these checks are not necessary. The 'UpgradeConfig' method is called at configuration load time and it fills defaults for missing configuration values. But since Confd and Luxid were implemented in Haskell, the above configuration values (e.g., 'nodegroups', 'networks' and now 'disks') will always be present in the config file, otherwise the Haskell code will fail. I didn't remove the other checks because it wasn't relevant with my patch neither did I add the above check for the 'disks' configuration value because it is not needed. Thanks, Ilias > > def _UpgradeEnabledDiskTemplates(self): > > """Upgrade the cluster's enabled disk templates by inspecting the > > currently > > diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs > > index 020e518..3cf1ce8 100644 > > --- a/src/Ganeti/Objects.hs > > +++ b/src/Ganeti/Objects.hs > > @@ -757,6 +757,7 @@ $(buildObject "ConfigData" "config" $ > > , simpleField "nodegroups" [t| Container NodeGroup |] > > , simpleField "instances" [t| Container Instance |] > > , simpleField "networks" [t| Container Network |] > > + , simpleField "disks" [t| Container Disk |] > > ] > > ++ timeStampFields > > ++ serialFields) > > diff --git a/test/hs/Test/Ganeti/Objects.hs b/test/hs/Test/Ganeti/Objects.hs > > index bdad838..1a31b55 100644 > > --- a/test/hs/Test/Ganeti/Objects.hs > > +++ b/test/hs/Test/Ganeti/Objects.hs > > @@ -296,6 +296,7 @@ genEmptyCluster ncount = do > > else GenericContainer nodemap > > continsts = GenericContainer Map.empty > > networks = GenericContainer Map.empty > > + disks = GenericContainer Map.empty > > let contgroups = GenericContainer $ Map.singleton guuid grp > > serial <- arbitrary > > -- timestamp fields > > @@ -303,7 +304,7 @@ genEmptyCluster ncount = do > > mtime <- arbitrary > > cluster <- resize 8 arbitrary > > let c = ConfigData version cluster contnodes contgroups continsts > > networks > > - ctime mtime serial > > + disks ctime mtime serial > > return c > > > > -- | FIXME: make an even simpler base version of creating a cluster. > > -- > > 1.9.1 > >
signature.asc
Description: Digital signature
