[libvirt] [PATCH 2/3] conf: Refactor storage of guest capabilities

Peter Krempa pkrempa at redhat.com
Tue Nov 12 13:33:45 UTC 2019


The capabilities are declared in the XML schema so passing feature names
as strings from hypervisor drivers makes no sense.

Additionally some of the features expose so called 'toggles' while
others not. This knowledge was encoded by a bunch of 'STREQ's in the
formatter.

Change all of this by declaring the features as an enum and use it
instead of a dynamically allocated array.

Presence of 'toggles' is encoded together with the conversion strings
rather than in the formatter directly.

Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
 src/conf/capabilities.c                    | 123 +++++++++++++--------
 src/conf/capabilities.h                    |  24 +++-
 tests/qemucaps2xmloutdata/caps.aarch64.xml |   2 +-
 tests/qemucaps2xmloutdata/caps.x86_64.xml  |   4 +-
 4 files changed, 95 insertions(+), 58 deletions(-)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 953464b09d..1666e1e496 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -150,15 +150,6 @@ virCapabilitiesFreeGuestDomain(virCapsGuestDomainPtr dom)
     VIR_FREE(dom);
 }

-static void
-virCapabilitiesFreeGuestFeature(virCapsGuestFeaturePtr feature)
-{
-    if (feature == NULL)
-        return;
-    VIR_FREE(feature->name);
-    VIR_FREE(feature);
-}
-
 void
 virCapabilitiesFreeGuest(virCapsGuestPtr guest)
 {
@@ -176,10 +167,6 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
         virCapabilitiesFreeGuestDomain(guest->arch.domains[i]);
     VIR_FREE(guest->arch.domains);

-    for (i = 0; i < guest->nfeatures; i++)
-        virCapabilitiesFreeGuestFeature(guest->features[i]);
-    VIR_FREE(guest->features);
-
     VIR_FREE(guest);
 }

@@ -552,6 +539,24 @@ virCapabilitiesAddGuestDomain(virCapsGuestPtr guest,
 }


+struct virCapsGuestFeatureInfo {
+    const char *name;
+    bool togglesRequired;
+};
+
+static const struct virCapsGuestFeatureInfo virCapsGuestFeatureInfos[VIR_CAPS_GUEST_FEATURE_TYPE_LAST] = {
+    [VIR_CAPS_GUEST_FEATURE_TYPE_PAE] = { "pae", false },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_NONPAE] = { "nonpae", false },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_IA64_BE] = { "ia64_be", false },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_ACPI] = { "acpi", true },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_APIC] = { "apic", true },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_CPUSELECTION] = { "cpuselection", false },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT] = { "deviceboot", false },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT] = { "disksnapshot", true },
+    [VIR_CAPS_GUEST_FEATURE_TYPE_HAP] = { "hap", true },
+};
+
+
 /**
  * virCapabilitiesAddGuestFeature:
  * @guest: guest to associate feature with
@@ -567,25 +572,32 @@ virCapabilitiesAddGuestFeature(virCapsGuestPtr guest,
                                bool defaultOn,
                                bool toggle)
 {
-    virCapsGuestFeaturePtr feature;
+    virCapsGuestFeaturePtr feature = NULL;
+    bool togglesRequired = false;
+    size_t i;

-    if (VIR_ALLOC(feature) < 0)
-        goto no_memory;
+    for (i = 0; i < VIR_CAPS_GUEST_FEATURE_TYPE_LAST; i++) {
+        if (STRNEQ(name, virCapsGuestFeatureInfos[i].name))
+            continue;

-    feature->name = g_strdup(name);
-    feature->defaultOn = defaultOn;
-    feature->toggle = toggle;
+        feature = guest->features + i;
+        togglesRequired = virCapsGuestFeatureInfos[i].togglesRequired;
+    }

-    if (VIR_RESIZE_N(guest->features, guest->nfeatures_max,
-                     guest->nfeatures, 1) < 0)
-        goto no_memory;
-    guest->features[guest->nfeatures++] = feature;
+    if (!feature) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("invalid feature '%s'"), name);
+        return NULL;
+    }

-    return feature;
+    feature->present = true;

- no_memory:
-    virCapabilitiesFreeGuestFeature(feature);
-    return NULL;
+    if (togglesRequired) {
+        feature->defaultOn = virTristateSwitchFromBool(defaultOn);
+        feature->toggle = virTristateBoolFromBool(toggle);
+    }
+
+    return feature;
 }

 /**
@@ -1204,6 +1216,40 @@ virCapabilitiesFormatHostXML(virCapsHostPtr host,
 }


+static void
+virCapabilitiesFormatGuestFeatures(virCapsGuestPtr guest,
+                                   virBufferPtr buf)
+{
+    g_auto(virBuffer) childBuf = VIR_BUFFER_INITIALIZER;
+    size_t i;
+
+    virBufferSetChildIndent(&childBuf, buf);
+
+    for (i = 0; i < VIR_CAPS_GUEST_FEATURE_TYPE_LAST; i++) {
+        virCapsGuestFeaturePtr feature = guest->features + i;
+
+        if (!feature->present)
+            continue;
+
+        virBufferAsprintf(&childBuf, "<%s", virCapsGuestFeatureInfos[i].name);
+
+        if (feature->defaultOn) {
+            virBufferAsprintf(&childBuf, " default='%s'",
+                              virTristateSwitchTypeToString(feature->defaultOn));
+        }
+
+        if (feature->toggle) {
+            virBufferAsprintf(&childBuf, " toggle='%s'",
+                              virTristateBoolTypeToString(feature->toggle));
+        }
+
+        virBufferAddLit(&childBuf, "/>\n");
+    }
+
+    virXMLFormatElement(buf, "features", NULL, &childBuf);
+}
+
+
 static void
 virCapabilitiesFormatGuestXML(virCapsGuestPtr *guests,
                               size_t nguests,
@@ -1273,29 +1319,8 @@ virCapabilitiesFormatGuestXML(virCapsGuestPtr *guests,
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</arch>\n");

-        if (guests[i]->nfeatures) {
-            virBufferAddLit(buf, "<features>\n");
-            virBufferAdjustIndent(buf, 2);
+        virCapabilitiesFormatGuestFeatures(guests[i], buf);

-            for (j = 0; j < guests[i]->nfeatures; j++) {
-                if (STREQ(guests[i]->features[j]->name, "pae") ||
-                    STREQ(guests[i]->features[j]->name, "nonpae") ||
-                    STREQ(guests[i]->features[j]->name, "ia64_be") ||
-                    STREQ(guests[i]->features[j]->name, "cpuselection") ||
-                    STREQ(guests[i]->features[j]->name, "deviceboot")) {
-                    virBufferAsprintf(buf, "<%s/>\n",
-                                      guests[i]->features[j]->name);
-                } else {
-                    virBufferAsprintf(buf, "<%s default='%s' toggle='%s'/>\n",
-                                      guests[i]->features[j]->name,
-                                      guests[i]->features[j]->defaultOn ? "on" : "off",
-                                      guests[i]->features[j]->toggle ? "yes" : "no");
-                }
-            }
-
-            virBufferAdjustIndent(buf, -2);
-            virBufferAddLit(buf, "</features>\n");
-        }
         virBufferAdjustIndent(buf, -2);
         virBufferAddLit(buf, "</guest>\n\n");
     }
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index 4abd3dcabd..dd972b1a67 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -32,10 +32,24 @@

 #include <libxml/xpath.h>

+typedef enum {
+    VIR_CAPS_GUEST_FEATURE_TYPE_PAE = 0,
+    VIR_CAPS_GUEST_FEATURE_TYPE_NONPAE,
+    VIR_CAPS_GUEST_FEATURE_TYPE_IA64_BE,
+    VIR_CAPS_GUEST_FEATURE_TYPE_ACPI,
+    VIR_CAPS_GUEST_FEATURE_TYPE_APIC,
+    VIR_CAPS_GUEST_FEATURE_TYPE_CPUSELECTION,
+    VIR_CAPS_GUEST_FEATURE_TYPE_DEVICEBOOT,
+    VIR_CAPS_GUEST_FEATURE_TYPE_DISKSNAPSHOT,
+    VIR_CAPS_GUEST_FEATURE_TYPE_HAP,
+
+    VIR_CAPS_GUEST_FEATURE_TYPE_LAST
+} virCapsGuestFeatureType;
+
 struct _virCapsGuestFeature {
-    char *name;
-    bool defaultOn;
-    bool toggle;
+    bool present;
+    virTristateSwitch defaultOn;
+    virTristateBool toggle;
 };

 struct _virCapsGuestMachine {
@@ -68,9 +82,7 @@ struct _virCapsGuestArch {
 struct _virCapsGuest {
     int ostype;
     virCapsGuestArch arch;
-    size_t nfeatures;
-    size_t nfeatures_max;
-    virCapsGuestFeaturePtr *features;
+    virCapsGuestFeature features[VIR_CAPS_GUEST_FEATURE_TYPE_LAST];
 };

 struct _virCapsHostNUMACellCPU {
diff --git a/tests/qemucaps2xmloutdata/caps.aarch64.xml b/tests/qemucaps2xmloutdata/caps.aarch64.xml
index f6572c8ecd..5dca6d3102 100644
--- a/tests/qemucaps2xmloutdata/caps.aarch64.xml
+++ b/tests/qemucaps2xmloutdata/caps.aarch64.xml
@@ -17,10 +17,10 @@
       <domain type='kvm'/>
     </arch>
     <features>
+      <acpi default='on' toggle='yes'/>
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
-      <acpi default='on' toggle='yes'/>
     </features>
   </guest>

diff --git a/tests/qemucaps2xmloutdata/caps.x86_64.xml b/tests/qemucaps2xmloutdata/caps.x86_64.xml
index d41693a001..35359780c4 100644
--- a/tests/qemucaps2xmloutdata/caps.x86_64.xml
+++ b/tests/qemucaps2xmloutdata/caps.x86_64.xml
@@ -17,11 +17,11 @@
       <domain type='kvm'/>
     </arch>
     <features>
+      <acpi default='on' toggle='yes'/>
+      <apic default='on' toggle='no'/>
       <cpuselection/>
       <deviceboot/>
       <disksnapshot default='on' toggle='no'/>
-      <acpi default='on' toggle='yes'/>
-      <apic default='on' toggle='no'/>
     </features>
   </guest>

-- 
2.23.0




More information about the libvir-list mailing list