Re: [libvirt] [PATCH] Don't list capabilities entries if emulators can't be accessed

[Cole Robinson <crobinso redhat com>]

Daniel P. Berrange wrote:
> On Wed, Aug 20, 2008 at 12:51:05PM -0400, Cole Robinson wrote:
> 
>> diff --git a/src/qemu_conf.c b/src/qemu_conf.c
>> index dc9e42a..0328cc1 100644
>> --- a/src/qemu_conf.c
>> +++ b/src/qemu_conf.c
>> @@ -230,6 +230,10 @@ qemudCapsInitGuest(virCapsPtr caps,
>>      virCapsGuestPtr guest;
>>      int i;
>>  
>> +    /* Check for existance of base emulator */
>> +    if (access(info->binary, X_OK) == -1)
>> +        return 0;
>> +
> 
> This isn't right - this means that if KVM is installed, but QEMU is
> not installed you won't get any capabilities.
> 
> Basically we need todo all the access() checks for QEMU, KVM, /dev/kvm
> up-front. And then generated the capabilites if either QEMU or KVM is
> available.
> 

Okay, I think this patch solves the issues. We check upfront
for the base emulator and potential kvm emulators (qemu-kvm,
and /usr/bin/kvm for ubuntu/upstream .spec). If nothing is
found, just return. If only the base emulator is found, skip
kvm even if /dev/kvm exists. If only kvm bin is found, add
the base emulator capabilities only if emulator and host arch
matches.

Thanks,
Cole

diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index ee5851d..4ef75dd 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -226,7 +226,29 @@ qemudCapsInitGuest(virCapsPtr caps,
                    const struct qemu_arch_info *info,
                    int hvm) {
     virCapsGuestPtr guest;
-    int i;
+    int i, haskvm, hasbase, samearch;
+    const char *kvmbin = NULL;
+
+    /* Check for existance of base emulator */
+    hasbase = (access(info->binary, X_OK) == 0);
+
+    samearch = STREQ(info->arch, hostmachine);
+    if (samearch) {
+        const char *const kvmbins[] = { "/usr/bin/qemu-kvm", /* Fedora */
+                                        "/usr/bin/kvm" }; /* Upstream .spec */
+
+        for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
+            if ((haskvm = (access(kvmbins[i], X_OK) == 0))) {
+                kvmbin = kvmbins[i];
+                break;
+            }
+        }
+    } else {
+        haskvm = 0;
+    }
+
+    if (!hasbase && !haskvm)
+        return 0;
 
     if ((guest = virCapabilitiesAddGuest(caps,
                                          hvm ? "hvm" : "xen",
@@ -239,8 +261,7 @@ qemudCapsInitGuest(virCapsPtr caps,
         return -1;
 
     if (hvm) {
-        /* Check for existance of base emulator */
-        if (access(info->binary, X_OK) == 0 &&
+        if (hasbase &&
             virCapabilitiesAddGuestDomain(guest,
                                           "qemu",
                                           NULL,
@@ -250,7 +271,7 @@ qemudCapsInitGuest(virCapsPtr caps,
             return -1;
 
         /* If guest & host match, then we can accelerate */
-        if (STREQ(info->arch, hostmachine)) {
+        if (samearch) {
             if (access("/dev/kqemu", F_OK) == 0 &&
                 virCapabilitiesAddGuestDomain(guest,
                                               "kqemu",
@@ -261,9 +282,10 @@ qemudCapsInitGuest(virCapsPtr caps,
                 return -1;
 
             if (access("/dev/kvm", F_OK) == 0 &&
+                haskvm &&
                 virCapabilitiesAddGuestDomain(guest,
                                               "kvm",
-                                              "/usr/bin/qemu-kvm",
+                                              kvmbin,
                                               NULL,
                                               0,
                                               NULL) == NULL)