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>

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)
+    for (i = 0; i < VIR_CAPS_GUEST_FEATURE_TYPE_LAST; i++) {
-        goto no_memory;
+        if (STRNEQ(name, virCapsGuestFeatureInfos[i].name))
+            continue;
 
-    feature->name = g_strdup(name);
+        feature = guest->features + i;
-    feature->defaultOn = defaultOn;
+        togglesRequired = virCapsGuestFeatureInfos[i].togglesRequired;
-    feature->toggle = toggle;
+    }
 
-    if (VIR_RESIZE_N(guest->features, guest->nfeatures_max,
+    if (!feature) {
-                     guest->nfeatures, 1) < 0)
+        virReportError(VIR_ERR_INTERNAL_ERROR,
-        goto no_memory;
+                       _("invalid feature '%s'"), name);
-    guest->features[guest->nfeatures++] = feature;
+        return NULL;
+    }
+
+    feature->present = true;
+
+    if (togglesRequired) {
+        feature->defaultOn = virTristateSwitchFromBool(defaultOn);
+        feature->toggle = virTristateBoolFromBool(toggle);
+    }
 
     return feature;
-
- no_memory:
-    virCapabilitiesFreeGuestFeature(feature);
-    return NULL;
 }
 
 /**
@@ -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) {
+        virCapabilitiesFormatGuestFeatures(guests[i], buf);
-            virBufferAddLit(buf, "<features>\n");
-            virBufferAdjustIndent(buf, 2);
 
-            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");
     }

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 present;
-    bool defaultOn;
+    virTristateSwitch defaultOn;
-    bool toggle;
+    virTristateBool toggle;
 };
 
 struct _virCapsGuestMachine {
@@ -68,9 +82,7 @@ struct _virCapsGuestArch {
 struct _virCapsGuest {
     int ostype;
     virCapsGuestArch arch;
-    size_t nfeatures;
+    virCapsGuestFeature features[VIR_CAPS_GUEST_FEATURE_TYPE_LAST];
-    size_t nfeatures_max;
-    virCapsGuestFeaturePtr *features;
 };
 
 struct _virCapsHostNUMACellCPU {

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>
 

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>