On Wed, Feb 5, 2014 at 6:46 PM, Jose A. Lopes <[email protected]> wrote:
>> - else:
>> - # check the parameter validity (remote check)
>> - CheckOSParams(self, False, [self.cfg.GetMasterNode()],
>> - os_name, self.new_osp[os_name])
>> +
>> + # private os params
>> + self.new_osp_private =
>> objects.FillDict(cluster.osparams_private_cluster,
>> + {})
>> + if self.op.osparams_private_cluster:
>> + for os_name, osp in self.op.osparams_private_cluster.items():
>> + if os_name not in self.new_osp_private:
>> + self.new_osp_private[os_name] = {}
>> +
>> + self.new_osp_private[os_name] = GetUpdatedParams(
>> + self.new_osp_private[os_name], osp, use_none=True
>> + )
>> +
>> + if not self.new_osp_private[os_name]:
>> + # we removed all parameters
>> + del self.new_osp_private[os_name]
>> +
>
> Can we extract the code above to a separate function/method and thus
> avoid the code duplication for osparams and private osparams?
This refactoring makes me quite nervous. This is my best shot at it. I
have not tested it.
diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
index 7d15c36..f9da4aa 100644
--- a/lib/cmdlib/cluster.py
+++ b/lib/cmdlib/cluster.py
@@ -1163,44 +1163,7 @@ class LUClusterSetParams(LogicalUnit):
self.new_os_hvp[os_name][hv_name].update(hv_dict)
# os parameters
- self.new_osp = objects.FillDict(cluster.osparams, {})
- if self.op.osparams:
- for os_name, osp in self.op.osparams.items():
- if os_name not in self.new_osp:
- self.new_osp[os_name] = {}
-
- self.new_osp[os_name] = GetUpdatedParams(self.new_osp[os_name], osp,
- use_none=True)
-
- if not self.new_osp[os_name]:
- # we removed all parameters
- del self.new_osp[os_name]
-
- # private os params
- self.new_osp_private = objects.FillDict(cluster.osparams_private_cluster,
- {})
- if self.op.osparams_private_cluster:
- for os_name, osp in self.op.osparams_private_cluster.items():
- if os_name not in self.new_osp_private:
- self.new_osp_private[os_name] = {}
-
- self.new_osp_private[os_name] = GetUpdatedParams(
- self.new_osp_private[os_name], osp, use_none=True
- )
-
- if not self.new_osp_private[os_name]:
- # we removed all parameters
- del self.new_osp_private[os_name]
-
- # Remove os validity check
- changed_oses = (set(self.new_osp.keys()) |
set(self.new_osp_private.keys()))
- for os_name in changed_oses:
- os_params = cluster.SimpleFillOS(os_name,
- self.new_osp.get(os_name, {}),
- self.new_osp_private.get(os_name, {}))
- # check the parameter validity (remote check)
- CheckOSParams(self, False, [self.cfg.GetMasterNode()],
- os_name, os_params)
+ self._BuildOSParams(cluster, os_name)
# changes to the hypervisor list
if self.op.enabled_hypervisors is not None:
@@ -1254,6 +1217,35 @@ class LUClusterSetParams(LogicalUnit):
" specified" % self.op.default_iallocator,
errors.ECODE_INVAL)
+ def _BuildOSParams(self, cluster, os_name):
+ "Calculate the new OS parameters for this operation."
+
+ def _GetNewParams(source, new_params):
+ "Wrapper around GetUpdatedParams."
+ result = objects.FillDict(source, {}) # deep copy of source
+ for os_name in new_params:
+ result[os_name] = GetUpdatedParams(result.get(os_name, {}),
+ new_params[os_name],
+ use_none=True)
+ if not result[os_name]:
+ del result[os_name] # we removed all parameters
+ return result
+
+ self.new_osp = _GetNewParams(cluster.osparams,
+ self.op.osparams)
+ self.new_osp_private = _GetNewParams(cluster.osparams_private_cluster,
+ self.op.osparams_private_cluster)
+
+ # Remove os validity check
+ changed_oses = (set(self.new_osp.keys()) |
set(self.new_osp_private.keys()))
+ for os_name in changed_oses:
+ os_params = cluster.SimpleFillOS(os_name,
+ self.new_osp.get(os_name, {}),
+ self.new_osp_private.get(os_name, {}))
+ # check the parameter validity (remote check)
+ CheckOSParams(self, False, [self.cfg.GetMasterNode()],
+ os_name, os_params)
+
def _CheckDiskTemplateConsistency(self):
"""Check whether the disk templates that are going to be disabled
are still in use by some instances.
--
Raffa Santi
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
