[libvirt] [PATCH] qemu: Add USB and memory balloon by default for aarch64/virt guests
Basically all existing guest types, regardless of the architectur, get both a USB controller and a virtio memory balloon by default. s390 guests are an exception, for the very good reason that they don't support USB at all; the other exception is aarch64/virt guests, but in the latter case isn't a compelling reason for them to deviate from the widely adopted convention, especially since: * x86/q35 guests, which aarch64/virt guests are for the most part identical to, add these devices by default; * it's trivial to opt out of both default devices by setting model='none'; * higher level applications such as Nova expect at least the USB controller to be present. So add it by default for newly-defined guests. Existing guests will, of course, be left unchanged. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1538637 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_domain.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6b4bd3cca..f1c3b3d1e 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2630,7 +2630,8 @@ qemuDomainDefAddImplicitInputDevice(virDomainDef *def) static int qemuDomainDefAddDefaultDevices(virDomainDefPtr def, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + unsigned int parseFlags) { bool addDefaultUSB = true; int usbModel = -1; /* "default for machinetype" */ @@ -2680,10 +2681,33 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_ARMV7L: case VIR_ARCH_AARCH64: - addDefaultUSB = false; - addDefaultMemballoon = false; - if (qemuDomainIsVirt(def)) + if (qemuDomainIsVirt(def)) { + /* All mach-virt guests get a PCIe Root, if supported by + * the QEMU binary */ addPCIeRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_OBJECT_GPEX); + } + + if (qemuDomainIsVirt(def) && + parseFlags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE) { + /* In addition to PCIe Root, newly-defined mach-virt guests + * also get a couple more devices so that they're more similar + * to guests on other architectures, notably x86/q35: + * + * 1) a USB3 controller, if supported by the QEMU binary; + * 2) a virtio memory balloon, as per the defaults defined + * above. + */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QEMU_XHCI)) + usbModel = VIR_DOMAIN_CONTROLLER_MODEL_USB_QEMU_XHCI; + else + addDefaultUSB = false; + } else { + /* Other ARM guests (and existing mach-virt guests, in order + * to preserve guest ABI compatibility) don't get a PCIe Root, + * a USB controller or a memory balloon */ + addDefaultUSB = false; + addDefaultMemballoon = false; + } break; case VIR_ARCH_PPC64: @@ -3187,7 +3211,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, goto cleanup; } - if (qemuDomainDefAddDefaultDevices(def, qemuCaps) < 0) + if (qemuDomainDefAddDefaultDevices(def, qemuCaps, parseFlags) < 0) goto cleanup; if (qemuCanonicalizeMachine(def, qemuCaps) < 0) -- 2.14.3
On Thu, Jan 25, 2018 at 05:45:51PM +0100, Andrea Bolognani wrote:
Basically all existing guest types, regardless of the architectur, get both a USB controller and a virtio memory balloon by default.
s390 guests are an exception, for the very good reason that they don't support USB at all; the other exception is aarch64/virt guests, but in the latter case isn't a compelling reason for them to deviate from the widely adopted convention, especially since:
* x86/q35 guests, which aarch64/virt guests are for the most part identical to, add these devices by default; * it's trivial to opt out of both default devices by setting model='none';
Except that this requires code changes to downstream applications to actually do this now, otherwise guests that they were expecting to not have USB for, now suddenly get it.
* higher level applications such as Nova expect at least the USB controller to be present.
This doesn't really help nova in practice, because it needs to operate correctly with pre-existing libbvirt releases, and even on x86 it should not be relying on the default USB1 controller, but rather adding a USB2 or USB3 controller.
So add it by default for newly-defined guests. Existing guests will, of course, be left unchanged.
That is still harmful, because an existing mgmt application release that runs on new libvirt has its guest configs suddenly changed, especially if using transient guests.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1538637
I consider that bug wontfix. It is just exchanging one set of problems for a different set of problems. 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 :|
On Thu, 2018-01-25 at 16:58 +0000, Daniel P. Berrangé wrote:
On Thu, Jan 25, 2018 at 05:45:51PM +0100, Andrea Bolognani wrote:
Basically all existing guest types, regardless of the architectur, get both a USB controller and a virtio memory balloon by default.
s390 guests are an exception, for the very good reason that they don't support USB at all; the other exception is aarch64/virt guests, but in the latter case isn't a compelling reason for them to deviate from the widely adopted convention, especially since:
* x86/q35 guests, which aarch64/virt guests are for the most part identical to, add these devices by default; * it's trivial to opt out of both default devices by setting model='none';
Except that this requires code changes to downstream applications to actually do this now, otherwise guests that they were expecting to not have USB for, now suddenly get it.
If an application breaks based on the USB controller being either present or not present, then they shouldn't be relying on libvirt's default but rather explicitly opt either in or out. That said, I expect breakages caused by adding a controller to be way less likely than those caused by removing one.
* higher level applications such as Nova expect at least the USB controller to be present.
This doesn't really help nova in practice, because it needs to operate correctly with pre-existing libbvirt releases, and even on x86 it should not be relying on the default USB1 controller, but rather adding a USB2 or USB3 controller.
Right. Nova should still be fixed to explicitly opt in to the USB controller regardless of libvirt's defaults, as per the above.
So add it by default for newly-defined guests. Existing guests will, of course, be left unchanged.
That is still harmful, because an existing mgmt application release that runs on new libvirt has its guest configs suddenly changed, especially if using transient guests.
We don't offer any guarantees when it comes to what new guests will look like, though, only that we won't alter existing ones. So again, any application relying on the presence or lack of a specific controller or device should explicitly declare so.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1538637
I consider that bug wontfix. It is just exchanging one set of problems for a different set of problems.
As mentioned above, any application that breaks because of a couple of added (not removed) controllers or devices to newly-defined guests will probably break in fairly interesting ways more often than not. And I believe there is value in keeping x86_64/q35 and aarch64/virt guests closely aligned, so I still think we want to merge this or an equivalent change. -- Andrea Bolognani / Red Hat / Virtualization
W dniu 29.01.2018 o 13:04, Andrea Bolognani pisze:
This doesn't really help nova in practice, because it needs to operate correctly with pre-existing libbvirt releases, and even on x86 it should not be relying on the default USB1 controller, but rather adding a USB2 or USB3 controller.
Right. Nova should still be fixed to explicitly opt in to the USB controller regardless of libvirt's defaults, as per the above.
I wrote a patch for Nova to do that [1]. Have to test. But the problem is that Nova creates XML with domain definition and give it to libvirt. Which expects 'qemu-xhci' or 'nec-xhci' for XHCI USB Host. If I want to play nice I have to go for 'nec-xhci' because it works with Qemu 2.8 while (much better) 'qemu-xhci' requires Qemu 2.10 version. What IMHO libvirt should support is "generic-xhci" controller which would be 'qemu' or 'nec' as libvirt knows which Qemu version it deals with and I have no idea is there a way to find that data in higher layers (like Nova). 1. https://review.openstack.org/#/c/538003/ Software compatibility is a bitch. Layering does not make it easier. In Nova code I can not do "exec('qemu-system-aarch64 --version') as 'nova-compute' container does not even have that binary installed - in 'kolla' based deployments it exists in 'nova-libvirt' image. In Nova I do not know is 'qemu-xhci' available as it is neither in 'virsh capabilities' nor 'virsh domcapabilities' output. In Nova I can not say "libvirt: please give me best-available xhci" as there is no way for it. And even if it gets added then I can not use it because people may not have latest one. In Nova I prefer to add 'nec-xhci' usb host controller to be sure that I will get working usb without checking which libvirt/qemu versions I have.
[re-added danpb to CC] On Tue, 2018-01-30 at 22:48 +0100, Marcin Juszkiewicz wrote:
W dniu 29.01.2018 o 13:04, Andrea Bolognani pisze:
This doesn't really help nova in practice, because it needs to operate correctly with pre-existing libbvirt releases, and even on x86 it should not be relying on the default USB1 controller, but rather adding a USB2 or USB3 controller. Right. Nova should still be fixed to explicitly opt in to the USB controller regardless of libvirt's defaults, as per the above.
I wrote a patch for Nova to do that [1]. Have to test.
But the problem is that Nova creates XML with domain definition and give it to libvirt. Which expects 'qemu-xhci' or 'nec-xhci' for XHCI USB Host.
If I want to play nice I have to go for 'nec-xhci' because it works with Qemu 2.8 while (much better) 'qemu-xhci' requires Qemu 2.10 version.
What IMHO libvirt should support is "generic-xhci" controller which would be 'qemu' or 'nec' as libvirt knows which Qemu version it deals with and I have no idea is there a way to find that data in higher layers (like Nova).
libvirt will already (at least for ppc64/pseries, aarch64/virt and x86_64/q35 guests) automatically pick qemu-xhci over nec-xhci or USB2 controllers if the QEMU binary supports it. You don't need to specify the model at all, just add <controller type='usb'/> to the XML and you'll get an optimal default; that's what I meant when I talked about "opting in" above. For x86/pc guests the default is still USB1 IIRC, which we might want to change to USB2 instead - you're unlikely to want USB3 in that case because of guest OS compatibility reasons. Still, having to special-case a single machine type is better than trying to figure out the model yourself every single time. Some of the above was introduced fairly recently or has not even made it into a release yet, so if you need your application to work with older libvirt releases then I see two options: * code the model selection up yourself. As you mentioned above, there are a number of factors that make this quite tricky; * live with the fact that your guests might end up with nec-xhci or even a USB1 controller until libvirt has been updated to a recent enough version. Your guests will still get *some* USB controller with the above, so USB devices will work. Personally I'd go for the latter, but it really depends on your needs and constraints. -- Andrea Bolognani / Red Hat / Virtualization
On Mon, Jan 29, 2018 at 01:04:33PM +0100, Andrea Bolognani wrote:
On Thu, 2018-01-25 at 16:58 +0000, Daniel P. Berrangé wrote:
On Thu, Jan 25, 2018 at 05:45:51PM +0100, Andrea Bolognani wrote:
Basically all existing guest types, regardless of the architectur, get both a USB controller and a virtio memory balloon by default.
s390 guests are an exception, for the very good reason that they don't support USB at all; the other exception is aarch64/virt guests, but in the latter case isn't a compelling reason for them to deviate from the widely adopted convention, especially since:
* x86/q35 guests, which aarch64/virt guests are for the most part identical to, add these devices by default; * it's trivial to opt out of both default devices by setting model='none';
Except that this requires code changes to downstream applications to actually do this now, otherwise guests that they were expecting to not have USB for, now suddenly get it.
If an application breaks based on the USB controller being either present or not present, then they shouldn't be relying on libvirt's default but rather explicitly opt either in or out.
It is not merely the mgmt application that may break, but the guest OS inside too. When we suddenly expose new hardware to a guest that was not previously present you can certainly trigger latent problems in the guest OS. It could slow boot at a key phase, or trigger loading of bad drivers, or any number of other things that can occurr when you change hardware visible to an OS. Even exposing hardware that has no guest support at all can be problematic. The reason we had to add memballoon model=none support originally was that users & tools were not happy with Windows device manager showing an "unknown" device with no driver present.
So add it by default for newly-defined guests. Existing guests will, of course, be left unchanged.
That is still harmful, because an existing mgmt application release that runs on new libvirt has its guest configs suddenly changed, especially if using transient guests.
We don't offer any guarantees when it comes to what new guests will look like, though, only that we won't alter existing ones.
As long as the machine type hasn't changed, the new guests should get exactly the same hardware as previosly deployed guests had. When we've violated that in the past it has caused problems. We have knowingly change this in the past when we believed there were no significant production users (eg when aarch64 switched to PCI instread of MMIO by default, or we changed controller setup on q35). In general we must not do these kind of things on stuff that is expected to be in active use. So I stil consider this change to be something we should not do. It is easy to fix Nova to setup USB if it desires it, so there's no blocking item that requires us to do this in libvirt. 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 :|
W dniu 01.02.2018 o 13:54, Daniel P. Berrangé pisze:
So I stil consider this change to be something we should not do.
As a person who started whole discussion I fully agree.
It is easy to fix Nova to setup USB if it desires it, so there's no blocking item that requires us to do this in libvirt. Patch to Nova is in review [1] and it simply adds usb host controller (default type) on !x86(-64) architectures.
On Thu, 2018-02-01 at 12:54 +0000, Daniel P. Berrangé wrote:
If an application breaks based on the USB controller being either present or not present, then they shouldn't be relying on libvirt's default but rather explicitly opt either in or out.
It is not merely the mgmt application that may break, but the guest OS inside too. When we suddenly expose new hardware to a guest that was not previously present you can certainly trigger latent problems in the guest OS. It could slow boot at a key phase, or trigger loading of bad drivers, or any number of other things that can occurr when you change hardware visible to an OS.
Note that I'm not advocating adding controllers or any other hardware to *existing* guests - that would clearly be a guest ABI breakage and thus Extremely Bad™. For newly-defined guests, however, none of the above applies AFAICT.
Even exposing hardware that has no guest support at all can be problematic. The reason we had to add memballoon model=none support originally was that users & tools were not happy with Windows device manager showing an "unknown" device with no driver present.
All guest OSs you might want to run on aarch64/virt support both USB3 and virtio-memballoon AFAIK, so this shouldn't be an issue either.
We don't offer any guarantees when it comes to what new guests will look like, though, only that we won't alter existing ones.
As long as the machine type hasn't changed, the new guests should get exactly the same hardware as previosly deployed guests had. When we've violated that in the past it has caused problems. We have knowingly change this in the past when we believed there were no significant production users (eg when aarch64 switched to PCI instread of MMIO by default, or we changed controller setup on q35). In general we must not do these kind of things on stuff that is expected to be in active use.
I think that's too strict a policy, one that we have very little chance of actually being able to enforce. We certainly haven't in the past, eg. any time we started picking a better controller or device by default, or tweaked our PCI device placement algorithm, knowing that existing guests had the model and address stored in the XML and thus would not be affected. If a management application or a user is completely reliant on guests presenting a very specific set of devices or address allocation, then they should be passing all those information to libvirt in the first place.
So I stil consider this change to be something we should not do. It is easy to fix Nova to setup USB if it desires it, so there's no blocking item that requires us to do this in libvirt.
It's not really blocking anything, because of course there are ways around it and, as mentioned, requesting an USB controller explicitly is always better than assuming libvirt will provide you with one. Still, it's a pointless difference between the mostly identical x86_64/q35 and aarch64/virt, so it would be nice to get rid of it. But I guess I'm to blame for not realizing earlier :) -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Feb 01, 2018 at 02:44:03PM +0100, Andrea Bolognani wrote:
On Thu, 2018-02-01 at 12:54 +0000, Daniel P. Berrangé wrote:
If an application breaks based on the USB controller being either present or not present, then they shouldn't be relying on libvirt's default but rather explicitly opt either in or out.
It is not merely the mgmt application that may break, but the guest OS inside too. When we suddenly expose new hardware to a guest that was not previously present you can certainly trigger latent problems in the guest OS. It could slow boot at a key phase, or trigger loading of bad drivers, or any number of other things that can occurr when you change hardware visible to an OS.
Note that I'm not advocating adding controllers or any other hardware to *existing* guests - that would clearly be a guest ABI breakage and thus Extremely Bad™. For newly-defined guests, however, none of the above applies AFAICT.
There's no practical way to distinguish an existing guest from a new guest being provisioned. With transient domains they are one & the same. Even with persistent guests the distinction is far from clear. 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 :|
On Thu, 2018-02-01 at 13:52 +0000, Daniel P. Berrangé wrote:
Note that I'm not advocating adding controllers or any other hardware to *existing* guests - that would clearly be a guest ABI breakage and thus Extremely Bad™. For newly-defined guests, however, none of the above applies AFAICT.
There's no practical way to distinguish an existing guest from a new guest being provisioned. With transient domains they are one & the same. Even with persistent guests the distinction is far from clear.
We have VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, which is only passed in when the guest is new - or the user has updated the XML themselves, in which case all bets are off when it comes to guest ABI stability anyway. There are cases, when importing an existing OS image as opposed to installing from scratch, where scenarios like the ones you described might show up, but again I don't think it's realistic to expect all guests to have the same exact hardware regardless of libvirt version as long as they share machine type. I don't think we want to paint ourselves in that corner. Plus, even if we did everything right in libvirt, guests defined at different times would end up having different ABI if QEMU has been upgraded in the meantime and thus uses a new default machine type. -- Andrea Bolognani / Red Hat / Virtualization
On Thu, Feb 01, 2018 at 15:24:32 +0100, Andrea Bolognani wrote:
On Thu, 2018-02-01 at 13:52 +0000, Daniel P. Berrangé wrote:
Note that I'm not advocating adding controllers or any other hardware to *existing* guests - that would clearly be a guest ABI breakage and thus Extremely Bad™. For newly-defined guests, however, none of the above applies AFAICT.
There's no practical way to distinguish an existing guest from a new guest being provisioned. With transient domains they are one & the same. Even with persistent guests the distinction is far from clear.
We have VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, which is only passed in when the guest is new - or the user has updated the XML themselves, in which case all bets are off when it comes to guest ABI stability anyway.
Even with this flag we can't just change anything. Maybe I named that flag wrong, but it was really meant for stuff that would break a running (or saved guest) if changed, but does not really change the hardware itself too much. One example is the update of memory sizes in certain situations. This does not give us free card of changing hardware models or adding new stuff even in cases where we know that the machine will be booted again simply for the fact that the OS may misbehave.
There are cases, when importing an existing OS image as opposed to installing from scratch, where scenarios like the ones you described might show up, but again I don't think it's realistic to expect all guests to have the same exact hardware regardless of libvirt version as long as they share machine type. I don't think we want to paint ourselves in that corner.
It may be not realistic in some cases but we seem to try really hard to achieve this. Examples exactly described in this thread. Memory baloon or USB was enabled by default in qemu a very long time ago and to this day we honor this on the x86 platform.
Plus, even if we did everything right in libvirt, guests defined at different times would end up having different ABI if QEMU has been upgraded in the meantime and thus uses a new default machine type.
The notion of the machine type is important for migration. In such case the machine needs to be exactly the same since you can't detect any difference. Even when you are guaranteed to know that the VM will be booted freshly the models of hardware in use should not change since your OS may contain only drivers used at the installation time and your VM would not work again. During the boot-up procedure you can change some things, e.g. PCI addresses and stuff which is generally allowed to change. As Daniel said ... it's impossible to know for libvirt whether a given VM is being installed or it's old and the VIR_DOMAIN_DEF_PARSE_ABI_UPDATE flag is definitely not the one we can base such decisions on.
On Thu, 2018-02-01 at 15:49 +0100, Peter Krempa wrote:
We have VIR_DOMAIN_DEF_PARSE_ABI_UPDATE, which is only passed in when the guest is new - or the user has updated the XML themselves, in which case all bets are off when it comes to guest ABI stability anyway.
Even with this flag we can't just change anything. Maybe I named that flag wrong, but it was really meant for stuff that would break a running (or saved guest) if changed, but does not really change the hardware itself too much. One example is the update of memory sizes in certain situations.
This does not give us free card of changing hardware models or adding new stuff even in cases where we know that the machine will be booted again simply for the fact that the OS may misbehave.
I would never suggest changing controller models :)
Plus, even if we did everything right in libvirt, guests defined at different times would end up having different ABI if QEMU has been upgraded in the meantime and thus uses a new default machine type.
The notion of the machine type is important for migration. In such case the machine needs to be exactly the same since you can't detect any difference.
Even when you are guaranteed to know that the VM will be booted freshly the models of hardware in use should not change since your OS may contain only drivers used at the installation time and your VM would not work again.
Okay, the one about drivers is a reasonable point.
As Daniel said ... it's impossible to know for libvirt whether a given VM is being installed or it's old and the VIR_DOMAIN_DEF_PARSE_ABI_UPDATE flag is definitely not the one we can base such decisions on.
I also realized that my patch would not do what I wanted it to in any case, because adds the USB controller and memory balloon to guests without even inspecting the flags. Looks like we're stuck with management applications having to deal with this themselvels. Oh well. -- Andrea Bolognani / Red Hat / Virtualization
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Marcin Juszkiewicz -
Peter Krempa