From 205ab586688b287bc8c1f4796fa685bff4d3a8f9 Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Mon, 18 May 2009 21:15:41 +0200
Subject: [PATCH] Move to data-based hvparam checks instead of code
Currently the hypervisor parameters are checked using hard-coded snippets in
each hypervisor. However, most parameter checks fall into three cases:
- file check
- directory check
- string value in a set
And the remaining ones are checked using simple functions.
This patch moves to a declarative-style for these parameter checks; in
hv_base we add the necessary infrastructure for these checks, and the
above common cases.
This translates into complete removal of the Check/Verify functions for
the Xen hypervisors, and a drastic reduction for the KVM one (which has
inter-parameter dependencies and thus can't use a simple table).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>
---
lib/hypervisor/hv_base.py | 101 +++++++++++++++++++---
lib/hypervisor/hv_kvm.py | 149 ++++++++-------------------------
lib/hypervisor/hv_xen.py | 170 ++++++--------------------------------
3 files changed, 150 insertions(+), 270 deletions(-)
diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py
index e5de1b7cc..7f100476f 100644
--- a/lib/hypervisor/hv_base.py
+++ b/lib/hypervisor/hv_base.py
@@ -21,12 +21,65 @@
"""Base class for all hypervisors
+The syntax for the _CHECK variables and the contents of the PARAMETERS
+dict is the same, see the docstring for L{BaseHypervisor.PARAMETERS}.
+
+@var _FILE_CHECK: stub for file checks, without the required flag
+@var _DIR_CHECK: stub for directory checks, without the required flag
+@var REQ_FILE_CHECK: mandatory file parameter
+@var OPT_FILE_CHECK: optional file parameter
+@var REQ_DIR_CHECK: mandatory directory parametr
+@var OPT_DIR_CHECK: optional directory parameter
+@var NO_CHECK: parameter without any checks at all
+@var REQUIRED_CHECK: parameter required to exist (and non-false), but
+ without other checks; beware that this can't be used for boolean
+ parameters, where you should use NO_CHECK or a custom checker
+
"""
+import os
import re
from ganeti import errors
+from ganeti import utils
+
+
+# Read the BaseHypervisor.PARAMETERS docstring for the syntax of the
+# _CHECK values
+
+# must be afile
+_FILE_CHECK = (os.path.isabs, "must be an absolute path",
+ os.path.isfile, "not found or not a file")
+
+# must be a directory
+_DIR_CHECK = (os.path.isabs, "must be an absolute path",
+ os.path.isdir, "not found or not a directory")
+
+# nice wrappers for users
+REQ_FILE_CHECK = (True, ) + _FILE_CHECK
+OPT_FILE_CHECK = (False, ) + _FILE_CHECK
+REQ_DIR_CHECK = (True, ) + _DIR_CHECK
+OPT_DIR_CHECK = (False, ) + _DIR_CHECK
+
+# no checks at all
+NO_CHECK = (False, None, None, None, None)
+
+# required, but no other checks
+REQUIRED_CHECK = (True, None, None, None, None)
+
+def ParamInSet(required, my_set):
+ """Builds parameter checker for set membership.
+
+ @type required: boolean
+ @param required: whether this is a required parameter
+ @type my_set: tuple, list or set
+ @param my_set: allowed values set
+
+ """
+ fn = lambda x: x in my_set
+ err = ("The value must be one of: %s" % utils.CommaJoin(my_set))
+ return (required, fn, err, None, None)
class BaseHypervisor(object):
@@ -35,8 +88,18 @@ class BaseHypervisor(object):
The goal is that all aspects of the virtualisation technology are
abstracted away from the rest of code.
+ @cvar PARAMETERS: a dict of parameter name: check type; the check type is
+ a five-tuple containing:
+ - the required flag (boolean)
+ - a function to check for syntax, that will be used in
+ L{CheckParameterSyntax}, in the master daemon process
+ - an error message for the above function
+ - a function to check for parameter validity on the remote node,
+ in the L{ValidateParameters} function
+ - an error message for the above function
+
"""
- PARAMETERS = []
+ PARAMETERS = {}
def __init__(self):
pass
@@ -171,14 +234,25 @@ class BaseHypervisor(object):
"""
for key in hvparams:
if key not in cls.PARAMETERS:
- raise errors.HypervisorError("Hypervisor parameter '%s'"
- " not supported" % key)
- for key in cls.PARAMETERS:
- if key not in hvparams:
- raise errors.HypervisorError("Hypervisor parameter '%s'"
- " missing" % key)
-
- def ValidateParameters(self, hvparams):
+ raise errors.HypervisorError("Parameter '%s' is not supported" % key)
+
+ # cheap tests that run on the master, should not access the world
+ for name, (required, check_fn, errstr, _, _) in cls.PARAMETERS.items():
+ if name not in hvparams:
+ raise errors.HypervisorError("Parameter '%s' is missing" % name)
+ value = hvparams[name]
+ if not required and not value:
+ continue
+ if not value:
+ raise errors.HypervisorError("Parameter '%s' is required but"
+ " is currently not defined" % (name, ))
+ if check_fn is not None and not check_fn(value):
+ raise errors.HypervisorError("Parameter '%s' fails syntax"
+ " check: %s (current value: '%s')" %
+ (name, errstr, value))
+
+ @classmethod
+ def ValidateParameters(cls, hvparams):
"""Check the given parameters for validity.
This should check the passed set of parameters for
@@ -189,7 +263,14 @@ class BaseHypervisor(object):
@raise errors.HypervisorError: when a parameter is not valid
"""
- pass
+ for name, (required, _, _, check_fn, errstr) in cls.PARAMETERS.items():
+ value = hvparams[name]
+ if not required and not value:
+ continue
+ if check_fn is not None and not check_fn(value):
+ raise errors.HypervisorError("Parameter '%s' fails"
+ " validation: %s (current value: '%s')" %
+ (name, errstr, value))
def GetLinuxNodeInfo(self):
"""For linux systems, return actual OS information.
diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index 3b9a0dfe2..bc7294792 100644
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -48,23 +48,30 @@ class KVMHypervisor(hv_base.BaseHypervisor):
_CONF_DIR = _ROOT_DIR + "/conf" # contains instances startup data
_DIRS = [_ROOT_DIR, _PIDS_DIR, _CTRL_DIR, _CONF_DIR]
- PARAMETERS = [
- constants.HV_KERNEL_PATH,
- constants.HV_INITRD_PATH,
- constants.HV_ROOT_PATH,
- constants.HV_KERNEL_ARGS,
- constants.HV_ACPI,
- constants.HV_SERIAL_CONSOLE,
- constants.HV_VNC_BIND_ADDRESS,
- constants.HV_VNC_TLS,
- constants.HV_VNC_X509,
- constants.HV_VNC_X509_VERIFY,
- constants.HV_CDROM_IMAGE_PATH,
- constants.HV_BOOT_ORDER,
- constants.HV_NIC_TYPE,
- constants.HV_DISK_TYPE,
- constants.HV_USB_MOUSE,
- ]
+ PARAMETERS = {
+ constants.HV_KERNEL_PATH: hv_base.OPT_FILE_CHECK,
+ constants.HV_INITRD_PATH: hv_base.OPT_FILE_CHECK,
+ constants.HV_ROOT_PATH: hv_base.NO_CHECK,
+ constants.HV_KERNEL_ARGS: hv_base.NO_CHECK,
+ constants.HV_ACPI: hv_base.NO_CHECK,
+ constants.HV_SERIAL_CONSOLE: hv_base.NO_CHECK,
+ constants.HV_VNC_BIND_ADDRESS: \
+ (False, lambda x: (utils.IsValidIP(x) or os.path.isabs(x)),
+ "the VNC bind address must be either a valid IP address or an absolute"
+ " pathname", None, None),
+ constants.HV_VNC_TLS: hv_base.NO_CHECK,
+ constants.HV_VNC_X509: hv_base.OPT_DIR_CHECK,
+ constants.HV_VNC_X509_VERIFY: hv_base.NO_CHECK,
+ constants.HV_CDROM_IMAGE_PATH: hv_base.OPT_FILE_CHECK,
+ constants.HV_BOOT_ORDER: \
+ hv_base.ParamInSet(True, constants.HT_KVM_VALID_BO_TYPES),
+ constants.HV_NIC_TYPE: \
+ hv_base.ParamInSet(True, constants.HT_KVM_VALID_NIC_TYPES),
+ constants.HV_DISK_TYPE: \
+ hv_base.ParamInSet(True, constants.HT_KVM_VALID_DISK_TYPES),
+ constants.HV_USB_MOUSE: \
+ hv_base.ParamInSet(False, constants.HT_KVM_VALID_MOUSE_TYPES),
+ }
_MIGRATION_STATUS_RE = re.compile('Migration\s+status:\s+(\w+)',
re.M | re.I)
@@ -688,109 +695,23 @@ class KVMHypervisor(hv_base.BaseHypervisor):
kernel_path = hvparams[constants.HV_KERNEL_PATH]
if kernel_path:
- if not os.path.isabs(hvparams[constants.HV_KERNEL_PATH]):
- raise errors.HypervisorError("The kernel path must be an absolute path"
- ", if defined")
-
if not hvparams[constants.HV_ROOT_PATH]:
- raise errors.HypervisorError("Need a root partition for the instance"
- ", if a kernel is defined")
-
- if hvparams[constants.HV_INITRD_PATH]:
- if not os.path.isabs(hvparams[constants.HV_INITRD_PATH]):
- raise errors.HypervisorError("The initrd path must an absolute path"
- ", if defined")
+ raise errors.HypervisorError("Need a root partition for the instance,"
+ " if a kernel is defined")
- vnc_bind_address = hvparams[constants.HV_VNC_BIND_ADDRESS]
- if vnc_bind_address:
- if not utils.IsValidIP(vnc_bind_address):
- if not os.path.isabs(vnc_bind_address):
- raise errors.HypervisorError("The VNC bind address must be either"
- " a valid IP address or an absolute"
- " pathname. '%s' given" %
- vnc_bind_address)
-
- if hvparams[constants.HV_VNC_X509_VERIFY] and \
- not hvparams[constants.HV_VNC_X509]:
- raise errors.HypervisorError("%s must be defined, if %s is" %
- (constants.HV_VNC_X509,
- constants.HV_VNC_X509_VERIFY))
-
- if hvparams[constants.HV_VNC_X509]:
- if not os.path.isabs(hvparams[constants.HV_VNC_X509]):
- raise errors.HypervisorError("The vnc x509 path must an absolute path"
- ", if defined")
-
- iso_path = hvparams[constants.HV_CDROM_IMAGE_PATH]
- if iso_path and not os.path.isabs(iso_path):
- raise errors.HypervisorError("The path to the CDROM image must be"
- " an absolute path, if defined")
+ if (hvparams[constants.HV_VNC_X509_VERIFY] and
+ not hvparams[constants.HV_VNC_X509]):
+ raise errors.HypervisorError("%s must be defined, if %s is" %
+ (constants.HV_VNC_X509,
+ constants.HV_VNC_X509_VERIFY))
boot_order = hvparams[constants.HV_BOOT_ORDER]
- if boot_order not in constants.HT_KVM_VALID_BO_TYPES:
- raise errors.HypervisorError(\
- "The boot order must be one of %s" %
- utils.CommaJoin(constants.HT_KVM_VALID_BO_TYPES))
- if boot_order == constants.HT_BO_CDROM and not iso_path:
+ if (boot_order == constants.HT_BO_CDROM and
+ not hvparams[constants.HV_CDROM_IMAGE_PATH]):
raise errors.HypervisorError("Cannot boot from cdrom without an"
" ISO path")
-
- nic_type = hvparams[constants.HV_NIC_TYPE]
- if nic_type not in constants.HT_KVM_VALID_NIC_TYPES:
- raise errors.HypervisorError(\
- "Invalid NIC type %s specified for the KVM"
- " hypervisor. Please choose one of: %s" %
- (nic_type, utils.CommaJoin(constants.HT_KVM_VALID_NIC_TYPES)))
- elif (boot_order == constants.HT_BO_NETWORK and
- nic_type == constants.HT_NIC_PARAVIRTUAL):
+ if (boot_order == constants.HT_BO_NETWORK and
+ hvparams[constants.HV_NIC_TYPE] == constants.HT_NIC_PARAVIRTUAL):
raise errors.HypervisorError("Cannot boot from a paravirtual NIC. Please"
" change the NIC type.")
-
- disk_type = hvparams[constants.HV_DISK_TYPE]
- if disk_type not in constants.HT_KVM_VALID_DISK_TYPES:
- raise errors.HypervisorError(\
- "Invalid disk type %s specified for the KVM"
- " hypervisor. Please choose one of: %s" %
- (disk_type, utils.CommaJoin(constants.HT_KVM_VALID_DISK_TYPES)))
-
- mouse_type = hvparams[constants.HV_USB_MOUSE]
- if mouse_type and mouse_type not in constants.HT_KVM_VALID_MOUSE_TYPES:
- raise errors.HypervisorError(\
- "Invalid usb mouse type %s specified for the KVM hypervisor. Please"
- " choose one of %s" %
- utils.CommaJoin(constants.HT_KVM_VALID_MOUSE_TYPES))
-
- def ValidateParameters(self, hvparams):
- """Check the given parameters for validity.
-
- For the KVM hypervisor, this checks the existence of the
- kernel.
-
- """
- super(KVMHypervisor, self).ValidateParameters(hvparams)
-
- kernel_path = hvparams[constants.HV_KERNEL_PATH]
- if kernel_path and not os.path.isfile(kernel_path):
- raise errors.HypervisorError("Instance kernel '%s' not found or"
- " not a file" % kernel_path)
- initrd_path = hvparams[constants.HV_INITRD_PATH]
- if initrd_path and not os.path.isfile(initrd_path):
- raise errors.HypervisorError("Instance initrd '%s' not found or"
- " not a file" % initrd_path)
-
- vnc_bind_address = hvparams[constants.HV_VNC_BIND_ADDRESS]
- if vnc_bind_address and not utils.IsValidIP(vnc_bind_address) and \
- not os.path.isdir(vnc_bind_address):
- raise errors.HypervisorError("Instance vnc bind address must be either"
- " an ip address or an existing directory")
-
- vnc_x509 = hvparams[constants.HV_VNC_X509]
- if vnc_x509 and not os.path.isdir(vnc_x509):
- raise errors.HypervisorError("Instance vnc x509 path '%s' not found"
- " or not a directory" % vnc_x509)
-
- iso_path = hvparams[constants.HV_CDROM_IMAGE_PATH]
- if iso_path and not os.path.isfile(iso_path):
- raise errors.HypervisorError("Instance cdrom image '%s' not found or"
- " not a file" % iso_path)
diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
index 9779e51b2..c444cf026 100644
--- a/lib/hypervisor/hv_xen.py
+++ b/lib/hypervisor/hv_xen.py
@@ -401,58 +401,12 @@ class XenHypervisor(hv_base.BaseHypervisor):
class XenPvmHypervisor(XenHypervisor):
"""Xen PVM hypervisor interface"""
- PARAMETERS = [
- constants.HV_KERNEL_PATH,
- constants.HV_INITRD_PATH,
- constants.HV_ROOT_PATH,
- constants.HV_KERNEL_ARGS,
- ]
-
- @classmethod
- def CheckParameterSyntax(cls, hvparams):
- """Check the given parameters for validity.
-
- For the PVM hypervisor, this only check the existence of the
- kernel.
-
- @type hvparams: dict
- @param hvparams: dictionary with parameter names/value
- @raise errors.HypervisorError: when a parameter is not valid
-
- """
- super(XenPvmHypervisor, cls).CheckParameterSyntax(hvparams)
-
- if not hvparams[constants.HV_KERNEL_PATH]:
- raise errors.HypervisorError("Need a kernel for the instance")
-
- if not os.path.isabs(hvparams[constants.HV_KERNEL_PATH]):
- raise errors.HypervisorError("The kernel path must be an absolute path")
-
- if not hvparams[constants.HV_ROOT_PATH]:
- raise errors.HypervisorError("Need a root partition for the instance")
-
- if hvparams[constants.HV_INITRD_PATH]:
- if not os.path.isabs(hvparams[constants.HV_INITRD_PATH]):
- raise errors.HypervisorError("The initrd path must be an absolute path"
- ", if defined")
-
- def ValidateParameters(self, hvparams):
- """Check the given parameters for validity.
-
- For the PVM hypervisor, this only check the existence of the
- kernel.
-
- """
- super(XenPvmHypervisor, self).ValidateParameters(hvparams)
-
- kernel_path = hvparams[constants.HV_KERNEL_PATH]
- if not os.path.isfile(kernel_path):
- raise errors.HypervisorError("Instance kernel '%s' not found or"
- " not a file" % kernel_path)
- initrd_path = hvparams[constants.HV_INITRD_PATH]
- if initrd_path and not os.path.isfile(initrd_path):
- raise errors.HypervisorError("Instance initrd '%s' not found or"
- " not a file" % initrd_path)
+ PARAMETERS = {
+ constants.HV_KERNEL_PATH: hv_base.REQ_FILE_CHECK,
+ constants.HV_INITRD_PATH: hv_base.OPT_FILE_CHECK,
+ constants.HV_ROOT_PATH: hv_base.REQUIRED_CHECK,
+ constants.HV_KERNEL_ARGS: hv_base.NO_CHECK,
+ }
@classmethod
def _WriteConfigFile(cls, instance, block_devices):
@@ -510,100 +464,24 @@ class XenPvmHypervisor(XenHypervisor):
class XenHvmHypervisor(XenHypervisor):
"""Xen HVM hypervisor interface"""
- PARAMETERS = [
- constants.HV_ACPI,
- constants.HV_BOOT_ORDER,
- constants.HV_CDROM_IMAGE_PATH,
- constants.HV_DISK_TYPE,
- constants.HV_NIC_TYPE,
- constants.HV_PAE,
- constants.HV_VNC_BIND_ADDRESS,
- constants.HV_KERNEL_PATH,
- constants.HV_DEVICE_MODEL,
- ]
-
- @classmethod
- def CheckParameterSyntax(cls, hvparams):
- """Check the given parameter syntax.
-
- """
- super(XenHvmHypervisor, cls).CheckParameterSyntax(hvparams)
- # boot order verification
- boot_order = hvparams[constants.HV_BOOT_ORDER]
- if not boot_order or len(boot_order.strip("acdn")) != 0:
- raise errors.HypervisorError("Invalid boot order '%s' specified,"
- " must be one or more of [acdn]" %
- boot_order)
- # device type checks
- nic_type = hvparams[constants.HV_NIC_TYPE]
- if nic_type not in constants.HT_HVM_VALID_NIC_TYPES:
- raise errors.HypervisorError(\
- "Invalid NIC type %s specified for the Xen"
- " HVM hypervisor. Please choose one of: %s"
- % (nic_type, utils.CommaJoin(constants.HT_HVM_VALID_NIC_TYPES)))
- disk_type = hvparams[constants.HV_DISK_TYPE]
- if disk_type not in constants.HT_HVM_VALID_DISK_TYPES:
- raise errors.HypervisorError(\
- "Invalid disk type %s specified for the Xen"
- " HVM hypervisor. Please choose one of: %s"
- % (disk_type, utils.CommaJoin(constants.HT_HVM_VALID_DISK_TYPES)))
- # vnc_bind_address verification
- vnc_bind_address = hvparams[constants.HV_VNC_BIND_ADDRESS]
- if vnc_bind_address:
- if not utils.IsValidIP(vnc_bind_address):
- raise errors.OpPrereqError("given VNC bind address '%s' doesn't look"
- " like a valid IP address" %
- vnc_bind_address)
-
- iso_path = hvparams[constants.HV_CDROM_IMAGE_PATH]
- if iso_path and not os.path.isabs(iso_path):
- raise errors.HypervisorError("The path to the HVM CDROM image must"
- " be an absolute path or None, not %s" %
- iso_path)
-
- if not hvparams[constants.HV_KERNEL_PATH]:
- raise errors.HypervisorError("Need a kernel for the instance")
-
- if not os.path.isabs(hvparams[constants.HV_KERNEL_PATH]):
- raise errors.HypervisorError("The kernel path must be an absolute path")
-
- if not hvparams[constants.HV_DEVICE_MODEL]:
- raise errors.HypervisorError("Need a device model for the instance")
-
- if not os.path.isabs(hvparams[constants.HV_DEVICE_MODEL]):
- raise errors.HypervisorError("The device model must be an absolute path")
-
-
- def ValidateParameters(self, hvparams):
- """Check the given parameters for validity.
-
- For the PVM hypervisor, this only check the existence of the
- kernel.
-
- @type hvparams: dict
- @param hvparams: dictionary with parameter names/value
- @raise errors.HypervisorError: when a parameter is not valid
-
- """
- super(XenHvmHypervisor, self).ValidateParameters(hvparams)
-
- # hvm_cdrom_image_path verification
- iso_path = hvparams[constants.HV_CDROM_IMAGE_PATH]
- if iso_path and not os.path.isfile(iso_path):
- raise errors.HypervisorError("The HVM CDROM image must either be a"
- " regular file or a symlink pointing to"
- " an existing regular file, not %s" %
- iso_path)
-
- kernel_path = hvparams[constants.HV_KERNEL_PATH]
- if not os.path.isfile(kernel_path):
- raise errors.HypervisorError("Instance kernel '%s' not found or"
- " not a file" % kernel_path)
-
- device_model = hvparams[constants.HV_DEVICE_MODEL]
- if not os.path.isfile(device_model):
- raise errors.HypervisorError("Device model '%s' not found or"
- " not a file" % device_model)
+ PARAMETERS = {
+ constants.HV_ACPI: hv_base.NO_CHECK,
+ constants.HV_BOOT_ORDER: (True, ) + \
+ (lambda x: x and len(x.strip("acdn")) == 0,
+ "Invalid boot order specified, must be one or more of [acdn]",
+ None, None),
+ constants.HV_CDROM_IMAGE_PATH: hv_base.OPT_FILE_CHECK,
+ constants.HV_DISK_TYPE: \
+ hv_base.ParamInSet(True, constants.HT_HVM_VALID_DISK_TYPES),
+ constants.HV_NIC_TYPE: \
+ hv_base.ParamInSet(True, constants.HT_HVM_VALID_NIC_TYPES),
+ constants.HV_PAE: hv_base.NO_CHECK,
+ constants.HV_VNC_BIND_ADDRESS: \
+ (False, utils.IsValidIP,
+ "VNC bind address is not a valid IP address", None, None),
+ constants.HV_KERNEL_PATH: hv_base.REQ_FILE_CHECK,
+ constants.HV_DEVICE_MODEL: hv_base.REQ_FILE_CHECK,
+ }
@classmethod
def _WriteConfigFile(cls, instance, block_devices):
--
GitLab