Re: [libvirt] [PATCH v2 01/42] 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 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&gt; --- 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

@@ -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/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

@@ -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_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;