10:43 a.m.
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 fallthough 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.
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
src/conf/domain_addr.c | 5 +++++
src/conf/domain_conf.c | 1 +
src/conf/domain_conf.h | 4 ++++
src/qemu/qemu_command.c | 17 ++++++++++++++---
src/qemu/qemu_domain.c | 10 +++++++++-
src/qemu/qemu_domain_address.c | 22 ++++++++++++++++++++++
src/vbox/vbox_common.c | 13 +++++++++----
7 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 6422682391..df19c6be1a 100644
--- 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,9 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT:
+ break;
+
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("PCI controller model was not set
correctly"));
@@ -1746,6 +1750,7 @@ virDomainUSBAddressControllerModelToPorts(virDomainControllerDefPtr
cont)
return cont->opts.usbopts.ports;
return 8;
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST:
break;
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index fb732a0c2a..abc3332377 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10196,6 +10196,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 6cd81ef2de..78b57aa6d4 100644
--- 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/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6c73cd7bfe..2a75a169c2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2671,10 +2671,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;
@@ -2771,9 +2777,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-E 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;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 178ec24ae7..e114f5dfcf 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4098,11 +4098,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;
@@ -4189,6 +4194,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;
}
@@ -4236,6 +4242,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;
}
@@ -4468,6 +4475,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;
}
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index e28119e4b1..de565dbf74 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -513,6 +513,16 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_CONTROLLER_TYPE_USB:
switch ((virDomainControllerModelUSB) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT:
+ /* XXX it isn't clear that this is the right
+ * thing to do here. We probably ought to
+ * return 0, but that breaks test suite right
+ * now because we call this method before we
+ * have turned the _DEFAULT case into a
+ * specific choice
+ */
+ return pciFlags;
+
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI:
return pcieFlags;
@@ -540,6 +550,16 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
switch ((virDomainControllerModelSCSI) cont->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_DEFAULT:
+ /* XXX it isn't clear that this is the right
+ * thing to do here. We probably ought to
+ * return 0, but that breaks test suite right
+ * now because we call this method before we
+ * have turned the _DEFAULT case into a
+ * specific choice
+ */
+ return pciFlags;
+
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_VIRTIO_SCSI:
return virtioFlags;
@@ -2165,6 +2185,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 +2573,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/vbox/vbox_common.c b/src/vbox/vbox_common.c
index aed14f02d7..07f4308784 100644
--- 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;
}
}
--
2.14.3