
On Thu, 2018-08-16 at 17:01 +0200, Cornelia Huck wrote:
On Thu, 16 Aug 2018 16:52:18 +0200 Andrea Bolognani <abologna@redhat.com> wrote:
On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote:
@@ -3227,6 +3227,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; + addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI); break;
I wonder if you might want to do this, and other stuff, only if qemuDomainIsS390CCW()... It seems like that function is not used a lot, which might mean that someone is going to have to perform some serious cleaning up if there is ever another machine type on s390, or perhaps that it just doesn't quite apply here.
Not a libvirt person, but that other QEMU s390 machine type is long gone (and was never supported in libvirt AFAIK anyway), and I don't see any reason to add an additional s390 machine type to QEMU from where we stand now.
New machine types happen when you least expect them :) But given that the current code would need an overhaul anyway if a new machine type was introduced, I'm okay with merging this as-is. -- Andrea Bolognani / Red Hat / Virtualization