conf: Refactor storage of guest capabilities

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@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 7edec75c310a9b40c8c124f5fe9bb3a4fdcc6d4c..67776b4fd3326895e6ca6bbad8b1fe201a480102 100644 (file)
--- 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;
 }
 
 /**
@@ -1192,6 +1204,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,
@@ -1261,29 +1307,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 4abd3dcabd0754e8f24bc98526c0e3297ff5f16e..dd972b1a677880aa0a7467b985ef0e3668fa2c29 100644 (file)
--- 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 f6572c8ecd4b5a30f7df3c15f366c1dfb4981eeb..5dca6d3102753052ab2474b0e17787304f9cb12a 100644 (file)
--- 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 d41693a00173fe2dc17c27569da817ce7343ebc7..35359780c42fc4d9e5a2bda51c4b795e3b477567 100644 (file)
--- 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>