11:11 a.m.
Just like qemuDomainDefaultSCSIControllerModel() before that,
split the function into an internal part and a trivial public
wrapper.
This allows us to rewrite the internal logic in the much more
compact and readable
if (condition)
return value;
if (other_condition)
return other_value;
return default_value;
style, while still implementing the usual error handling
behavior and avoiding any ambiguity caused by
VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT having value -1.
The conversion is straightforward if messy, with one notable
exception: the piix3-uhci case, which serves as our fallback,
has to be moved from the top of the function to the bottom due
the change from the previous set-and-overwrite approach to the
new return-early approach.
The use of 'else if' is eliminated completely, comments are
updated and improved.
Signed-off-by: Andrea Bolognani <abologna(a)redhat.com>
---
src/qemu/qemu_domain.c | 143 +++++++++++++++++++++++++++++------------
1 file changed, 101 insertions(+), 42 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6801a883f4..db09af496b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4157,62 +4157,121 @@ qemuDomainDefaultSCSIControllerModel(virDomainControllerModelSCSI
*model,
}
-static int
-qemuDomainDefaultUSBControllerModel(virDomainControllerModelUSB *model,
- bool autoAdded,
- const virDomainDef *def,
- virQEMUCaps *qemuCaps,
- unsigned int parseFlags)
+/* Internal. Use qemuDomainDefaultUSBControllerModel() instead */
+static virDomainControllerModelUSB
+qemuDomainDefaultUSBControllerModelInternal(bool autoAdded,
+ const virDomainDef *def,
+ virQEMUCaps *qemuCaps,
+ unsigned int parseFlags)
{
- /* Default USB controller is piix3-uhci if available. */
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
+ bool abiUpdate = !!(parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
if (ARCH_IS_S390(def->os.arch)) {
/* No default model on s390x, one has to be provided
* explicitly by the user */
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
- } else if (ARCH_IS_PPC64(def->os.arch)) {
- /* To not break migration we need to set default USB controller
- * for ppc64 to pci-ohci if we cannot change ABI of the VM.
- * The nec-usb-xhci or qemu-xhci controller is used as default
- * only for newly defined domains or devices. */
- if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) &&
- virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
- } else if ((parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) &&
- virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
- } else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) {
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
- } else {
- /* Explicitly fallback to legacy USB controller for PPC64. */
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
- }
- } else if (def->os.arch == VIR_ARCH_AARCH64) {
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_NONE;
+ }
+
+ if (ARCH_IS_PPC64(def->os.arch)) {
+ /* Newly-defined guests should use USB3 if possible */
+ if (abiUpdate && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
+ if (abiUpdate && virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
+
+ /* To preserve backwards compatibility, existing guests need to
+ * use pci-ohci (USB2) instead */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;
+
+ /* If neither USB3 nor USB2 can be used, bail */
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
+ }
+
+ if (def->os.arch == VIR_ARCH_AARCH64) {
+ /* Prefer USB3 */
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI))
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
- else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI))
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
+
+ /* If USB3 is unavailable, we should probably bail by
+ * returning VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT here
+ * instead of possibly falling back to piix3-uhci (USB1), but
+ * historically we haven't done that and starting now might
+ * cause backwards compatibility issues */
}
if (ARCH_IS_X86(def->os.arch)) {
if (qemuDomainIsQ35(def) && autoAdded) {
- /* Prefer adding a USB3 controller if supported, fall back
- * to USB2 if there is no USB3 available, and if that's
- * unavailable don't add anything.
- */
+ /* Prefer USB3 */
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI))
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
- else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI))
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
- else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_USB_EHCI1))
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1;
- else
- *model = VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI;
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NEC_USB_XHCI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI;
+
+ /* Fall back to USB2 */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_USB_EHCI1))
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1;
+
+ /* If no suitable device is available, simply avoid
+ * adding the controller.
+ *
+ * Note that this is only the case for the USB controller
+ * that gets automatically added for new guests, and only
+ * for the q35 machine type */
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
}
}
+ /* The default USB controller is piix3-uhci (USB1) if available.
+ * This choice is a fairly poor one, rooted primarily in historical
+ * reasons; thankfully, in most cases we will overwrite it with a
+ * much more reasonable value below */
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;
+
+ return VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT;
+}
+
+
+/**
+ * qemuDomainDefaultUSBControllerModel:
+ * @model: return location
+ * @autoAdded: whether this controller is being automatically added
+ * @def: domain definition
+ * @cont: controller definition, or NULL
+ * @qemuCaps: QEMU capabilities, or NULL
+ * @parseFlags: parse flags
+ *
+ * Choose a reasonable model to use for a USB controller where a
+ * specific one hasn't been provided by the user.
+ *
+ * The choice is based on a number of factors, including the guest's
+ * architecture and machine type. @qemuCaps, if provided, might be
+ * taken into consideration too.
+ *
+ * @autoAdded should be true is this is a controller that libvirt is
+ * trying to automatically add on domain creation for the user's
+ * convenience: in that case, the function might decide that the best
+ * course of action is to not add the controller after all. This
+ * decision will be communicated to the caller by setting @model
+ * to VIR_DOMAIN_CONTROLLER_MODEL_USB_DEFAULT.
+ *
+ * Not being able to come up with a value is NOT considered a
+ * failure. It's up to the caller to decide how to handle that
+ * scenario.
+ *
+ * Returns: 0 on success, <0 on failure.
+ */
+static int
+qemuDomainDefaultUSBControllerModel(virDomainControllerModelUSB *model,
+ bool autoAdded,
+ const virDomainDef *def,
+ virQEMUCaps *qemuCaps,
+ unsigned int parseFlags)
+{
+ *model = qemuDomainDefaultUSBControllerModelInternal(autoAdded, def, qemuCaps,
parseFlags);
return 0;
}
--
2.43.0