GCC8 identified a critical flaw in the QEMU post-parse callback
They switch() based on the virDomainControllDef 'model', carefully
listing every single enum case..... but no default statement.
Unfortunately it is valid for "model" to have the valid "-1"
which does not map to any enum constant. So the lack of any
"default" statement, means when dealing with the USB controller
we mistakenly fall through to the IDE controller code.
The SCSI controller simililarly falls through to the virtio
serial controller case.
I tried just adding the obvious default case:
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index e28119e4b..3eaa22e39 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -532,6 +532,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
case VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2: /* xen only */
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_LAST:
+ default:
return 0;
}
@@ -553,6 +554,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
return pciFlags;
case VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST:
+ default:
return 0;
}
This causes many of the QEMU test cases to fail now with a message
"internal error: Cannot automatically add a new PCI bus for a device
with connect flags 00"
I could theoretically make the "default:" case return "pciFlags" when
model == -1, but this seems dubious because it feels like it is blindly
assuming that any unknown controller model is PCI based, but it would
be functionally equivalent to the current behaviour where USB accidentally
falls through to the IDE code.
Thoughts ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
