From 58a5965221b0114e63c8d88c066b50d40376bcba Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Thu, 27 Jan 2011 16:44:17 +0100
Subject: [PATCH] cluster verify: add hvparams verification
Currently, the validity of the hypervisor parameters is only checked
at init/modification time, and not in the cluster verify. This is bad,
as it can lead to inconsistent state that is only detected when the
next modification (which can be unrelated) is made, leading to
unexpected error messages.
This patch adds both syntax verification (in masterd) and validity
verification on remote nodes. The downside of the patch is that on
clusters with many instances which have custom parameters, it will be
slow. A possible improvement would be to detect duplicate, identical
set of parameters, and collapse these into a single verification, but
that is left as a TODO (in case it becomes problematic).
An additional change is in utils.ForceDict, where we said 'key',
whereas this function is always used with parameter dicts, so I
changed it to "Unknown parameter".
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
---
lib/backend.py | 9 +++++++++
lib/cmdlib.py | 42 ++++++++++++++++++++++++++++++++++++++++++
lib/constants.py | 1 +
lib/utils/__init__.py | 2 +-
4 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/lib/backend.py b/lib/backend.py
index 8f53364ac..8426fee78 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -506,6 +506,15 @@ def VerifyNode(what, cluster_name):
val = "Error while checking hypervisor: %s" % str(err)
tmp[hv_name] = val
+ if constants.NV_HVPARAMS in what and vm_capable:
+ result[constants.NV_HVPARAMS] = tmp = []
+ for source, hv_name, hvparms in what[constants.NV_HVPARAMS]:
+ try:
+ logging.info("Validating hv %s, %s", hv_name, hvparms)
+ hypervisor.GetHypervisor(hv_name).ValidateParameters(hvparms)
+ except errors.HypervisorError, err:
+ tmp.append((source, hv_name, str(err)))
+
if constants.NV_FILELIST in what:
result[constants.NV_FILELIST] = utils.FingerprintFiles(
what[constants.NV_FILELIST])
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index c5620186a..6051ee5e6 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -1397,6 +1397,13 @@ class LUClusterVerify(LogicalUnit):
_ErrorIf(test, self.ENODEHV, node,
"hypervisor %s verify failure: '%s'", hv_name, hv_result)
+ hvp_result = nresult.get(constants.NV_HVPARAMS, None)
+ if ninfo.vm_capable and isinstance(hvp_result, list):
+ for item, hv_name, hv_result in hvp_result:
+ _ErrorIf(True, self.ENODEHV, node,
+ "hypervisor %s parameter verify failure (source %s): %s",
+ hv_name, item, hv_result)
+
test = nresult.get(constants.NV_NODESETUP,
["Missing NODESETUP results"])
_ErrorIf(test, self.ENODESETUP, node, "node setup error: %s",
@@ -2029,6 +2036,21 @@ class LUClusterVerify(LogicalUnit):
return instdisk
+ def _VerifyHVP(self, hvp_data):
+ """Verifies locally the syntax of the hypervisor parameters.
+
+ """
+ for item, hv_name, hv_params in hvp_data:
+ msg = ("hypervisor %s parameters syntax check (source %s): %%s" %
+ (item, hv_name))
+ try:
+ hv_class = hypervisor.GetHypervisor(hv_name)
+ utils.ForceDictType(hv_params, constants.HVS_PARAMETER_TYPES)
+ hv_class.CheckParameterSyntax(hv_params)
+ except errors.GenericError, err:
+ self._ErrorIf(True, self.ECLUSTERCFG, None, msg % str(err))
+
+
def BuildHooksEnv(self):
"""Build hooks env.
@@ -2094,12 +2116,32 @@ class LUClusterVerify(LogicalUnit):
local_checksums = utils.FingerprintFiles(file_names)
+ # Compute the set of hypervisor parameters
+ hvp_data = []
+ for hv_name in hypervisors:
+ hvp_data.append(("cluster", hv_name, cluster.GetHVDefaults(hv_name)))
+ for os_name, os_hvp in cluster.os_hvp.items():
+ for hv_name, hv_params in os_hvp.items():
+ if not hv_params:
+ continue
+ full_params = cluster.GetHVDefaults(hv_name, os_name=os_name)
+ hvp_data.append(("os %s" % os_name, hv_name, full_params))
+ # TODO: collapse identical parameter values in a single one
+ for instance in instanceinfo.values():
+ if not instance.hvparams:
+ continue
+ hvp_data.append(("instance %s" % instance.name, instance.hypervisor,
+ cluster.FillHV(instance)))
+ # and verify them locally
+ self._VerifyHVP(hvp_data)
+
feedback_fn("* Gathering data (%d nodes)" % len(nodelist))
node_verify_param = {
constants.NV_FILELIST: file_names,
constants.NV_NODELIST: [node.name for node in nodeinfo
if not node.offline],
constants.NV_HYPERVISOR: hypervisors,
+ constants.NV_HVPARAMS: hvp_data,
constants.NV_NODENETTEST: [(node.name, node.primary_ip,
node.secondary_ip) for node in nodeinfo
if not node.offline],
diff --git a/lib/constants.py b/lib/constants.py
index df51ef38d..37ef696a5 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -838,6 +838,7 @@ NV_DRBDLIST = "drbd-list"
NV_FILELIST = "filelist"
NV_HVINFO = "hvinfo"
NV_HYPERVISOR = "hypervisor"
+NV_HVPARAMS = "hvparms"
NV_INSTANCELIST = "instancelist"
NV_LVLIST = "lvlist"
NV_MASTERIP = "master-ip"
diff --git a/lib/utils/__init__.py b/lib/utils/__init__.py
index 5fd66d9b2..cdf6a13e2 100644
--- a/lib/utils/__init__.py
+++ b/lib/utils/__init__.py
@@ -82,7 +82,7 @@ def ForceDictType(target, key_types, allowed_values=None):
for key in target:
if key not in key_types:
- msg = "Unknown key '%s'" % key
+ msg = "Unknown parameter '%s'" % key
raise errors.TypeEnforcementError(msg)
if target[key] in allowed_values:
--
GitLab