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@xxxxxxxxxx> 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
