Commit 58a59652 authored by Iustin Pop's avatar Iustin Pop
Browse files

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: default avatarIustin Pop <>
Reviewed-by: default avatarMichael Hanselmann <>
parent 6f547f96
......@@ -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:"Validating hv %s, %s", hv_name, hvparms)
except errors.HypervisorError, err:
tmp.append((source, hv_name, str(err)))
if constants.NV_FILELIST in what:
result[constants.NV_FILELIST] = utils.FingerprintFiles(
......@@ -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))
hv_class = hypervisor.GetHypervisor(hv_name)
utils.ForceDictType(hv_params, constants.HVS_PARAMETER_TYPES)
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:
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:
hvp_data.append(("instance %s" %, instance.hypervisor,
# and verify them locally
feedback_fn("* Gathering data (%d nodes)" % len(nodelist))
node_verify_param = {
constants.NV_FILELIST: file_names,
constants.NV_NODELIST: [ for node in nodeinfo
if not node.offline],
constants.NV_HYPERVISOR: hypervisors,
constants.NV_HVPARAMS: hvp_data,
constants.NV_NODENETTEST: [(, node.primary_ip,
node.secondary_ip) for node in nodeinfo
if not node.offline],
......@@ -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"
......@@ -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:
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment