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

[Andrea Bolognani]

On Fri, 2018-08-03 at 13:59 +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

s/its/it's/

> lack a IDE CDROM and get PCI-X instad of PCI which changes the topology

s/PCI-X instad/PCIe instead/

[...]
> +    /* 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

s/lbivirt/libvirt/

> +     * keep 'pc' as default machine no matter what QEMU says.
> +     */
> +    if (qemuCaps->arch == VIR_ARCH_X86_64 ||
> +        qemuCaps->arch == VIR_ARCH_I686)
> +        preferredAlias = "pc";

You can use ARCH_IS_X86() here.

Since we're shielding users from changes in the default x86
machine type, I think it would make sense to do the same for other
architectures at the same time: for example, ppc64 should prefer
pseries, s390 should prefer s390-ccw-virtio and so on.

I wonder how to handle architectures where QEMU never declared a
default machine type, such as aarch64 and riscv64, though: I think
it would make sense to prefer the virt machine type there, but I'm
not entirely sure that wouldn't cause any breakages either...

[...]
> +        if (STREQ_NULLABLE(mach->alias, preferredAlias))
> +            preferredIdx = qemuCaps->nmachineTypes - 1;

Eduardo has a good point here - we should make sure preferredAlias
is not NULL before attempting to set preferredIdx.

[...]
> -    if (defIdx)
> -        virQEMUCapsSetDefaultMachine(qemuCaps, defIdx);
> +    if (preferredIdx == -1)
> +        preferredIdx = defIdx;
> +    virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx);

With this change, you're calling virQEMUCapsSetDefaultMachine()
even when the default machine type is already at the beginning
of the list, which didn't happen before. Shouldn't make any
difference in practice; however, I find preferredIdx and defIdx
having different semantics a bit confusing, so I'd really rather
you made sure that doesn't happen...

-- 
Andrea Bolognani / Red Hat / Virtualization