[libvirt] [PATCH] qemu: Permit PCI-free aarch64 mach-virt guests

[Martin Kletzander]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160617/b531a154/attachment-0001.sig>