[libvirt] [PATCH] qemu: ensure default machine types don't change if QEMU changes
It is increasingly likely that some distro is going to change the default "x86" machine type in QEMU from "pc" to "q35". This will certainly break existing applications which write their XML on the assumption that it is using a "pc" machine by default. For example they'll lack a IDE CDROM and get PCIe instead of PCI which changes the topology radically. Libvirt promises to isolate applications from hypervisor changes that may cause incompatibilities, so we must ensure that we always use the "pc" machine type if it is available. Only use QEMU's own reported default machine type if "pc" does not exist. This issue is not x86-only, other arches are liable t change their default machine, while some arches don't report any default at all causing libvirt to pick the first machine in the list. Thus to guarantee stability to applications, declare a preferred default machine for all architectures we currently support with QEMU. Note this change assumes there will always be a "pc" alias as long as a versioned "pc-XXX" machine type exists. If QEMU were to ship a "pc-XXX" machine type but not provide the "pc" alias, it is too hard to decide which to default so. Versioned machine types are supposed to be considered opaque strings, so we can't apply any sensible ordering ourselves and QEMU isn't reporting the list of machines in any sensible ordering itself. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/qemu/qemu_capabilities.c | 79 ++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 0fb800589a..045e2bd489 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2233,6 +2233,61 @@ virQEMUCapsProbeQMPDevices(virQEMUCapsPtr qemuCaps, } +/* Historically QEMU x86 targets defaulted to 'pc' machine type but + * in future x86_64 might switch 'q35'. Such a change is considered + * an ABI break from libvirt's POV. Other QEMU targets may not declare + * a default macine at all, causing libvirt to use the first reported + * machinbe in the list. + * + * Here we record a preferred default machine for all arches, so + * that we're not vulnerable to changes in QEMU defaults or machine + * list ordering. + */ +static const char *preferredMachines[VIR_ARCH_LAST] = +{ + [VIR_ARCH_ALPHA] = "clipper", + [VIR_ARCH_ARMV6L] = NULL, /* No QEMU impl */ + [VIR_ARCH_ARMV7L] = "integratorcp", + [VIR_ARCH_ARMV7B] = "integratorcp", + + [VIR_ARCH_AARCH64] = "integratorcp", + [VIR_ARCH_CRIS] = "axis-dev88", + [VIR_ARCH_I686] = "pc", + [VIR_ARCH_ITANIUM] = NULL, /* doesn't exist in QEMU any more */ + [VIR_ARCH_LM32] = "lm32-evr", + + [VIR_ARCH_M68K] = "mcf5208evb", + [VIR_ARCH_MICROBLAZE] = "petalogix-s3adsp1800", + [VIR_ARCH_MICROBLAZEEL] = "petalogix-s3adsp1800", + [VIR_ARCH_MIPS] = "malta", + [VIR_ARCH_MIPSEL] = "malta", + + [VIR_ARCH_MIPS64] = "malta", + [VIR_ARCH_MIPS64EL] = "malta", + [VIR_ARCH_OR32] = "or1k-sim", + [VIR_ARCH_PARISC] = NULL, /* No QEMU impl */ + [VIR_ARCH_PARISC64] = NULL, /* No QEMU impl */ + + [VIR_ARCH_PPC] = "g3beige", + [VIR_ARCH_PPCLE] = "g3beige", + [VIR_ARCH_PPC64] = "pseries", + [VIR_ARCH_PPC64LE] = "pseries", + [VIR_ARCH_PPCEMB] = "bamboo", + + [VIR_ARCH_S390] = NULL, /* No QEMU impl*/ + [VIR_ARCH_S390X] = "s390-ccw-virtio", + [VIR_ARCH_SH4] = "shix", + [VIR_ARCH_SH4EB] = "shix", + [VIR_ARCH_SPARC] = "SS-5", + + [VIR_ARCH_SPARC64] = "sun4u", + [VIR_ARCH_UNICORE32] = "puv3", + [VIR_ARCH_X86_64] = "pc", + [VIR_ARCH_XTENSA] = "sim", + [VIR_ARCH_XTENSAEB] = "sim", + +}; + static int virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) @@ -2241,7 +2296,9 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, int nmachines = 0; int ret = -1; size_t i; - size_t defIdx = 0; + ssize_t defIdx = -1; + ssize_t preferredIdx = -1; + const char *preferredMachine = preferredMachines[qemuCaps->arch]; if ((nmachines = qemuMonitorGetMachines(mon, &machines)) < 0) return -1; @@ -2263,12 +2320,28 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, mach->maxCpus = machines[i]->maxCpus; mach->hotplugCpus = machines[i]->hotplugCpus; + if (preferredMachine && + (STREQ_NULLABLE(mach->alias, preferredMachine) || + STREQ(mach->name, preferredMachine))) + preferredIdx = qemuCaps->nmachineTypes - 1; + if (machines[i]->isDefault) defIdx = qemuCaps->nmachineTypes - 1; } - if (defIdx) - virQEMUCapsSetDefaultMachine(qemuCaps, defIdx); + /* + * We'll prefer to use our own historical default machine + * to avoid mgmt apps seeing semantics changes when QEMU + * alters it defaults. + * + * Our preferred pmachine might have been compiled out of + * QEMU at build time though, so we still fallback to honouring + * QEMU's reported default in that case + */ + if (preferredIdx == -1) + preferredIdx = defIdx; + if (preferredIdx != -1) + virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx); ret = 0; -- 2.17.1
On Tue, 2018-08-07 at 13:22 +0100, Daniel P. Berrangé wrote: [...]
+/* Historically QEMU x86 targets defaulted to 'pc' machine type but + * in future x86_64 might switch 'q35'. Such a change is considered
s/switch/switch to/
+ * an ABI break from libvirt's POV. Other QEMU targets may not declare + * a default macine at all, causing libvirt to use the first reported
s/macine/machine/
+ * machinbe in the list.
s/machinbe/machine/ [...]
+ [VIR_ARCH_SPARC64] = "sun4u", + [VIR_ARCH_UNICORE32] = "puv3", + [VIR_ARCH_X86_64] = "pc", + [VIR_ARCH_XTENSA] = "sim", + [VIR_ARCH_XTENSAEB] = "sim", +
No empty line here. The list looks correct as far as the architectures I'm familiar with are concerned, so I'm going to assume whatever method you used to generate it is sound and skip double-checking it :) [...]
+ if (preferredMachine && + (STREQ_NULLABLE(mach->alias, preferredMachine) || + STREQ(mach->name, preferredMachine))) + preferredIdx = qemuCaps->nmachineTypes - 1;
Braces around the body here. [...]
- if (defIdx) - virQEMUCapsSetDefaultMachine(qemuCaps, defIdx); + /* + * We'll prefer to use our own historical default machine + * to avoid mgmt apps seeing semantics changes when QEMU + * alters it defaults.
s/it/its/ [...]
+ * Our preferred pmachine might have been compiled out of
s/pmachine/machine/
+ * QEMU at build time though, so we still fallback to honouring + * QEMU's reported default in that case + */ + if (preferredIdx == -1) + preferredIdx = defIdx; + if (preferredIdx != -1) + virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx);
This is certainly an improvement compared to the current situation, so Reviewed-by: Andrea Bolognani <abologna@redhat.com> I wonder if we shouldn't just drop the default machine type handling altogether at this point, though. I expect that downstreams who customize the list of machine types exposed by their QEMU binaries will want to patch the table above either way: for example, anyone shipping qemu-system-aarch64 in an enterprise context will definitely want to compile out integratorcp and use virt as the default instead; they might decide to introduce a default at the QEMU level (upstream QEMU doesn't have one for aarch64) but I'd expect them to also tweak libvirt's own defaults at the same time. With that in mind, I'm not sure whether leaving the default machine type handling in is not going to cause unexpected behavior rather than being helpful. -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Aug 07, 2018 at 03:54:06PM +0200, Andrea Bolognani wrote:
On Tue, 2018-08-07 at 13:22 +0100, Daniel P. Berrangé wrote:
[...]
+ * Our preferred pmachine might have been compiled out of
s/pmachine/machine/
+ * QEMU at build time though, so we still fallback to honouring + * QEMU's reported default in that case + */ + if (preferredIdx == -1) + preferredIdx = defIdx; + if (preferredIdx != -1) + virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx);
This is certainly an improvement compared to the current situation, so
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
I wonder if we shouldn't just drop the default machine type handling altogether at this point, though.
That's impossible as it violates the back compatibility guarantee and will certainly break applications 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 Tue, 2018-08-07 at 15:28 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 07, 2018 at 03:54:06PM +0200, Andrea Bolognani wrote:
I wonder if we shouldn't just drop the default machine type handling altogether at this point, though.
That's impossible as it violates the back compatibility guarantee and will certainly break applications
In abstract terms, given that the whole point of this exercise is shielding our users from changes in QEMU, why wouldn't we go the whole way and take QEMU defaults out of the picture entirely? I just can't picture a scenario where ignoring the QEMU defaults would actually cause issues, since we're basically moving the defaults into libvirt with this commit... Can you describe such a scenario? -- Andrea Bolognani / Red Hat / Virtualization
On Tue, Aug 07, 2018 at 04:57:28PM +0200, Andrea Bolognani wrote:
On Tue, 2018-08-07 at 15:28 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 07, 2018 at 03:54:06PM +0200, Andrea Bolognani wrote:
I wonder if we shouldn't just drop the default machine type handling altogether at this point, though.
That's impossible as it violates the back compatibility guarantee and will certainly break applications
In abstract terms, given that the whole point of this exercise is shielding our users from changes in QEMU, why wouldn't we go the whole way and take QEMU defaults out of the picture entirely?
I just can't picture a scenario where ignoring the QEMU defaults would actually cause issues, since we're basically moving the defaults into libvirt with this commit... Can you describe such a scenario?
Someone could build a QEMU with the "pc" machine type deleted entirely, in which case libvirt won't find the default. The best we can do there is fallback to QEMU's own default. To avoid that problem we would have to maintain a sorted list of every single known machine type in every QEMU which gets a bit ridiculous. 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 Tue, 2018-08-07 at 16:01 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 07, 2018 at 04:57:28PM +0200, Andrea Bolognani wrote:
I wonder if we shouldn't just drop the default machine type handling altogether at this point, though.
That's impossible as it violates the back compatibility guarantee and will certainly break applications
In abstract terms, given that the whole point of this exercise is shielding our users from changes in QEMU, why wouldn't we go the whole way and take QEMU defaults out of the picture entirely?
I just can't picture a scenario where ignoring the QEMU defaults would actually cause issues, since we're basically moving the defaults into libvirt with this commit... Can you describe such a scenario?
Someone could build a QEMU with the "pc" machine type deleted entirely, in which case libvirt won't find the default. The best we can do there is fallback to QEMU's own default.
Yeah, that's basically my point: wouldn't it be better to error out rather than silently pick up a different default which is not controlled by libvirt? Of course that would be slightly annoying for people building their own QEMU binaries with machine types ripped out, but I can't imagine that population being very large and they're certainly capable enough to figure out a way around the error; most people will get their QEMU and libvirt through downstream distributions, and if the downstream changes the list of machines available in QEMU they should patch libvirt to update the table anyway. Ultimately, users and applications should make sure they include an explicit machine type in the guest XML, so ideally in time this code path will not be hit at all.
To avoid that problem we would have to maintain a sorted list of every single known machine type in every QEMU which gets a bit ridiculous.
Fully agreed :) -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Aug 08, 2018 at 10:12:59AM +0200, Andrea Bolognani wrote:
On Tue, 2018-08-07 at 16:01 +0100, Daniel P. Berrangé wrote:
On Tue, Aug 07, 2018 at 04:57:28PM +0200, Andrea Bolognani wrote:
I wonder if we shouldn't just drop the default machine type handling altogether at this point, though.
That's impossible as it violates the back compatibility guarantee and will certainly break applications
In abstract terms, given that the whole point of this exercise is shielding our users from changes in QEMU, why wouldn't we go the whole way and take QEMU defaults out of the picture entirely?
I just can't picture a scenario where ignoring the QEMU defaults would actually cause issues, since we're basically moving the defaults into libvirt with this commit... Can you describe such a scenario?
Someone could build a QEMU with the "pc" machine type deleted entirely, in which case libvirt won't find the default. The best we can do there is fallback to QEMU's own default.
Yeah, that's basically my point: wouldn't it be better to error out rather than silently pick up a different default which is not controlled by libvirt?
Of course that would be slightly annoying for people building their own QEMU binaries with machine types ripped out, but I can't imagine that population being very large and they're certainly capable enough to figure out a way around the error; most people will get their QEMU and libvirt through downstream distributions, and if the downstream changes the list of machines available in QEMU they should patch libvirt to update the table anyway.
Ultimately, users and applications should make sure they include an explicit machine type in the guest XML, so ideally in time this code path will not be hit at all.
Again, any change which mandates specifying a machine type in the XML is a regression for libvirt's users. Even if there's no 'pc' machine present in the QEMU binary we must continue to provide a default of some kind. Sure some apps would break in this case regardless, but other apps will still work. There's no valid reason why we should break use of an XML config like this as it works fine with both i440fx and q35 <domain type='test'> <name>demo</name> <os> <type>hvm</type> </os> <memory>261072</memory> <vcpu>1</vcpu> <devices> <disk type='file'> <source file='/var/lib/libvirt/images/demo.img'/> <target bus="virtio" dev='vda'/> </disk> <interface type='bridge'> <source bridge='br0'/> <mac address='aa:00:00:00:00:11'/> <model type="virtio-net"/> </interface> <console type='pty'/> </devices> </domain> 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 Wed, 2018-08-08 at 10:19 +0100, Daniel P. Berrangé wrote:
On Wed, Aug 08, 2018 at 10:12:59AM +0200, Andrea Bolognani wrote:
On Tue, 2018-08-07 at 16:01 +0100, Daniel P. Berrangé wrote:
Someone could build a QEMU with the "pc" machine type deleted entirely, in which case libvirt won't find the default. The best we can do there is fallback to QEMU's own default.
Yeah, that's basically my point: wouldn't it be better to error out rather than silently pick up a different default which is not controlled by libvirt?
Of course that would be slightly annoying for people building their own QEMU binaries with machine types ripped out, but I can't imagine that population being very large and they're certainly capable enough to figure out a way around the error; most people will get their QEMU and libvirt through downstream distributions, and if the downstream changes the list of machines available in QEMU they should patch libvirt to update the table anyway.
Ultimately, users and applications should make sure they include an explicit machine type in the guest XML, so ideally in time this code path will not be hit at all.
Again, any change which mandates specifying a machine type in the XML is a regression for libvirt's users. Even if there's no 'pc' machine present in the QEMU binary we must continue to provide a default of some kind. Sure some apps would break in this case regardless, but other apps will still work. There's no valid reason why we should break use of an XML config like this as it works fine with both i440fx and q35
<domain type='test'> <name>demo</name> <os> <type>hvm</type> </os> <memory>261072</memory> <vcpu>1</vcpu> <devices> <disk type='file'> <source file='/var/lib/libvirt/images/demo.img'/> <target bus="virtio" dev='vda'/> </disk> <interface type='bridge'> <source bridge='br0'/> <mac address='aa:00:00:00:00:11'/> <model type="virtio-net"/> </interface> <console type='pty'/> </devices> </domain>
For some definition of "works fine": if demo.img contains a CentOS 6 installation, for example, the guest will get stuck during boot... Arguably an error message about the pc machine type not being available would be more useful to the user in that case. But I understand there are several cases in which such a configuration will result in a perfectly acceptable guest despite the differences in hardware between pc and q35, so I see the value in not breaking that use case. -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Daniel P. Berrangé