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

[Daniel P. Berrangé]

On Tue, Aug 07, 2018 at 03:53:53PM -0300, Eduardo Habkost wrote:
> 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.

Oh right, I mis-interpreted things :-(  This patch is pushed, so I'll think
about a followup fix.

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 :|