[libvirt] [PATCH] qemu: Use legacy USB on ppc64
Commit 8156493d8db9 changed libvirt so that '-device pci-ohci' would be used instead of '-usb' on ppc64 when no specific USB controller model had been specified in the guest configuration. While the device that ends up being presented by the guest is exactly the same, '-usb' causes it to be assigned to PCI address 00:00.0 while '-device pci-ohci', being subject to the regular PCI address assignment logic, will be at a different address. This PCI address mismatch breaks migration of existing guests to new libvirt versions. Luckily, when QEMU has switched its default '-usb' controller from pci-ohci to nec-usb-xhci (QEMU commit 57040d451315), it has done so without affecting older machine types, which means we can keep using '-usb' without risking guest ABI breakage. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357468 --- src/qemu/qemu_command.c | 8 +++++--- tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2e04f57..9050e77 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2827,9 +2827,11 @@ qemuBuildControllerDevCommandLine(virCommandPtr cmd, /* We're not using legacy usb controller for q35 */ if (ARCH_IS_PPC64(def->os.arch)) { - /* For ppc64 the legacy was OHCI */ - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_OHCI)) - need_legacy = true; + /* ppc64 needs to use the -usb flag in order not to break + * migration of existing guests: the legacy USB controller + * uses a PCI address that we have no way of assigning + * using the usual -device machinery */ + need_legacy = true; } else { /* For anything else, we used PIIX3_USB_UHCI */ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI)) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args index 2ec2231..bed5045 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args @@ -15,5 +15,5 @@ QEMU_AUDIO_DRV=none \ -nodefaults \ -monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \ -boot c \ --device pci-ohci,id=usb,bus=pci,addr=0x1 \ +-usb \ -device virtio-balloon-pci,id=balloon0,bus=pci,addr=0x6 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args index 251e786..2839dea 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args @@ -19,6 +19,6 @@ server,nowait \ -mon chardev=charmonitor,id=monitor,mode=readline \ -no-acpi \ -boot c \ --device pci-ohci,id=usb,bus=pci,addr=0x1 \ +-usb \ -chardev pty,id=charserial0 \ -device spapr-vty,chardev=charserial0,reg=0x30000000 -- 2.7.4
On Tue, 2016-07-19 at 16:17 +0200, Andrea Bolognani wrote:
Commit 8156493d8db9 changed libvirt so that '-device pci-ohci' would be used instead of '-usb' on ppc64 when no specific USB controller model had been specified in the guest configuration. While the device that ends up being presented by the guest is exactly the same, '-usb' causes it to be assigned to PCI address 00:00.0 while '-device pci-ohci', being subject to the regular PCI address assignment logic, will be at a different address. This PCI address mismatch breaks migration of existing guests to new libvirt versions. Luckily, when QEMU has switched its default '-usb' controller from pci-ohci to nec-usb-xhci (QEMU commit 57040d451315), it has done so without affecting older machine types, which means we can keep using '-usb' without risking guest ABI breakage. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357468 --- src/qemu/qemu_command.c | 8 +++++--- tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-)
Ján pointed out that applying this would un-break migration for ppc64 guests created before 8156493d8db9, but at the same time break migration for those created since, eg. in the last ~6 months. I'm kinda out of ideas here, so if anyone has a brilliant plan to make migration work for both old and new guests, please do share :) -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Jul 19, 2016 at 07:04:36PM +0200, Andrea Bolognani wrote:
On Tue, 2016-07-19 at 16:17 +0200, Andrea Bolognani wrote:
Commit 8156493d8db9 changed libvirt so that '-device pci-ohci' would be used instead of '-usb' on ppc64 when no specific USB controller model had been specified in the guest configuration. While the device that ends up being presented by the guest is exactly the same, '-usb' causes it to be assigned to PCI address 00:00.0 while '-device pci-ohci', being subject to the regular PCI address assignment logic, will be at a different address. This PCI address mismatch breaks migration of existing guests to new libvirt versions. Luckily, when QEMU has switched its default '-usb' controller from pci-ohci to nec-usb-xhci (QEMU commit 57040d451315), it has done so without affecting older machine types, which means we can keep using '-usb' without risking guest ABI breakage. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357468 --- src/qemu/qemu_command.c | 8 +++++--- tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-)
Ján pointed out that applying this would un-break migration for ppc64 guests created before 8156493d8db9, but at the same time break migration for those created since, eg. in the last ~6 months.
I'm kinda out of ideas here, so if anyone has a brilliant plan to make migration work for both old and new guests, please do share :)
It seems that we could solve this by just changing the logic in qemuDomainAssignDevicePCISlots() so that when it is auto-assigning addresses, it always uses 00:00.0 for the USB controller when guest arch is ppc64. Existing guests deployed from a libvirt version using -device won't be affected, because we'll have recorded a PCI address for that in the XML and continue to use that. ie the auto-allocation logic won't run for them. Existing guests deployed from a libvirt version using -usb should then get the fixed PCI address of 00:00.0 when they upgrade to the fix libvirt (assuming they didn't get run with a broken libvirt in between). Regardless though, I'm very much against any change that tales us back to using -usb. This is long since obsolete syntax we should aim to never use - bringing ppc inline with other arches using -device is very much desirable. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Wed, 2016-07-20 at 14:17 +0100, Daniel P. Berrange wrote:
Commit 8156493d8db9 changed libvirt so that '-device pci-ohci' would be used instead of '-usb' on ppc64 when no specific USB controller model had been specified in the guest configuration. While the device that ends up being presented by the guest is exactly the same, '-usb' causes it to be assigned to PCI address 00:00.0 while '-device pci-ohci', being subject to the regular PCI address assignment logic, will be at a different address. This PCI address mismatch breaks migration of existing guests to new libvirt versions. Luckily, when QEMU has switched its default '-usb' controller from pci-ohci to nec-usb-xhci (QEMU commit 57040d451315), it has done so without affecting older machine types, which means we can keep using '-usb' without risking guest ABI breakage. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1357468 --- src/qemu/qemu_command.c | 8 +++++--- tests/qemuxml2argvdata/qemuxml2argv-ppc64-usb-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pseries-usb-default.args | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) Ján pointed out that applying this would un-break migration for ppc64 guests created before 8156493d8db9, but at the same time break migration for those created since, eg. in
On Tue, 2016-07-19 at 16:17 +0200, Andrea Bolognani wrote: the last ~6 months. I'm kinda out of ideas here, so if anyone has a brilliant plan to make migration work for both old and new guests, please do share :) It seems that we could solve this by just changing the logic in qemuDomainAssignDevicePCISlots() so that when it is auto-assigning addresses, it always uses 00:00.0 for the USB controller when guest arch is ppc64. Existing guests deployed from a libvirt version using -device won't be affected, because we'll have recorded a PCI address for that in the XML and continue to use that. ie the auto-allocation logic won't run for them. Existing guests deployed from a libvirt version using -usb should
On Tue, Jul 19, 2016 at 07:04:36PM +0200, Andrea Bolognani wrote: then get the fixed PCI address of 00:00.0 when they upgrade to the fix libvirt (assuming they didn't get run with a broken libvirt in between).
Mh, that could actually work! Thanks :) I'll try to cook up a patch tomorrow.
Regardless though, I'm very much against any change that tales us back to using -usb. This is long since obsolete syntax we should aim to never use - bringing ppc inline with other arches using -device is very much desirable.
Agreed - that was the reasoning behind the original, sadly flawed, attempt to get rid of it. -- Andrea Bolognani / Red Hat / Virtualization
On Wed, 2016-07-20 at 16:09 +0200, Andrea Bolognani wrote:
It seems that we could solve this by just changing the logic in qemuDomainAssignDevicePCISlots() so that when it is auto-assigning addresses, it always uses 00:00.0 for the USB controller when guest arch is ppc64. Existing guests deployed from a libvirt version using -device won't be affected, because we'll have recorded a PCI address for that in the XML and continue to use that. ie the auto-allocation logic won't run for them. Existing guests deployed from a libvirt version using -usb should then get the fixed PCI address of 00:00.0 when they upgrade to the fix libvirt (assuming they didn't get run with a broken libvirt in between). Mh, that could actually work! Thanks :) I'll try to cook up a patch tomorrow.
Unfortunately Martin pointed out a problem with this plan: even though they're not actually using it, old guests have been assigned a PCI address for the USB controller. Which means we can't tell whether <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller is an old guest that should be assigned the 00:00.0 address or a new guest that should keep using the 00:03.0 address :( -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Jul 21, 2016 at 04:56:56PM +0200, Andrea Bolognani wrote:
On Wed, 2016-07-20 at 16:09 +0200, Andrea Bolognani wrote:
It seems that we could solve this by just changing the logic in qemuDomainAssignDevicePCISlots() so that when it is auto-assigning addresses, it always uses 00:00.0 for the USB controller when guest arch is ppc64. Existing guests deployed from a libvirt version using -device won't be affected, because we'll have recorded a PCI address for that in the XML and continue to use that. ie the auto-allocation logic won't run for them. Existing guests deployed from a libvirt version using -usb should then get the fixed PCI address of 00:00.0 when they upgrade to the fix libvirt (assuming they didn't get run with a broken libvirt in between). Mh, that could actually work! Thanks :) I'll try to cook up a patch tomorrow.
Unfortunately Martin pointed out a problem with this plan: even though they're not actually using it, old guests have been assigned a PCI address for the USB controller. Which means we can't tell whether
<controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller
is an old guest that should be assigned the 00:00.0 address or a new guest that should keep using the 00:03.0 address :(
If we revert 8156493d8db95de91dd9ace743df0fd4dff98281 we'll go back to using -usb and the PCI addr reported in the XML is just completely wrong vs what's actually being used. If the user requests an explicit addr for the USB controller, we'll ignore it and that could cause boot failure if the user has requested something else in addr 00:00.0 I think we have no choice but to stick with using -device, because that is clearly the preferred syntax long term, and it allows us to actually honour the addresses requested in the XML, which the original code was not doing. IOW the old code < 1.3.1 using -usb was clearly broken, so I don't think reverting is an option we can take. Furthermore reverting it will instead break anyone with libvirt 1.3.1 -> 2.0.0 So no matter what we do some portion of users are screwed. On balance I think users of libvirt < 1.3.1 will just have to take the pain. We can document a procedure to minimize that pain. Specifically if you have libvirt < 1.3.1 and want to upgrade them 1. Use virsh save-image-edit to change the XML for all existing saved images, and fix the PCI address slot to be 0 instead of 3. This should allow new libvirt to load the image, since itsXML will now reflect the reality of what was used. 2. For any running guests edit /var/run/libvirt/qemu/*.xml to again fix the PCI address slot from 3 to 0. Then restart libvirtd so it reloads its config. This should then allow those running guests to be live migrated Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On Thu, 2016-07-21 at 16:31 +0100, Daniel P. Berrange wrote:
It seems that we could solve this by just changing the logic in qemuDomainAssignDevicePCISlots() so that when it is auto-assigning addresses, it always uses 00:00.0 for the USB controller when guest arch is ppc64. Existing guests deployed from a libvirt version using -device won't be affected, because we'll have recorded a PCI address for that in the XML and continue to use that. ie the auto-allocation logic won't run for them. Existing guests deployed from a libvirt version using -usb should then get the fixed PCI address of 00:00.0 when they upgrade to the fix libvirt (assuming they didn't get run with a broken libvirt in between). Mh, that could actually work! Thanks :) I'll try to cook up a patch tomorrow. Unfortunately Martin pointed out a problem with this plan: even though they're not actually using it, old guests have been assigned a PCI address for the USB controller. Which means we can't tell whether <controller type='usb' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> </controller is an old guest that should be assigned the 00:00.0 address or a new guest that should keep using the 00:03.0 address :( If we revert 8156493d8db95de91dd9ace743df0fd4dff98281 we'll go back to using -usb and the PCI addr reported in the XML is just completely wrong vs what's actually being used. If the user requests an explicit addr for the USB controller, we'll ignore it and that could cause boot failure if the user has requested something else in addr 00:00.0 I think we have no choice but to stick with using -device, because that is clearly the preferred syntax long term, and it allows us to actually honour the addresses requested in the XML, which the original code was not doing. IOW the old code < 1.3.1 using -usb was clearly broken, so I don't think reverting is an option we can take. Furthermore reverting it will instead break anyone with libvirt 1.3.1 -> 2.0.0
It was actually more broken than I remembered - it used -usb on ppc64 because the condition for not using it was to have PIIX3, which is IIUC x86-specific hardware.
So no matter what we do some portion of users are screwed. On balance I think users of libvirt < 1.3.1 will just have to take the pain. We can document a procedure to minimize that pain. Specifically if you have libvirt < 1.3.1 and want to upgrade them 1. Use virsh save-image-edit to change the XML for all existing saved images, and fix the PCI address slot to be 0 instead of 3. This should allow new libvirt to load the image, since itsXML will now reflect the reality of what was used.
I tried this with libvirt-1.2.17-13.el7.ppc64le. The save image contains just <controller type='usb' index='0'/> If you add a PCI address that's not 00:00.0, it will be ignored; 00:00.0 will cause save-image-edit to fail with XML error: Invalid PCI address 0000:00:00, at least one of domain, bus, or slot must be > 0
2. For any running guests edit /var/run/libvirt/qemu/*.xml to again fix the PCI address slot from 3 to 0. Then restart libvirtd so it reloads its config. This should then allow those running guests to be live migrated
This fails with the same error as above; moreover, the guest will disappear. The QEMU process will still be running, of course. I'm afraid the only way for people running libvirt < 1.3.1 to make their guests live migratable is to tweak the XML so that it looks like <controller type='usb' model='pci-ohci' ... and shutdown the guests. When they are started again, they will get -device pci-ohci,... on the command line and live migration (back and forth) will work as expected. I'd be extremely happy if I were proven wrong, though :) -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Daniel P. Berrange