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).