7:22 a.m.
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(a)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
8:54 a.m.
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(a)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
9:28 a.m.
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(a)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 :|
9:57 a.m.
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
10:01 a.m.
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 :|
3:12 a.m.
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
4:19 a.m.
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 :|
10:11 a.m.
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
2334
days inactive
2335
days old
7 comments
2 participants
participants (2)
-
Andrea Bolognani -
Daniel P. Berrangé