[libvirt] [PATCH 8/9] qemu: domain: Reflect USB controller model in guest XML

[Ján Tomko]

On Fri, Jul 29, 2016 at 07:46:28PM +0200, Andrea Bolognani wrote:
>When the user doesn't specify any model for a USB controller,
>we use an architecture-dependent default, but we don't reflect
>it in the guest XML.
>
>Pick the default USB controller model when parsing the guest
>XML instead of when creating the QEMU command line, so that
>our choice is saved back to disk.
>---
> src/qemu/qemu_domain.c                               | 20 ++++++++++++++++++++
> .../qemuxml2xmlout-ppc64-usb-controller.xml          |  2 +-
> .../qemuxml2xmlout-usb-controller-default-q35.xml    |  2 +-
> 3 files changed, 22 insertions(+), 2 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 9b439df..ffa7fa7 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -2573,6 +2573,26 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
>                                virDomainNumaGetNodeCount(def->numa));
>                 goto cleanup;
>             }
>+        } else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
>+                   cont->model == -1) {
>+            /* Pick a suitable default model for the USB controller if none
>+             * has been selected by the user.
>+             *
>+             * We rely on device availability instead of setting the model
>+             * unconditionally because, for some machine types, there's a
>+             * chance we will get away with using the legacy USB controller
>+             * when the relevant device is not available.
>+             *
>+             * See qemuBuildControllerDevCommandLine() */
>+            if (ARCH_IS_PPC64(def->os.arch)) {
>+                /* Default USB controller for ppc64 is pci-ohci */
>+                if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI))
>+                    cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI;

<thinking_out_loud>

So, if I understand correctly, if we left the model for PPC64 at -1:
[A] before 8156493 Fix USB model defaults for ppc64 [v1.3.1-rc1~47]
  we pass -usb on the command line, drop the controller from migration
  XML and possibly re-add it on the destination
[B] after that commit
  we pass -device pci-usb-ohci, lose that information in migration
[C] after 192a53e0 send default USB controller in xml [v1.3.5-rc1~459]
  We use -device pci-usb-ohci, passing the address to the
  destination

Migration between [A] and anything else is AFAIK broken since the
address used by -usb never matches the one we pick for the -device,
https://bugzilla.redhat.com/show_bug.cgi?id=1357468

Migration between [C] and [B] should just work as long as hotplugging
did not change the order of the devices and the controller auto-added on
the destination gets the same address as the one that was removed.

</thinking_out_loud>

Is it possible to create a -device syntax that will match the -usb
command line generated by [A] (e.g. by specifying a 0000:00.00 PCI
addr)?

This change should fix migration from [B] and [C] and back:
* model=-1 coming from those hosts will get adjusted to OHCI on parsing
* we should not format model=-1 when migrating back to those hosts


>+            } else {
>+                /* Default USB controller for anything else is piix3-uhci */
>+                if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI))
>+                    cont->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI;

For non-PPC64, migration was not broken before.

Ancient QEMUs not supporting QEMU_CAPS_PIIX3_USB_UHCI will still get
model=-1 (or not, I doubt anyone will run new libvirt with them).
Should we set it unconditionally for i440fx machines?
Everything else gets the model changed to PIIX3-UHCI.

Of course, this breaks migration to pre-0.9.x libvirt which did not
support USB controllers. I am okay with that, but you might want to get
a second opinion on that.

ACK

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160804/eaeb8e75/attachment-0001.sig>