On Fri, Jun 17, 2016 at 12:01:58PM -0400, Laine Stump wrote:
On 06/17/2016 11:46 AM, Martin Kletzander wrote:On Fri, Jun 17, 2016 at 11:36:05AM -0400, Laine Stump wrote:On 06/17/2016 08:43 AM, Andrea Bolognani wrote:* other than the pcie-root. This is so that there will be hot-pluggable - * PCI slots available + * PCI slots available. + * + * We skip this step for aarch64 mach-virt guests, where we want to + * be able to have a pure virtio-mmio topology */ if (virDomainControllerFind(def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1) < 0 && + !qemuDomainMachineIsVirt(def) &&You're assuming that the only virt* machinetypes will be aarch64, which may be reasonable now, but not in the future (periodically someone from qemu will mention the idea of a "virt" machinetype for x86, which is legacy-free and accepts only virtio devices). Wouldn't a more specific comparison be better here (and in the other places in this patch)?Just my $.02 here, but since our qemuDomainMachineIsVirt() is made specifically for aarch64 arches, I think the right thing to do would be just add architecture check into that function as using it throughout the codebase ought to actually be what all the callers want.Sure, a single function would be great. Its name should reflect that it is for *aarch64* virt machines though (in anticipation of other arches getting a virt machinetype).
I thought that was obvious but re-reading what I wrote it isn't. So to
be sure, I'll express my feelings with code, so it's more exact =)
Something along the lines of:
diff --git i/src/qemu/qemu_domain.c w/src/qemu/qemu_domain.c
index 2fe59d78fd2f..cc6b7ae10a08 100644
--- i/src/qemu/qemu_domain.c
+++ w/src/qemu/qemu_domain.c
@@ -4906,10 +4906,11 @@ qemuDomainMachineIsS390CCW(const virDomainDef *def)
bool
-qemuDomainMachineIsVirt(const virDomainDef *def)
+qemuDomainIsAArch64Virt(const virDomainDef *def)
{
- return STREQ(def->os.machine, "virt") ||
- STRPREFIX(def->os.machine, "virt-");
+ if (def->os.arch != VIR_ARCH_AARCH64) &&
+ STREQ(def->os.machine, "virt") ||
+ STRPREFIX(def->os.machine, "virt-");
}
--
Martin
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list