[libvirt] [PATCH] qemu: ensure "pc" machine is always used as default if available

[Eduardo Habkost]

On Tue, Aug 07, 2018 at 12:17:33PM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 03, 2018 at 01:05:49PM -0300, Eduardo Habkost wrote:
> > 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 at redhat.com>
> > 
> > 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.
> 
> If no -machine arg is present in ARGV, then the code will lookup the
> default machine type for the emulator binary in the capabilities
> record. So this should just "do the right thing" with my changes
> in this patch.

I don't see how it would do the right thing.  e.g.: if we have a
QEMU binary that defaults to "q35" and it is running without
"-machine", it will be emulating a q35 machine, not i440fx.  But
with this change qemuParseCommandLine() will incorrectly assume
the existing process is emulating i440fx.

qemuParseCommandLine() calls:

  capsdata = virCapabilitiesDomainDataLookup(caps, def->os.type, def->os.arch,
                                             def->virtType, NULL, NULL)

and as far as I can see, your patch will make
qemuParseCommandLine() set capsdata->machinetype to a pc-i440fx-*
machine, even if the QEMU binary defaults to something else.

-- 
Eduardo