On 02/20/2018 05:56 AM, Daniel P. Berrangé wrote:
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.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_addr.c | 6 +++--- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 4 ++++ src/libxl/libxl_conf.c | 2 +- src/qemu/qemu_command.c | 21 ++++++++++++++++----- src/qemu/qemu_domain.c | 14 +++++++++++--- src/qemu/qemu_domain_address.c | 19 +++++++++++++++++-- src/qemu/qemu_hotplug.c | 2 +- src/vbox/vbox_common.c | 13 +++++++++---- src/vmx/vmx.c | 2 +- 10 files changed, 65 insertions(+), 21 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 6422682391..4f969adc00 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c
[...]
@@ -1710,10 +1712,8 @@ virDomainUSBAddressControllerModelToPorts(virDomainControllerDefPtr cont) { int model = cont->model;
NIT: No need for @model anymore... Could just go directly to cont->model in the switch statement.
- if (model == -1) - model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI; - switch ((virDomainControllerModelUSB) 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:
Reviewed-by: John Ferlan <jferlan@redhat.com> John