On Fri, Aug 03, 2018 at 01:59:47PM +0100, Daniel P. Berrangé wrote: > 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 its using a "pc" machine by default. For example they'll > lack a IDE CDROM and get PCI-X instad 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. > > 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@xxxxxxxxxx> Won't this break qemuParseCommandLine() if it sees a QEMU binary running without "-machine"? It will assume the QEMU default is "pc" but this may be not true. > --- > src/qemu/qemu_capabilities.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 0fb800589a..9eb58afef3 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -2242,6 +2242,17 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, > int ret = -1; > size_t i; > size_t defIdx = 0; > + ssize_t preferredIdx = -1; > + const char *preferredAlias = NULL; > + > + /* Historically QEMU defaulted to 'pc' machine type but in > + * future might switch 'q35'. Such a change is considered > + * an ABI break from lbivirt's POV, so we must be sure to > + * keep 'pc' as default machine no matter what QEMU says. > + */ > + if (qemuCaps->arch == VIR_ARCH_X86_64 || > + qemuCaps->arch == VIR_ARCH_I686) > + preferredAlias = "pc"; > > if ((nmachines = qemuMonitorGetMachines(mon, &machines)) < 0) > return -1; > @@ -2263,12 +2274,16 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, > mach->maxCpus = machines[i]->maxCpus; > mach->hotplugCpus = machines[i]->hotplugCpus; > > + if (STREQ_NULLABLE(mach->alias, preferredAlias)) > + preferredIdx = qemuCaps->nmachineTypes - 1; > + Isn't STREQ_NULLABLE(NULL, NULL) true? You don't want to set preferredIdx here if preferredAlias==NULL. > if (machines[i]->isDefault) > defIdx = qemuCaps->nmachineTypes - 1; > } > > - if (defIdx) > - virQEMUCapsSetDefaultMachine(qemuCaps, defIdx); > + if (preferredIdx == -1) > + preferredIdx = defIdx; > + virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx); > > ret = 0; > > -- > 2.17.1 -- Eduardo -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
