10:45 a.m.
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(a)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
10:58 a.m.
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.
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 :|
6:04 a.m.
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
3:48 p.m.
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.
3:54 a.m.
[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
6:54 a.m.
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 :|
7:16 a.m.
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.
1. https://review.openstack.org/#/c/538003/
7:44 a.m.
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
7:52 a.m.
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 :|
8:24 a.m.
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
8:49 a.m.
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.
10:20 a.m.
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
2516
days inactive
2523
days old
11 comments
4 participants
participants (4)
-
Andrea Bolognani -
Daniel P. Berrangé -
Marcin Juszkiewicz -
Peter Krempa