From bbda37d1ca472b0d81f52b24f2f945d978b8bce3 Mon Sep 17 00:00:00 2001
Message-Id: <bbda37d1ca472b0d81f52b24f2f945d978b8bce3@dist-git>
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 18 Sep 2014 11:45:39 +0200
Subject: [PATCH] domaincaps: Expose UEFI binary path, if it exists

https://bugzilla.redhat.com/show_bug.cgi?id=1026772

Check to see if the UEFI binary mentioned in qemu.conf actually
exists, and if so expose it in domcapabilities like

<loader ...>
  <value>/path/to/ovmf</value>
</loader>

We introduce some generic domcaps infrastructure for handling
a dynamic list of string values, it may be of use for future bits.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit f05b6a918e283929f5d837cd1668cdcdb3834b9a)
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 docs/formatdomaincaps.html.in                      |  6 +++
 docs/schemas/domaincaps.rng                        | 17 +++++---
 src/conf/domain_capabilities.c                     | 29 +++++++++++++
 src/conf/domain_capabilities.h                     |  8 ++++
 src/qemu/qemu_capabilities.c                       | 32 +++++++++++---
 src/qemu/qemu_capabilities.h                       |  7 +++-
 src/qemu/qemu_driver.c                             |  6 ++-
 tests/domaincapsschemadata/domaincaps-full.xml     |  2 +
 .../domaincaps-qemu_1.6.50-1.xml                   |  1 +
 tests/domaincapstest.c                             | 49 +++++++++++++++++++---
 10 files changed, 140 insertions(+), 17 deletions(-)

diff --git a/docs/formatdomaincaps.html.in b/docs/formatdomaincaps.html.in
index 34d746d..6959dfe 100644
--- a/docs/formatdomaincaps.html.in
+++ b/docs/formatdomaincaps.html.in
@@ -105,6 +105,7 @@
   ...
   &lt;os supported='yes'&gt;
     &lt;loader supported='yes'&gt;
+      &lt;value&gt;/usr/share/OVMF/OVMF_CODE.fd&lt;/value&gt;
       &lt;enum name='type'&gt;
         &lt;value&gt;rom&lt;/value&gt;
         &lt;value&gt;pflash&lt;/value&gt;
@@ -122,6 +123,11 @@
     <p>For the <code>loader</code> element, the following can occur:</p>
 
     <dl>
+      <dt>value</dt>
+      <dd>List of known loader paths. Currently this is only used
+      to advertise known locations of OVMF binaries for qemu. Binaries
+      will only be listed if they actually exist on disk.</dd>
+
       <dt>type</dt>
       <dd>Whether loader is a typical BIOS (<code>rom</code>) or
       an UEFI binary (<code>pflash</code>). This refers to
diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng
index ad8d966..f4a555f 100644
--- a/docs/schemas/domaincaps.rng
+++ b/docs/schemas/domaincaps.rng
@@ -47,6 +47,9 @@
   <define name='loader'>
     <element name='loader'>
       <ref name='supported'/>
+      <optional>
+        <ref name='value'/>
+      </optional>
       <ref name='enum'/>
     </element>
   </define>
@@ -85,6 +88,14 @@
     </element>
   </define>
 
+  <define name='value'>
+    <zeroOrMore>
+      <element name='value'>
+        <text/>
+      </element>
+    </zeroOrMore>
+  </define>
+
   <define name='supported'>
     <attribute name='supported'>
       <choice>
@@ -100,11 +111,7 @@
         <attribute name='name'>
           <text/>
         </attribute>
-        <zeroOrMore>
-          <element name='value'>
-            <text/>
-          </element>
-        </zeroOrMore>
+        <ref name='value'/>
       </element>
     </zeroOrMore>
   </define>
diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 5a3c8e7..7c59912 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -48,12 +48,28 @@ VIR_ONCE_GLOBAL_INIT(virDomainCaps)
 
 
 static void
+virDomainCapsStringValuesFree(virDomainCapsStringValuesPtr values)
+{
+    size_t i;
+
+    if (!values || !values->values)
+        return;
+
+    for (i = 0; i < values->nvalues; i++)
+        VIR_FREE(values->values[i]);
+    VIR_FREE(values->values);
+}
+
+
+static void
 virDomainCapsDispose(void *obj)
 {
     virDomainCapsPtr caps = obj;
 
     VIR_FREE(caps->path);
     VIR_FREE(caps->machine);
+
+    virDomainCapsStringValuesFree(&caps->os.loader.values);
 }
 
 
@@ -156,6 +172,18 @@ virDomainCapsEnumFormat(virBufferPtr buf,
     return ret;
 }
 
+
+static void
+virDomainCapsStringValuesFormat(virBufferPtr buf,
+                                virDomainCapsStringValuesPtr values)
+{
+    size_t i;
+
+    for (i = 0; i < values->nvalues; i++)
+        virBufferEscapeString(buf, "<value>%s</value>\n", values->values[i]);
+}
+
+
 #define FORMAT_PROLOGUE(item)                                       \
     do {                                                            \
         virBufferAsprintf(buf, "<" #item " supported='%s'%s\n",     \
@@ -185,6 +213,7 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
 {
     FORMAT_PROLOGUE(loader);
 
+    virDomainCapsStringValuesFormat(buf, &loader->values);
     ENUM_PROCESS(loader, type, virDomainLoaderTypeToString);
     ENUM_PROCESS(loader, readonly, virTristateBoolTypeToString);
 
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index 768646b..597ac75 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -37,6 +37,13 @@ struct _virDomainCapsEnum {
     unsigned int values; /* Bitmask of values supported in the corresponding enum */
 };
 
+typedef struct _virDomainCapsStringValues virDomainCapsStringValues;
+typedef virDomainCapsStringValues *virDomainCapsStringValuesPtr;
+struct _virDomainCapsStringValues {
+    char **values;  /* raw string values */
+    size_t nvalues; /* number of strings */
+};
+
 typedef struct _virDomainCapsDevice virDomainCapsDevice;
 typedef virDomainCapsDevice *virDomainCapsDevicePtr;
 struct _virDomainCapsDevice {
@@ -47,6 +54,7 @@ typedef struct _virDomainCapsLoader virDomainCapsLoader;
 typedef virDomainCapsLoader *virDomainCapsLoaderPtr;
 struct _virDomainCapsLoader {
     virDomainCapsDevice device;
+    virDomainCapsStringValues values;   /* Info about values for the element */
     virDomainCapsEnum type;     /* Info about virDomainLoader */
     virDomainCapsEnum readonly; /* Info about readonly:virTristateBool */
 };
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f398e3a..67bfc36 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3611,10 +3611,30 @@ virQEMUCapsGetDefaultMachine(virQEMUCapsPtr qemuCaps)
 static int
 virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
                                 virDomainCapsLoaderPtr loader,
-                                virArch arch)
+                                virArch arch,
+                                virQEMUDriverConfigPtr cfg)
 {
+    size_t i;
+
     loader->device.supported = true;
 
+    if (VIR_ALLOC_N(loader->values.values, cfg->nloader) < 0)
+        return -1;
+
+    for (i = 0; i < cfg->nloader; i++) {
+        const char *filename = cfg->loader[i];
+
+        if (!virFileExists(filename)) {
+            VIR_DEBUG("loader filename=%s does not exist", filename);
+            continue;
+        }
+
+        if (VIR_STRDUP(loader->values.values[loader->values.nvalues],
+                       filename) < 0)
+            return -1;
+        loader->values.nvalues++;
+    }
+
     VIR_DOMAIN_CAPS_ENUM_SET(loader->type,
                              VIR_DOMAIN_LOADER_TYPE_ROM);
 
@@ -3636,12 +3656,13 @@ virQEMUCapsFillDomainLoaderCaps(virQEMUCapsPtr qemuCaps,
 static int
 virQEMUCapsFillDomainOSCaps(virQEMUCapsPtr qemuCaps,
                             virDomainCapsOSPtr os,
-                            virArch arch)
+                            virArch arch,
+                            virQEMUDriverConfigPtr cfg)
 {
     virDomainCapsLoaderPtr loader = &os->loader;
 
     os->device.supported = true;
-    if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch) < 0)
+    if (virQEMUCapsFillDomainLoaderCaps(qemuCaps, loader, arch, cfg) < 0)
         return -1;
     return 0;
 }
@@ -3725,7 +3746,8 @@ virQEMUCapsFillDomainDeviceHostdevCaps(virQEMUCapsPtr qemuCaps,
 
 int
 virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
-                          virQEMUCapsPtr qemuCaps)
+                          virQEMUCapsPtr qemuCaps,
+                          virQEMUDriverConfigPtr cfg)
 {
     virDomainCapsOSPtr os = &domCaps->os;
     virDomainCapsDeviceDiskPtr disk = &domCaps->disk;
@@ -3734,7 +3756,7 @@ virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
 
     domCaps->maxvcpus = maxvcpus;
 
-    if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps->arch) < 0 ||
+    if (virQEMUCapsFillDomainOSCaps(qemuCaps, os, domCaps->arch, cfg) < 0 ||
         virQEMUCapsFillDomainDeviceDiskCaps(qemuCaps, disk) < 0 ||
         virQEMUCapsFillDomainDeviceHostdevCaps(qemuCaps, hostdev) < 0)
         return -1;
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 828bba3..cf69e59 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -324,7 +324,12 @@ int virQEMUCapsInitGuestFromBinary(virCapsPtr caps,
                                    virQEMUCapsPtr kvmbinCaps,
                                    virArch guestarch);
 
+/* Forward declaration */
+typedef struct _virQEMUDriverConfig virQEMUDriverConfig;
+typedef virQEMUDriverConfig *virQEMUDriverConfigPtr;
+
 int virQEMUCapsFillDomainCaps(virDomainCapsPtr domCaps,
-                              virQEMUCapsPtr qemuCaps);
+                              virQEMUCapsPtr qemuCaps,
+                              virQEMUDriverConfigPtr cfg);
 
 #endif /* __QEMU_CAPABILITIES_H__*/
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 446dd63..d3ff143 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17068,12 +17068,15 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
     int virttype; /* virDomainVirtType */
     virDomainCapsPtr domCaps = NULL;
     int arch = virArchFromHost(); /* virArch */
+    virQEMUDriverConfigPtr cfg = NULL;
 
     virCheckFlags(0, ret);
 
     if (virConnectGetDomainCapabilitiesEnsureACL(conn) < 0)
         return ret;
 
+    cfg = virQEMUDriverGetConfig(driver);
+
     if (qemuHostdevHostSupportsPassthroughLegacy())
         virttype = VIR_DOMAIN_VIRT_KVM;
     else
@@ -17140,11 +17143,12 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
     if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, virttype)))
         goto cleanup;
 
-    if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps) < 0)
+    if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, cfg) < 0)
         goto cleanup;
 
     ret = virDomainCapsFormat(domCaps);
  cleanup:
+    virObjectUnref(cfg);
     virObjectUnref(domCaps);
     virObjectUnref(qemuCaps);
     return ret;
diff --git a/tests/domaincapsschemadata/domaincaps-full.xml b/tests/domaincapsschemadata/domaincaps-full.xml
index 9722772..96202bc 100644
--- a/tests/domaincapsschemadata/domaincaps-full.xml
+++ b/tests/domaincapsschemadata/domaincaps-full.xml
@@ -6,6 +6,8 @@
   <vcpu max='255'/>
   <os supported='yes'>
     <loader supported='yes'>
+      <value>/foo/bar</value>
+      <value>/tmp/my_path</value>
       <enum name='type'>
         <value>rom</value>
         <value>pflash</value>
diff --git a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
index 568cecb..346ef65 100644
--- a/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
+++ b/tests/domaincapsschemadata/domaincaps-qemu_1.6.50-1.xml
@@ -5,6 +5,7 @@
   <arch>x86_64</arch>
   <os supported='yes'>
     <loader supported='yes'>
+      <value>/usr/share/OVMF/OVMF_CODE.fd</value>
       <enum name='type'>
         <value>rom</value>
         <value>pflash</value>
diff --git a/tests/domaincapstest.c b/tests/domaincapstest.c
index 0c4b09f..8543963 100644
--- a/tests/domaincapstest.c
+++ b/tests/domaincapstest.c
@@ -34,6 +34,27 @@ typedef int (*virDomainCapsFill)(virDomainCapsPtr domCaps,
 #define SET_ALL_BITS(x) \
     memset(&(x.values), 0xff, sizeof(x.values))
 
+static int ATTRIBUTE_SENTINEL
+fillStringValues(virDomainCapsStringValuesPtr values, ...)
+{
+    int ret = 0;
+    va_list list;
+    const char *str;
+
+    va_start(list, values);
+    while ((str = va_arg(list, const char *))) {
+        if (VIR_REALLOC_N(values->values, values->nvalues + 1) < 0 ||
+            VIR_STRDUP(values->values[values->nvalues], str) < 0) {
+            ret = -1;
+            break;
+        }
+        values->nvalues++;
+    }
+    va_end(list);
+
+    return ret;
+}
+
 static int
 fillAll(virDomainCapsPtr domCaps,
         void *opaque ATTRIBUTE_UNUSED)
@@ -49,6 +70,11 @@ fillAll(virDomainCapsPtr domCaps,
     loader->device.supported = true;
     SET_ALL_BITS(loader->type);
     SET_ALL_BITS(loader->readonly);
+    if (fillStringValues(&loader->values,
+                         "/foo/bar",
+                         "/tmp/my_path",
+                         NULL) < 0)
+        return -1;
 
     disk->device.supported = true;
     SET_ALL_BITS(disk->diskDevice);
@@ -66,13 +92,21 @@ fillAll(virDomainCapsPtr domCaps,
 
 #ifdef WITH_QEMU
 # include "testutilsqemu.h"
+
+struct fillQemuCapsData {
+    virQEMUCapsPtr qemuCaps;
+    virQEMUDriverConfigPtr cfg;
+};
+
 static int
 fillQemuCaps(virDomainCapsPtr domCaps,
              void *opaque)
 {
-    virQEMUCapsPtr qemuCaps = (virQEMUCapsPtr) opaque;
+    struct fillQemuCapsData *data = (struct fillQemuCapsData *) opaque;
+    virQEMUCapsPtr qemuCaps = data->qemuCaps;
+    virQEMUDriverConfigPtr cfg = data->cfg;
 
-    if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps) < 0)
+    if (virQEMUCapsFillDomainCaps(domCaps, qemuCaps, cfg) < 0)
         return -1;
 
     /* The function above tries to query host's KVM & VFIO capabilities by
@@ -97,7 +131,7 @@ buildVirDomainCaps(const char *emulatorbin,
                    virDomainCapsFill fillFunc,
                    void *opaque)
 {
-    virDomainCapsPtr domCaps;
+    virDomainCapsPtr domCaps, ret = NULL;
 
     if (!(domCaps = virDomainCapsNew(emulatorbin, machine, arch, type)))
         goto cleanup;
@@ -107,8 +141,9 @@ buildVirDomainCaps(const char *emulatorbin,
         domCaps = NULL;
     }
 
+    ret = domCaps;
  cleanup:
-    return domCaps;
+    return ret;
 }
 
 struct test_virDomainCapsFormatData {
@@ -182,13 +217,16 @@ mymain(void)
 
 #ifdef WITH_QEMU
 
+    virQEMUDriverConfigPtr cfg = virQEMUDriverConfigNew(false);
+
 # define DO_TEST_QEMU(Filename, QemuCapsFile, Emulatorbin, Machine, Arch, Type, ...)    \
     do {                                                                                \
         const char *capsPath = abs_srcdir "/qemucapabilitiesdata/" QemuCapsFile ".caps";    \
         virQEMUCapsPtr qemuCaps = qemuTestParseCapabilities(capsPath);                  \
+        struct fillQemuCapsData fillData = {.qemuCaps = qemuCaps, .cfg = cfg};          \
         struct test_virDomainCapsFormatData data = {.filename = Filename,               \
             .emulatorbin = Emulatorbin, .machine = Machine, .arch = Arch,               \
-            .type = Type, .fillFunc = fillQemuCaps, .opaque = qemuCaps};                \
+            .type = Type, .fillFunc = fillQemuCaps, .opaque = &fillData};               \
         if (!qemuCaps) {                                                                \
             fprintf(stderr, "Unable to build qemu caps from %s\n", capsPath);           \
             ret = -1;                                                                   \
@@ -199,6 +237,7 @@ mymain(void)
     DO_TEST_QEMU("qemu_1.6.50-1", "caps_1.6.50-1", "/usr/bin/qemu-system-x86_64",
                  "pc-1.2",  VIR_ARCH_X86_64, VIR_DOMAIN_VIRT_KVM);
 
+    virObjectUnref(cfg);
 #endif /* WITH_QEMU */
 
     return ret;
-- 
2.1.0