[libvirt] [PATCH 3/3] qemu: caps: Advertise arm 32-on-64 KVM option

[Cole Robinson]

On 05/28/2015 07:31 AM, Daniel P. Berrange wrote:
> On Thu, May 21, 2015 at 07:03:28PM -0400, Cole Robinson wrote:
>> We need to use qemu-system-aarch64 to run armv7l KVM VMs on an aarch64
>> host.
>> ---
>>  src/qemu/qemu_capabilities.c | 42 ++++++++++++++++++++++--------------------
>>  1 file changed, 22 insertions(+), 20 deletions(-)
>>
>> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>> index 1e7bddb..7181865 100644
>> --- a/src/qemu/qemu_capabilities.c
>> +++ b/src/qemu/qemu_capabilities.c
>> @@ -723,19 +723,6 @@ virQEMUCapsFindBinaryForArch(virArch hostarch,
>>      return ret;
>>  }
>>  
>> -
>> -static bool
>> -virQEMUCapsIsValidForKVM(virArch hostarch,
>> -                         virArch guestarch)
>> -{
>> -    if (hostarch == guestarch)
>> -        return true;
>> -    if (hostarch == VIR_ARCH_X86_64 &&
>> -        guestarch == VIR_ARCH_I686)
>> -        return true;
>> -    return false;
>> -}
>> -
>>  static int
>>  virQEMUCapsInitGuest(virCapsPtr caps,
>>                       virQEMUCapsCachePtr cache,
>> @@ -747,6 +734,7 @@ virQEMUCapsInitGuest(virCapsPtr caps,
>>      char *binary = NULL;
>>      virQEMUCapsPtr qemubinCaps = NULL;
>>      virQEMUCapsPtr kvmbinCaps = NULL;
>> +    bool native_kvm, x86_32on64_kvm, arm_32on64_kvm;
>>      int ret = -1;
>>  
>>      /* Check for existence of base emulator, or alternate base
>> @@ -764,16 +752,30 @@ virQEMUCapsInitGuest(virCapsPtr caps,
>>  
>>      /* qemu-kvm/kvm binaries can only be used if
>>       *  - host & guest arches match
>> -     * Or
>> -     *  - hostarch is x86_64 and guest arch is i686
>> -     * The latter simply needs "-cpu qemu32"
>> +     *  - hostarch is x86_64 and guest arch is i686 (needs -cpu qemu32)
>> +     *  - hostarch is aarch64 and guest arch is armv7l (needs -cpu aarch64=off)
>>       */
>> -    if (virQEMUCapsIsValidForKVM(hostarch, guestarch)) {
>> -        const char *const kvmbins[] = { "/usr/libexec/qemu-kvm", /* RHEL */
>> -                                        "qemu-kvm", /* Fedora */
>> -                                        "kvm" }; /* Upstream .spec */
>> +    native_kvm = (hostarch == guestarch);
>> +    x86_32on64_kvm = (hostarch == VIR_ARCH_X86_64 &&
>> +        guestarch == VIR_ARCH_I686);
>> +    arm_32on64_kvm = (hostarch == VIR_ARCH_AARCH64 &&
>> +        guestarch == VIR_ARCH_ARMV7L);
>> +
>> +    if (native_kvm || x86_32on64_kvm || arm_32on64_kvm) {
>> +        const char *kvmbins[] = {
>> +            "/usr/libexec/qemu-kvm", /* RHEL */
>> +            "qemu-kvm", /* Fedora */
>> +            "kvm", /* Debian/Ubuntu */
>> +            NULL,
>> +        };
>> +
>> +        if (arm_32on64_kvm)
>> +            kvmbins[3] = "qemu-system-aarch64";
> 
> I'm unclear why you need to be adding this. We don't need it for
> the equivalent i686 with qemu-system-x86_64, as the earlier call
> to virQEMUCapsFindBinaryForArch() will already return the binary
> qemu-system-x86_64. IIUC, it should have returned the binary
> qemu-system-aarch64 too, so this just seems to duplicate the
> check for that binary.

We need this in the case you are running on an aarch64 host and have both
qemu-system-arm and qemu-system-aarch64 installed. In this case, when you want
to use KVM for arm32, you _have_ to use qemu-system-aarch64, qemu-system-arm
does not work. What you suggest would mean that qemu-system-arm is grabbed
from the caps cache.

x86 doesn't have this problem because qemu-system-i386, qemu-system-x86_64 and
by extension qemu-kvm can all be used to do 32-on-64 KVM.

Thanks,
Cole