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

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(a)redhat.com&gt; > > > > 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.