Move more hypervisor strings into constants

This patch adds constants for the mouse and boot order strings; while
there are still some issues remaining, we're trying to cleanup hardcoded
strings from the hypervisors.

Since the formatting of frozensets is currently wrong, we also add an
utility function for this and change all the error messages to use it.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

diff --git a/lib/constants.py b/lib/constants.py
index 058f2dd..494531a 100644 (file)
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -356,7 +356,7 @@ VNC_BASE_PORT = 5900
 VNC_PASSWORD_FILE = _autoconf.SYSCONFDIR + "/ganeti/vnc-cluster-password"
 VNC_DEFAULT_BIND_ADDRESS = '0.0.0.0'
 
-# Device types
+# NIC types
 HT_NIC_RTL8139 = "rtl8139"
 HT_NIC_NE2K_PCI = "ne2k_pci"
 HT_NIC_NE2K_ISA = "ne2k_isa"
@@ -366,25 +366,40 @@ HT_NIC_I8259ER = "i82559er"
 HT_NIC_PCNET = "pcnet"
 HT_NIC_E1000 = "e1000"
 HT_NIC_PARAVIRTUAL = HT_DISK_PARAVIRTUAL = "paravirtual"
-HT_DISK_IOEMU = "ioemu"
-HT_DISK_IDE = "ide"
-HT_DISK_SCSI = "scsi"
-HT_DISK_SD = "sd"
-HT_DISK_MTD = "mtd"
-HT_DISK_PFLASH = "pflash"
 
 HT_HVM_VALID_NIC_TYPES = frozenset([HT_NIC_RTL8139, HT_NIC_NE2K_PCI,
                                     HT_NIC_NE2K_ISA, HT_NIC_PARAVIRTUAL])
-HT_HVM_VALID_DISK_TYPES = frozenset([HT_DISK_PARAVIRTUAL, HT_DISK_IOEMU])
 HT_KVM_VALID_NIC_TYPES = frozenset([HT_NIC_RTL8139, HT_NIC_NE2K_PCI,
                                     HT_NIC_NE2K_ISA, HT_NIC_I82551,
                                     HT_NIC_I85557B, HT_NIC_I8259ER,
                                     HT_NIC_PCNET, HT_NIC_E1000,
                                     HT_NIC_PARAVIRTUAL])
+# Disk types
+HT_DISK_IOEMU = "ioemu"
+HT_DISK_IDE = "ide"
+HT_DISK_SCSI = "scsi"
+HT_DISK_SD = "sd"
+HT_DISK_MTD = "mtd"
+HT_DISK_PFLASH = "pflash"
+
+HT_HVM_VALID_DISK_TYPES = frozenset([HT_DISK_PARAVIRTUAL, HT_DISK_IOEMU])
 HT_KVM_VALID_DISK_TYPES = frozenset([HT_DISK_PARAVIRTUAL, HT_DISK_IDE,
                                      HT_DISK_SCSI, HT_DISK_SD, HT_DISK_MTD,
                                      HT_DISK_PFLASH])
 
+# Mouse types:
+HT_MOUSE_MOUSE = "mouse"
+HT_MOUSE_TABLET = "tablet"
+
+HT_KVM_VALID_MOUSE_TYPES = frozenset([HT_MOUSE_MOUSE, HT_MOUSE_TABLET])
+
+# Boot order
+HT_BO_CDROM = "cdrom"
+HT_BO_DISK = "disk"
+HT_BO_NETWORK = "network"
+
+HT_KVM_VALID_BO_TYPES = frozenset([HT_BO_CDROM, HT_BO_DISK, HT_BO_NETWORK])
+
 # Cluster Verify steps
 VERIFY_NPLUSONE_MEM = 'nplusone_mem'
 VERIFY_OPTIONAL_CHECKS = frozenset([VERIFY_NPLUSONE_MEM])
@@ -501,7 +516,7 @@ HVC_DEFAULTS = {
     HV_VNC_X509: '',
     HV_VNC_X509_VERIFY: False,
     HV_CDROM_IMAGE_PATH: '',
-    HV_BOOT_ORDER: "disk",
+    HV_BOOT_ORDER: HT_BO_DISK,
     HV_NIC_TYPE: HT_NIC_PARAVIRTUAL,
     HV_DISK_TYPE: HT_DISK_PARAVIRTUAL,
     HV_USB_MOUSE: '',

diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py
index 7e2fdea..ee33894 100644 (file)
--- a/lib/hypervisor/hv_kvm.py
+++ b/lib/hypervisor/hv_kvm.py
@@ -236,9 +236,9 @@ class KVMHypervisor(hv_base.BaseHypervisor):
       kvm_cmd.extend(['-no-acpi'])
 
     hvp = instance.hvparams
-    boot_disk = hvp[constants.HV_BOOT_ORDER] == "disk"
-    boot_cdrom = hvp[constants.HV_BOOT_ORDER] == "cdrom"
-    boot_network = hvp[constants.HV_BOOT_ORDER] == "network"
+    boot_disk = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_DISK
+    boot_cdrom = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_CDROM
+    boot_network = hvp[constants.HV_BOOT_ORDER] == constants.HT_BO_NETWORK
 
     if boot_network:
       kvm_cmd.extend(['-boot', 'n'])
@@ -723,35 +723,39 @@ class KVMHypervisor(hv_base.BaseHypervisor):
                                    " an absolute path, if defined")
 
     boot_order = hvparams[constants.HV_BOOT_ORDER]
-    if boot_order not in ('cdrom', 'disk', 'network'):
-      raise errors.HypervisorError("The boot order must be 'cdrom', 'disk' or"
-                                   " 'network'")
+    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 == 'cdrom' and not iso_path:
-      raise errors.HypervisorError("Cannot boot from cdrom without an ISO path")
+    if boot_order == constants.HT_BO_CDROM and not iso_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,
-                                    constants.HT_KVM_VALID_NIC_TYPES))
-    elif boot_order == 'network' and nic_type == constants.HT_NIC_PARAVIRTUAL:
+      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):
       raise errors.HypervisorError("Cannot boot from a paravirtual NIC. Please"
-                                   " change the nic type.")
+                                   " 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,
-                                    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 ('mouse', 'tablet'):
-      raise errors.HypervisorError("Invalid usb mouse type %s specified for"
-                                   " the KVM hyervisor. Please choose"
-                                   " 'mouse' or 'tablet'" % mouse_type)
+    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.

diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py
index 959958d..4579f4d 100644 (file)
--- a/lib/hypervisor/hv_xen.py
+++ b/lib/hypervisor/hv_xen.py
@@ -535,16 +535,16 @@ class XenHvmHypervisor(XenHypervisor):
     # 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,
-                                      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,
-                                      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:

diff --git a/lib/utils.py b/lib/utils.py
index 786cb0e..e876e2c 100644 (file)
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -1833,6 +1833,16 @@ def SafeEncode(text):
   return text
 
 
+def CommaJoin(names):
+  """Nicely join a set of identifiers.
+
+  @param names: set, list or tuple
+  @return: a string with the formatted results
+
+  """
+  return ", ".join(["'%s'" % val for val in names])
+
+
 def LockedMethod(fn):
   """Synchronized object access decorator.