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: NJohn Ferlan <jferlan@redhat.com>
Signed-off-by: NDaniel P. Berrangé <berrange@redhat.com>

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:

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;

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,

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 {

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)) {
 

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);

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;
             }

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;

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;
         }
     }

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,