conf: add enum constants for default controller models

The controller model is slightly unusual in that the default value is
-1, not 0. As a result the default value is not covered by any of the
existing enum cases. This in turn means that any switch() statements
that think they have covered all cases, will in fact not match the
default value at all. In the qemuDomainDeviceCalculatePCIConnectFlags()
method this has caused a serious mistake where we fallthrough from the
SCSI controller case, to the VirtioSerial controller case, and from
the USB controller case to the IDE controller case.

By adding explicit enum constant starting at -1, we can ensure switches
remember to handle the default case.

Reviewed-by: John Ferlan <jferlan@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 6422682391b7f56915c72feefcde69954de09a58..f70d740978a84bd0561f2c59272e0697490616e1 100644 (file)
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -39,6 +39,7 @@ virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model)
      * the equivalent VIR_PCI_CONNECT_TYPE_*.
      */
     switch (model) {
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
@@ -344,6 +345,7 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
         bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
         break;
 
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "%s", _("PCI controller model was not set correctly"));
@@ -1708,12 +1710,8 @@ virDomainUSBAddressSetFree(virDomainUSBAddressSetPtr addrs)
 static size_t
 virDomainUSBAddressControllerModelToPorts(virDomainControllerDefPtr cont)
 {
-    int model = cont->model;
-
-    if (model == -1)
-        model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
-
-    switch ((virDomainControllerModelUSB) model) {
+    switch ((virDomainControllerModelUSB) cont->model) {
+    case VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT:
     case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI:
     case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI:
     case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI:

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 362cab179898a6787514b0ce01855e72cd31e088..68a8d390168c804fb0e8099b51634b177ce0d8fa 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4999,7 +4999,7 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
      * been added in AddImplicitDevices, after we've done the per-device
      * post-parse. */
     for (i = 0; i < def->ncontrollers; i++) {
-        if (def->controllers[i]->model == -1 &&
+        if (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT &&
             def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
             virDomainDeviceDef device = {
                 .type = VIR_DOMAIN_DEVICE_CONTROLLER,
@@ -10197,6 +10197,7 @@ virDomainControllerDefParseXML(virDomainXMLOptionPtr xmlopt,
         case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
         case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS:
         case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS:
+        case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
         case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
             /* Other controller models don't require extra checks */
             break;

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 117efb1d1d6a457c66f0e975d3f0bac1e58aa2d3..2350926e5bdc2cb0042080c647f15917ef0399ff 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -685,6 +685,7 @@ typedef enum {
 
 
 typedef enum {
+    VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT = -1,
     VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT,
     VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT,
     VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
@@ -714,6 +715,7 @@ typedef enum {
 } virDomainControllerPCIModelName;
 
 typedef enum {
+    VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT = -1,
     VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO,
     VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC,
     VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC,
@@ -727,6 +729,7 @@ typedef enum {
 } virDomainControllerModelSCSI;
 
 typedef enum {
+    VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT = -1,
     VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI,
     VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI,
     VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI,
@@ -746,6 +749,7 @@ typedef enum {
 } virDomainControllerModelUSB;
 
 typedef enum {
+    VIR_DOMAIN_CONTROLLER_MODEL_IDE_DEFAULT = -1,
     VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX3,
     VIR_DOMAIN_CONTROLLER_MODEL_IDE_PIIX4,
     VIR_DOMAIN_CONTROLLER_MODEL_IDE_ICH6,

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 970cff205b6df8140bf0dba5abb11347c0762afd..0c5de344def6061acfa1528a4569e5f1763994bf 100644 (file)
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1894,7 +1894,7 @@ libxlMakeUSBController(virDomainControllerDefPtr controller,
     if (controller->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
         return -1;
 
-    if (controller->model == -1) {
+    if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT) {
         usbctrl->version = 2;
         usbctrl->type = LIBXL_USBCTRL_TYPE_QUSB;
     } else {

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d0b9b5246b04620fba23aeefb5b7947d37a65922..34dda232883ba3865dc48266e60669c053f019d1 100644 (file)
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2555,7 +2555,7 @@ qemuBuildUSBControllerDevStr(virDomainControllerDefPtr def,
 
     model = def->model;
 
-    if (model == -1) {
+    if (model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        "%s", _("no model provided for USB controller"));
         return -1;
@@ -2669,10 +2669,16 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
-        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("Unsupported controller model: %s"),
                            virDomainControllerModelSCSITypeToString(def->model));
+            goto error;
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unexpected SCSI controller model %d"),
+                           def->model);
+            goto error;
         }
         virBufferAsprintf(&buf, ",id=%s", def->info.alias);
         break;
@@ -2777,9 +2783,14 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef,
                 virBufferAsprintf(&buf, ",numa_node=%d", pciopts->numaNode);
             break;
         case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("Unsupported PCI Express root controller"));
+            goto error;
+        case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
         case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                           _("wrong function called"));
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unexpected PCI controller model %d"),
+                           def->model);
             goto error;
         }
         break;
@@ -2912,7 +2923,7 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd,
             }
 
             if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
-                cont->model == -1 &&
+                cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT &&
                 !qemuDomainIsQ35(def) &&
                 !qemuDomainIsVirt(def)) {
 

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5d0a18343e09f3318a2fbc7bf8e0f4461b669f09..593b9d0fb51ff753493fbc818ad62ae3acbd932d 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4139,11 +4139,16 @@ qemuDomainCheckSCSIControllerModel(virQEMUCapsPtr qemuCaps,
     case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_AUTO:
     case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_BUSLOGIC:
     case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
-    case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("Unsupported controller model: %s"),
                        virDomainControllerModelSCSITypeToString(model));
         return false;
+    case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unexpected SCSI controller model %d"),
+                       model);
+        return false;
     }
 
     return true;
@@ -4230,6 +4235,7 @@ qemuDomainDeviceDefValidateControllerSCSI(const virDomainControllerDef *controll
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VMPVSCSI:
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
             break;
     }
@@ -4277,6 +4283,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
 
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
         break;
     }
@@ -4509,6 +4516,7 @@ qemuDomainDeviceDefValidateControllerPCI(const virDomainControllerDef *controlle
 
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
         break;
     }
 
@@ -4861,7 +4869,7 @@ qemuDomainControllerDefPostParse(virDomainControllerDefPtr cont,
         break;
 
     case VIR_DOMAIN_CONTROLLER_TYPE_USB:
-        if (cont->model == -1 && qemuCaps) {
+        if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT && qemuCaps) {
             /* Pick a suitable default model for the USB controller if none
              * has been selected by the user and we have the qemuCaps for
              * figuring out which contollers are supported.
@@ -5879,7 +5887,7 @@ qemuDomainDefFormatBufInternal(virQEMUDriverPtr driver,
          */
         if (ARCH_IS_X86(def->os.arch) && qemuDomainIsI440FX(def) &&
             usb && usb->idx == 0 &&
-            (usb->model == -1 ||
+            (usb->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT ||
              usb->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI)) {
             VIR_DEBUG("Removing default USB controller from domain '%s'"
                       " for migration compatibility", def->name);

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index e28119e4b1c326cb0bb6f14d2a2dbb24fdba6be6..dd3e703ca54500cb043eee7c628b284a388e5474 100644 (file)
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -513,6 +513,15 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
 
         case VIR_DOMAIN_CONTROLLER_TYPE_USB:
             switch ((virDomainControllerModelUSB) cont->model) {
+            case VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT:
+                /* qemuDomainControllerDefPostParse should have
+                 * changed 'model' to an explicit USB model in
+                 * most cases. Since we're still on the default
+                 * though, we must be going to use "-usb", which
+                 * is assumed to be a PCI default
+                 */
+                return pciFlags;
+
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
             case VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI:
                 return pcieFlags;
@@ -540,6 +549,9 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
 
         case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
             switch ((virDomainControllerModelSCSI) cont->model) {
+            case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+                return 0;
+
             case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
                 return virtioFlags;
 
@@ -1275,7 +1287,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
              addr->function == 1) ||
             (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->idx == 0 &&
              (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
-              cont->model == -1) && addr->function == 2)) {
+              cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT) &&
+             addr->function == 2)) {
             /* Note the check for nbuses > 0 - if there are no PCI
              * buses, we skip this check. This is a quirk required for
              * some machinetypes such as s390, which pretend to have a
@@ -1435,7 +1448,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
         } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
                    cont->idx == 0 &&
                    (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
-                    cont->model == -1)) {
+                    cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT)) {
             if (virDeviceInfoPCIAddressPresent(&cont->info)) {
                 if (cont->info.addr.pci.domain != 0 ||
                     cont->info.addr.pci.bus != 0 ||
@@ -2165,6 +2178,7 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont,
             *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE;
         break;
     case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+    case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
     case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
         break;
     }
@@ -2552,6 +2566,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
             case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
             case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+            case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
             case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
                 break;
             }

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3c87d737453ad1ab2e298e4f146c4cde0ffc50eb..04f4f689a90745338c88180f5791b3da4d5857f9 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -612,7 +612,7 @@ qemuDomainFindOrCreateSCSIDiskController(virQEMUDriverPtr driver,
         return NULL;
     cont->type = VIR_DOMAIN_CONTROLLER_TYPE_SCSI;
     cont->idx = controller;
-    if (model == -1)
+    if (model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT)
         cont->model = qemuDomainGetSCSIControllerModel(vm->def, cont, priv->qemuCaps);
     else
         cont->model = model;

diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index aed14f02d70a69883ac40633413a9ccee69a5c86..07f43087847c5cdc1b30dc2419a3445380ee6170 100644 (file)
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -410,12 +410,17 @@ vboxSetStorageController(virDomainControllerDefPtr controller,
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_IBMVSCSI:
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
         case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSISAS1078:
-        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("The vbox driver does not support %s SCSI "
                              "controller model"),
                            virDomainControllerModelSCSITypeToString(controller->model));
             goto cleanup;
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+        case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unexpected SCSI controller model %d"),
+                           controller->model);
+            goto cleanup;
         }
     /* libvirt ide model => vbox ide model */
     } else if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE) {
@@ -433,10 +438,10 @@ vboxSetStorageController(virDomainControllerDefPtr controller,
 
             break;
         case VIR_DOMAIN_CONTROLLER_MODEL_IDE_LAST:
+        case VIR_DOMAIN_CONTROLLER_MODEL_IDE_DEFAULT:
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("The vbox driver does not support %s IDE "
-                             "controller model"),
-                             virDomainControllerModelIDETypeToString(controller->model));
+                           _("Unexpected IDE controller model %d"),
+                           controller->model);
             goto cleanup;
         }
     }

diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index 7a749f93ab6afc3bf49893e1e2b3f48bef0f6f28..1d13ab03db4665df9582d1f1f808db29001f146f 100644 (file)
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1169,7 +1169,7 @@ virVMXHandleLegacySCSIDiskDriverName(virDomainDefPtr def,
         return -1;
     }
 
-    if (controller->model == -1) {
+    if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT) {
         controller->model = model;
     } else if (controller->model != model) {
         virReportError(VIR_ERR_INTERNAL_ERROR,