[libvirt] [PATCH] qemu: ensure default machine types don't change if QEMU changes

[Andrea Bolognani]

On Tue, 2018-08-07 at 13:22 +0100, Daniel P. Berrangé wrote:
[...]
> +/* Historically QEMU x86 targets defaulted to 'pc' machine type but
> + * in future x86_64 might switch 'q35'. Such a change is considered

s/switch/switch to/

> + * an ABI break from libvirt's POV. Other QEMU targets may not declare
> + * a default macine at all, causing libvirt to use the first reported

s/macine/machine/

> + * machinbe in the list.

s/machinbe/machine/

[...]
> +    [VIR_ARCH_SPARC64] = "sun4u",
> +    [VIR_ARCH_UNICORE32] = "puv3",
> +    [VIR_ARCH_X86_64] = "pc",
> +    [VIR_ARCH_XTENSA] = "sim",
> +    [VIR_ARCH_XTENSAEB] = "sim",
> +

No empty line here.

The list looks correct as far as the architectures I'm familiar
with are concerned, so I'm going to assume whatever method you used
to generate it is sound and skip double-checking it :)

[...]
> +        if (preferredMachine &&
> +            (STREQ_NULLABLE(mach->alias, preferredMachine) ||
> +             STREQ(mach->name, preferredMachine)))
> +            preferredIdx = qemuCaps->nmachineTypes - 1;

Braces around the body here.

[...]
> -    if (defIdx)
> -        virQEMUCapsSetDefaultMachine(qemuCaps, defIdx);
> +    /*
> +     * We'll prefer to use our own historical default machine
> +     * to avoid mgmt apps seeing semantics changes when QEMU
> +     * alters it defaults.

s/it/its/

[...]
> +     * Our preferred pmachine might have been compiled out of

s/pmachine/machine/

> +     * QEMU at build time though, so we still fallback to honouring
> +     * QEMU's reported default in that case
> +     */
> +    if (preferredIdx == -1)
> +        preferredIdx = defIdx;
> +    if (preferredIdx != -1)
> +        virQEMUCapsSetDefaultMachine(qemuCaps, preferredIdx);

This is certainly an improvement compared to the current situation,
so

  Reviewed-by: Andrea Bolognani <abologna at redhat.com>

I wonder if we shouldn't just drop the default machine type handling
altogether at this point, though.

I expect that downstreams who customize the list of machine types
exposed by their QEMU binaries will want to patch the table above
either way: for example, anyone shipping qemu-system-aarch64 in an
enterprise context will definitely want to compile out integratorcp
and use virt as the default instead; they might decide to introduce
a default at the QEMU level (upstream QEMU doesn't have one for
aarch64) but I'd expect them to also tweak libvirt's own defaults
at the same time.

With that in mind, I'm not sure whether leaving the default machine
type handling in is not going to cause unexpected behavior rather
than being helpful.

-- 
Andrea Bolognani / Red Hat / Virtualization