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