Blob Blame History Raw
From bbad015e6ffe6623b55a1c61aeb0a8d3e65bce6a Mon Sep 17 00:00:00 2001
Message-Id: <bbad015e6ffe6623b55a1c61aeb0a8d3e65bce6a.1383922567.git.jdenemar@redhat.com>
From: Peter Krempa <pkrempa@redhat.com>
Date: Fri, 8 Nov 2013 12:33:33 +0100
Subject: [PATCH] conf: Refactor storing and usage of feature flags

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

Currently we were storing domain feature flags in a bit field as the
they were either enabled or disabled. New features such as paravirtual
spinlocks however can be tri-state as the default option may depend on
hypervisor version.

To allow storing tri-state feature state in the same place instead of
having to declare dedicated variables for each feature this patch
refactors the bit field to an array.

(cherry picked from commit de7b5faf43645229c74843032402c0d58a397e88)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/conf/domain_conf.c     | 159 ++++++++++++++++++++++++++++++---------------
 src/conf/domain_conf.h     |   2 +-
 src/libxl/libxl_conf.c     |   9 ++-
 src/lxc/lxc_container.c    |   6 +-
 src/qemu/qemu_command.c    |  15 ++---
 src/vbox/vbox_tmpl.c       |  45 +++++++------
 src/xenapi/xenapi_driver.c |  10 +--
 src/xenapi/xenapi_utils.c  |  22 +++----
 src/xenxs/xen_sxpr.c       |  20 +++---
 src/xenxs/xen_xm.c         |  30 ++++-----
 10 files changed, 187 insertions(+), 131 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 79dfb05..6664c2a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11319,10 +11319,10 @@ virDomainDefParseXML(xmlDocPtr xml,
                            _("unexpected feature '%s'"), nodes[i]->name);
             goto error;
         }
-        def->features |= (1 << val);
-        if (val == VIR_DOMAIN_FEATURE_APIC) {
-            tmp = virXPathString("string(./features/apic/@eoi)", ctxt);
-            if (tmp) {
+
+        switch ((enum virDomainFeature) val) {
+        case VIR_DOMAIN_FEATURE_APIC:
+            if ((tmp = virXPathString("string(./features/apic/@eoi)", ctxt))) {
                 int eoi;
                 if ((eoi = virDomainFeatureStateTypeFromString(tmp)) <= 0) {
                     virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -11333,11 +11333,23 @@ virDomainDefParseXML(xmlDocPtr xml,
                 def->apic_eoi = eoi;
                 VIR_FREE(tmp);
             }
+            /* fallthrough */
+        case VIR_DOMAIN_FEATURE_ACPI:
+        case VIR_DOMAIN_FEATURE_PAE:
+        case VIR_DOMAIN_FEATURE_HAP:
+        case VIR_DOMAIN_FEATURE_VIRIDIAN:
+        case VIR_DOMAIN_FEATURE_PRIVNET:
+        case VIR_DOMAIN_FEATURE_HYPERV:
+            def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON;
+            break;
+
+        case VIR_DOMAIN_FEATURE_LAST:
+            break;
         }
     }
     VIR_FREE(nodes);
 
-    if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) {
+    if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) {
         int feature;
         int value;
         node = ctxt->node;
@@ -13321,12 +13333,16 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
 {
     size_t i;
 
-    /* basic check */
-    if (src->features != dst->features) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                       _("Target domain features %d does not match source %d"),
-                       dst->features, src->features);
-        return false;
+    for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
+        if (src->features[i] != dst->features[i]) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("State of feature '%s' differs: "
+                             "source: '%s', destination: '%s'"),
+                           virDomainFeatureTypeToString(i),
+                           virDomainFeatureStateTypeToString(src->features[i]),
+                           virDomainFeatureStateTypeToString(dst->features[i]));
+            return false;
+        }
     }
 
     /* APIC EOI */
@@ -13340,7 +13356,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
     }
 
     /* hyperv */
-    if (src->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) {
+    if (src->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) {
         for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
             switch ((enum virDomainHyperv) i) {
             case VIR_DOMAIN_HYPERV_RELAXED:
@@ -16647,58 +16663,99 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         virBufferAddLit(buf, "  </idmap>\n");
     }
 
+    for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
+        if (def->features[i] != VIR_DOMAIN_FEATURE_STATE_DEFAULT)
+            break;
+    }
 
-    if (def->features) {
+    if (i != VIR_DOMAIN_FEATURE_LAST) {
         virBufferAddLit(buf, "  <features>\n");
+
         for (i = 0; i < VIR_DOMAIN_FEATURE_LAST; i++) {
-            if (def->features & (1 << i) && i != VIR_DOMAIN_FEATURE_HYPERV) {
-                const char *name = virDomainFeatureTypeToString(i);
-                if (!name) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR,
-                                   _("unexpected feature %zu"), i);
-                    goto error;
-                }
-                virBufferAsprintf(buf, "    <%s", name);
-                if (i == VIR_DOMAIN_FEATURE_APIC && def->apic_eoi) {
-                    virBufferAsprintf(buf,
-                                      " eoi='%s'",
-                                      virDomainFeatureStateTypeToString(def->apic_eoi));
-                }
-                virBufferAddLit(buf, "/>\n");
+            const char *name = virDomainFeatureTypeToString(i);
+            size_t j;
+
+            if (!name) {
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("unexpected feature %zu"), i);
+                goto error;
             }
-        }
 
-        if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) {
-            virBufferAddLit(buf, "    <hyperv>\n");
-            for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
-                switch ((enum virDomainHyperv) i) {
-                case VIR_DOMAIN_HYPERV_RELAXED:
-                case VIR_DOMAIN_HYPERV_VAPIC:
-                    if (def->hyperv_features[i])
-                        virBufferAsprintf(buf, "      <%s state='%s'/>\n",
-                                          virDomainHypervTypeToString(i),
-                                          virDomainFeatureStateTypeToString(
-                                              def->hyperv_features[i]));
+            switch ((enum virDomainFeature) i) {
+            case VIR_DOMAIN_FEATURE_ACPI:
+            case VIR_DOMAIN_FEATURE_PAE:
+            case VIR_DOMAIN_FEATURE_HAP:
+            case VIR_DOMAIN_FEATURE_VIRIDIAN:
+            case VIR_DOMAIN_FEATURE_PRIVNET:
+                switch ((enum virDomainFeatureState) def->features[i]) {
+                case VIR_DOMAIN_FEATURE_STATE_DEFAULT:
                     break;
 
-                case VIR_DOMAIN_HYPERV_SPINLOCKS:
-                    if (def->hyperv_features[i] == 0)
-                        break;
+                case VIR_DOMAIN_FEATURE_STATE_ON:
+                   virBufferAsprintf(buf, "    <%s/>\n", name);
+                   break;
+
+                case VIR_DOMAIN_FEATURE_STATE_LAST:
+                case VIR_DOMAIN_FEATURE_STATE_OFF:
+                   virReportError(VIR_ERR_INTERNAL_ERROR,
+                                 _("Unexpected state of feature '%s'"), name);
+
+                   goto error;
+                   break;
+                }
+
+                break;
 
-                    virBufferAsprintf(buf, "      <spinlocks state='%s'",
-                                      virDomainFeatureStateTypeToString(
-                                          def->hyperv_features[i]));
-                    if (def->hyperv_features[i] == VIR_DOMAIN_FEATURE_STATE_ON)
-                        virBufferAsprintf(buf, " retries='%d'",
-                                          def->hyperv_spinlocks);
+            case VIR_DOMAIN_FEATURE_APIC:
+                if (def->features[i] == VIR_DOMAIN_FEATURE_STATE_ON) {
+                    virBufferAddLit(buf, "    <apic");
+                    if (def->apic_eoi) {
+                        virBufferAsprintf(buf, " eoi='%s'",
+                                          virDomainFeatureStateTypeToString(def->apic_eoi));
+                    }
                     virBufferAddLit(buf, "/>\n");
-                    break;
+                }
+                break;
 
-                case VIR_DOMAIN_HYPERV_LAST:
+            case VIR_DOMAIN_FEATURE_HYPERV:
+                if (def->features[i] != VIR_DOMAIN_FEATURE_STATE_ON)
                     break;
+
+                virBufferAddLit(buf, "    <hyperv>\n");
+                for (j = 0; j < VIR_DOMAIN_HYPERV_LAST; j++) {
+                    switch ((enum virDomainHyperv) j) {
+                    case VIR_DOMAIN_HYPERV_RELAXED:
+                    case VIR_DOMAIN_HYPERV_VAPIC:
+                        if (def->hyperv_features[j])
+                            virBufferAsprintf(buf, "      <%s state='%s'/>\n",
+                                              virDomainHypervTypeToString(j),
+                                              virDomainFeatureStateTypeToString(
+                                                  def->hyperv_features[j]));
+                        break;
+
+                    case VIR_DOMAIN_HYPERV_SPINLOCKS:
+                        if (def->hyperv_features[j] == 0)
+                            break;
+
+                        virBufferAsprintf(buf, "      <spinlocks state='%s'",
+                                          virDomainFeatureStateTypeToString(
+                                              def->hyperv_features[j]));
+                        if (def->hyperv_features[j] == VIR_DOMAIN_FEATURE_STATE_ON)
+                            virBufferAsprintf(buf, " retries='%d'",
+                                              def->hyperv_spinlocks);
+                        virBufferAddLit(buf, "/>\n");
+                        break;
+
+                    case VIR_DOMAIN_HYPERV_LAST:
+                        break;
+                    }
                 }
+                virBufferAddLit(buf, "    </hyperv>\n");
+                break;
+
+            case VIR_DOMAIN_FEATURE_LAST:
+                break;
             }
-            virBufferAddLit(buf, "    </hyperv>\n");
         }
 
         virBufferAddLit(buf, "  </features>\n");
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5ad0318..3d2b3a4 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1972,7 +1972,7 @@ struct _virDomainDef {
 
     virDomainOSDef os;
     char *emulator;
-    int features;
+    int features[VIR_DOMAIN_FEATURE_LAST];
     /* enum virDomainFeatureState */
     int apic_eoi;
     /* These options are of type virDomainFeatureState */
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 5273a26..6bfe934 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -373,11 +373,14 @@ libxlMakeDomBuildInfo(virDomainObjPtr vm, libxl_domain_config *d_config)
         char bootorder[VIR_DOMAIN_BOOT_LAST + 1];
 
         libxl_defbool_set(&b_info->u.hvm.pae,
-                          def->features & (1 << VIR_DOMAIN_FEATURE_PAE));
+                          def->features[VIR_DOMAIN_FEATURE_PAE] ==
+                          VIR_DOMAIN_FEATURE_STATE_ON);
         libxl_defbool_set(&b_info->u.hvm.apic,
-                          def->features & (1 << VIR_DOMAIN_FEATURE_APIC));
+                          def->features[VIR_DOMAIN_FEATURE_APIC] ==
+                          VIR_DOMAIN_FEATURE_STATE_ON);
         libxl_defbool_set(&b_info->u.hvm.acpi,
-                          def->features & (1 << VIR_DOMAIN_FEATURE_ACPI));
+                          def->features[VIR_DOMAIN_FEATURE_ACPI] ==
+                          VIR_DOMAIN_FEATURE_STATE_ON);
         for (i = 0; i < def->clock.ntimers; i++) {
             if (def->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_HPET &&
                 def->clock.timers[i]->present == 1) {
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 9479498..01ed93e 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -1838,8 +1838,8 @@ static int lxcContainerChild(void *data)
     }
 
     /* rename and enable interfaces */
-    if (lxcContainerRenameAndEnableInterfaces(!!(vmDef->features &
-                                                 (1 << VIR_DOMAIN_FEATURE_PRIVNET)),
+    if (lxcContainerRenameAndEnableInterfaces(vmDef->features[VIR_DOMAIN_FEATURE_PRIVNET] ==
+                                              VIR_DOMAIN_FEATURE_STATE_ON,
                                               argv->nveths,
                                               argv->veths) < 0) {
         goto cleanup;
@@ -1929,7 +1929,7 @@ lxcNeedNetworkNamespace(virDomainDefPtr def)
     size_t i;
     if (def->nets != NULL)
         return true;
-    if (def->features & (1 << VIR_DOMAIN_FEATURE_PRIVNET))
+    if (def->features[VIR_DOMAIN_FEATURE_PRIVNET] == VIR_DOMAIN_FEATURE_STATE_ON)
         return true;
     for (i = 0; i < def->nhostdevs; i++) {
         if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES &&
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0292b72..f14b0f2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6574,7 +6574,7 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver,
         have_cpu = true;
     }
 
-    if (def->features & (1 << VIR_DOMAIN_FEATURE_HYPERV)) {
+    if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) {
         if (!have_cpu) {
             virBufferAdd(&buf, default_model, -1);
             have_cpu = true;
@@ -7905,7 +7905,7 @@ qemuBuildCommandLine(virConnectPtr conn,
     }
 
     if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)) {
-        if (!(def->features & (1 << VIR_DOMAIN_FEATURE_ACPI)))
+        if (def->features[VIR_DOMAIN_FEATURE_ACPI] != VIR_DOMAIN_FEATURE_STATE_ON)
             virCommandAddArg(cmd, "-no-acpi");
     }
 
@@ -10634,7 +10634,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
             if (*feature == '\0')
                 goto syntax;
 
-            dom->features |= (1 << VIR_DOMAIN_FEATURE_HYPERV);
+            dom->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_DOMAIN_FEATURE_STATE_ON;
 
             if ((f = virDomainHypervTypeFromString(feature)) < 0) {
                 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -10888,7 +10888,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
             goto error;
         if (strstr(path, "kvm")) {
             def->virtType = VIR_DOMAIN_VIRT_KVM;
-            def->features |= (1 << VIR_DOMAIN_FEATURE_PAE);
+            def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
         }
     }
 
@@ -10901,8 +10901,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
 
     if ((def->os.arch == VIR_ARCH_I686) ||
         (def->os.arch == VIR_ARCH_X86_64))
-        def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI)
-        /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/;
+        def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
 
 #define WANT_VALUE()                                                   \
     const char *val = progargv[++i];                                   \
@@ -11196,7 +11195,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
             def->disks[def->ndisks++] = disk;
             disk = NULL;
         } else if (STREQ(arg, "-no-acpi")) {
-            def->features &= ~(1 << VIR_DOMAIN_FEATURE_ACPI);
+            def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_DEFAULT;
         } else if (STREQ(arg, "-no-reboot")) {
             def->onReboot = VIR_DOMAIN_LIFECYCLE_DESTROY;
         } else if (STREQ(arg, "-no-kvm")) {
@@ -11314,7 +11313,7 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps,
                     def->mem.nosharepages = true;
                 } else if (STRPREFIX(param, "accel=kvm")) {
                     def->virtType = VIR_DOMAIN_VIRT_KVM;
-                    def->features |= (1 << VIR_DOMAIN_FEATURE_PAE);
+                    def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
                 }
             }
             virStringFreeList(list);
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 5b17048..3e9f0d9 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2362,7 +2362,6 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
                 }
             }
 
-            def->features = 0;
 #if VBOX_API_VERSION < 3001
             machine->vtbl->GetPAEEnabled(machine, &PAEEnabled);
 #elif VBOX_API_VERSION == 3001
@@ -2370,21 +2369,18 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) {
 #elif VBOX_API_VERSION >= 3002
             machine->vtbl->GetCPUProperty(machine, CPUPropertyType_PAE, &PAEEnabled);
 #endif
-            if (PAEEnabled) {
-                def->features = def->features | (1 << VIR_DOMAIN_FEATURE_PAE);
-            }
+            if (PAEEnabled)
+                def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
 
             machine->vtbl->GetBIOSSettings(machine, &bios);
             if (bios) {
                 bios->vtbl->GetACPIEnabled(bios, &ACPIEnabled);
-                if (ACPIEnabled) {
-                    def->features = def->features | (1 << VIR_DOMAIN_FEATURE_ACPI);
-                }
+                if (ACPIEnabled)
+                    def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
 
                 bios->vtbl->GetIOAPICEnabled(bios, &IOAPICEnabled);
-                if (IOAPICEnabled) {
-                    def->features = def->features | (1 << VIR_DOMAIN_FEATURE_APIC);
-                }
+                if (IOAPICEnabled)
+                    def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
 
                 VBOX_RELEASE(bios);
             }
@@ -5101,40 +5097,43 @@ static virDomainPtr vboxDomainDefineXML(virConnectPtr conn, const char *xml) {
     }
 
 #if VBOX_API_VERSION < 3001
-    rc = machine->vtbl->SetPAEEnabled(machine, (def->features) &
-                                      (1 << VIR_DOMAIN_FEATURE_PAE));
+    rc = machine->vtbl->SetPAEEnabled(machine,
+                                      def->features[VIR_DOMAIN_FEATURE_PAE] ==
+                                      VIR_DOMAIN_FEATURE_STATE_ON);
 #elif VBOX_API_VERSION == 3001
     rc = machine->vtbl->SetCpuProperty(machine, CpuPropertyType_PAE,
-                                       (def->features) &
-                                       (1 << VIR_DOMAIN_FEATURE_PAE));
+                                       def->features[VIR_DOMAIN_FEATURE_PAE] ==
+                                       VIR_DOMAIN_FEATURE_STATE_ON);
 #elif VBOX_API_VERSION >= 3002
     rc = machine->vtbl->SetCPUProperty(machine, CPUPropertyType_PAE,
-                                       (def->features) &
-                                       (1 << VIR_DOMAIN_FEATURE_PAE));
+                                       def->features[VIR_DOMAIN_FEATURE_PAE] ==
+                                       VIR_DOMAIN_FEATURE_STATE_ON);
 #endif
     if (NS_FAILED(rc)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("could not change PAE status to: %s, rc=%08x"),
-                       ((def->features) & (1 << VIR_DOMAIN_FEATURE_PAE))
+                       (def->features[VIR_DOMAIN_FEATURE_PAE] == VIR_DOMAIN_FEATURE_STATE_ON)
                        ? _("Enabled") : _("Disabled"), (unsigned)rc);
     }
 
     machine->vtbl->GetBIOSSettings(machine, &bios);
     if (bios) {
-        rc = bios->vtbl->SetACPIEnabled(bios, (def->features) &
-                                        (1 << VIR_DOMAIN_FEATURE_ACPI));
+        rc = bios->vtbl->SetACPIEnabled(bios,
+                                        def->features[VIR_DOMAIN_FEATURE_ACPI] ==
+                                        VIR_DOMAIN_FEATURE_STATE_ON);
         if (NS_FAILED(rc)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("could not change ACPI status to: %s, rc=%08x"),
-                           ((def->features) & (1 << VIR_DOMAIN_FEATURE_ACPI))
+                           (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_DOMAIN_FEATURE_STATE_ON)
                            ? _("Enabled") : _("Disabled"), (unsigned)rc);
         }
-        rc = bios->vtbl->SetIOAPICEnabled(bios, (def->features) &
-                                          (1 << VIR_DOMAIN_FEATURE_APIC));
+        rc = bios->vtbl->SetIOAPICEnabled(bios,
+                                          def->features[VIR_DOMAIN_FEATURE_APIC] ==
+                                          VIR_DOMAIN_FEATURE_STATE_ON);
         if (NS_FAILED(rc)) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("could not change APIC status to: %s, rc=%08x"),
-                           ((def->features) & (1 << VIR_DOMAIN_FEATURE_APIC))
+                           (def->features[VIR_DOMAIN_FEATURE_APIC] == VIR_DOMAIN_FEATURE_STATE_ON)
                            ? _("Enabled") : _("Disabled"), (unsigned)rc);
         }
         VBOX_RELEASE(bios);
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index bca19af..5f8ca08 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -1474,15 +1474,15 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
         for (i = 0; i < result->size; i++) {
             if (STREQ(result->contents[i].val, "true")) {
                 if (STREQ(result->contents[i].key, "acpi"))
-                    defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_ACPI);
+                    defPtr->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
                 else if (STREQ(result->contents[i].key, "apic"))
-                    defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_APIC);
+                    defPtr->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
                 else if (STREQ(result->contents[i].key, "pae"))
-                    defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_PAE);
+                    defPtr->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
                 else if (STREQ(result->contents[i].key, "hap"))
-                    defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_HAP);
+                    defPtr->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON;
                 else if (STREQ(result->contents[i].key, "viridian"))
-                    defPtr->features = defPtr->features | (1<<VIR_DOMAIN_FEATURE_VIRIDIAN);
+                    defPtr->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON;
             }
         }
         xen_string_string_map_free(result);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index 91ae54b..02d4885 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -511,18 +511,16 @@ createVMRecordFromXml(virConnectPtr conn, virDomainDefPtr def,
     if (def->onCrash)
         (*record)->actions_after_crash = actionCrashLibvirt2XenapiEnum(def->onCrash);
 
-    if (def->features) {
-        if (def->features & (1 << VIR_DOMAIN_FEATURE_ACPI))
-            allocStringMap(&strings, (char *)"acpi", (char *)"true");
-        if (def->features & (1 << VIR_DOMAIN_FEATURE_APIC))
-            allocStringMap(&strings, (char *)"apic", (char *)"true");
-        if (def->features & (1 << VIR_DOMAIN_FEATURE_PAE))
-            allocStringMap(&strings, (char *)"pae", (char *)"true");
-        if (def->features & (1 << VIR_DOMAIN_FEATURE_HAP))
-            allocStringMap(&strings, (char *)"hap", (char *)"true");
-        if (def->features & (1 << VIR_DOMAIN_FEATURE_VIRIDIAN))
-            allocStringMap(&strings, (char *)"viridian", (char *)"true");
-    }
+    if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_DOMAIN_FEATURE_STATE_ON)
+        allocStringMap(&strings, (char *)"acpi", (char *)"true");
+    if (def->features[VIR_DOMAIN_FEATURE_APIC] == VIR_DOMAIN_FEATURE_STATE_ON)
+        allocStringMap(&strings, (char *)"apic", (char *)"true");
+    if (def->features[VIR_DOMAIN_FEATURE_PAE] == VIR_DOMAIN_FEATURE_STATE_ON)
+        allocStringMap(&strings, (char *)"pae", (char *)"true");
+    if (def->features[VIR_DOMAIN_FEATURE_HAP] == VIR_DOMAIN_FEATURE_STATE_ON)
+        allocStringMap(&strings, (char *)"hap", (char *)"true");
+    if (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == VIR_DOMAIN_FEATURE_STATE_ON)
+        allocStringMap(&strings, (char *)"viridian", (char *)"true");
     if (strings != NULL)
         (*record)->platform = strings;
 
diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c
index fbbbaa9..fb01e0d 100644
--- a/src/xenxs/xen_sxpr.c
+++ b/src/xenxs/xen_sxpr.c
@@ -1204,15 +1204,15 @@ xenParseSxpr(const struct sexpr *root,
 
     if (hvm) {
         if (sexpr_int(root, "domain/image/hvm/acpi"))
-            def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI);
+            def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (sexpr_int(root, "domain/image/hvm/apic"))
-            def->features |= (1 << VIR_DOMAIN_FEATURE_APIC);
+            def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (sexpr_int(root, "domain/image/hvm/pae"))
-            def->features |= (1 << VIR_DOMAIN_FEATURE_PAE);
+            def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (sexpr_int(root, "domain/image/hvm/hap"))
-            def->features |= (1 << VIR_DOMAIN_FEATURE_HAP);
+            def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (sexpr_int(root, "domain/image/hvm/viridian"))
-            def->features |= (1 << VIR_DOMAIN_FEATURE_VIRIDIAN);
+            def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON;
     }
 
     /* 12aaf4a2486b (3.0.3) added a second low-priority 'localtime' setting */
@@ -2335,15 +2335,15 @@ xenFormatSxpr(virConnectPtr conn,
                 }
             }
 
-            if (def->features & (1 << VIR_DOMAIN_FEATURE_ACPI))
+            if (def->features[VIR_DOMAIN_FEATURE_ACPI] == VIR_DOMAIN_FEATURE_STATE_ON)
                 virBufferAddLit(&buf, "(acpi 1)");
-            if (def->features & (1 << VIR_DOMAIN_FEATURE_APIC))
+            if (def->features[VIR_DOMAIN_FEATURE_APIC] == VIR_DOMAIN_FEATURE_STATE_ON)
                 virBufferAddLit(&buf, "(apic 1)");
-            if (def->features & (1 << VIR_DOMAIN_FEATURE_PAE))
+            if (def->features[VIR_DOMAIN_FEATURE_PAE] == VIR_DOMAIN_FEATURE_STATE_ON)
                 virBufferAddLit(&buf, "(pae 1)");
-            if (def->features & (1 << VIR_DOMAIN_FEATURE_HAP))
+            if (def->features[VIR_DOMAIN_FEATURE_HAP] == VIR_DOMAIN_FEATURE_STATE_ON)
                 virBufferAddLit(&buf, "(hap 1)");
-            if (def->features & (1 << VIR_DOMAIN_FEATURE_VIRIDIAN))
+            if (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] == VIR_DOMAIN_FEATURE_STATE_ON)
                 virBufferAddLit(&buf, "(viridian 1)");
 
             virBufferAddLit(&buf, "(usb 1)");
diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
index 1ffea84..1a1cd12 100644
--- a/src/xenxs/xen_xm.c
+++ b/src/xenxs/xen_xm.c
@@ -398,23 +398,23 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
         if (xenXMConfigGetBool(conf, "pae", &val, 0) < 0)
             goto cleanup;
         else if (val)
-            def->features |= (1 << VIR_DOMAIN_FEATURE_PAE);
+            def->features[VIR_DOMAIN_FEATURE_PAE] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (xenXMConfigGetBool(conf, "acpi", &val, 0) < 0)
             goto cleanup;
         else if (val)
-            def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI);
+            def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (xenXMConfigGetBool(conf, "apic", &val, 0) < 0)
             goto cleanup;
         else if (val)
-            def->features |= (1 << VIR_DOMAIN_FEATURE_APIC);
+            def->features[VIR_DOMAIN_FEATURE_APIC] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (xenXMConfigGetBool(conf, "hap", &val, 0) < 0)
             goto cleanup;
         else if (val)
-            def->features |= (1 << VIR_DOMAIN_FEATURE_HAP);
+            def->features[VIR_DOMAIN_FEATURE_HAP] = VIR_DOMAIN_FEATURE_STATE_ON;
         if (xenXMConfigGetBool(conf, "viridian", &val, 0) < 0)
             goto cleanup;
         else if (val)
-            def->features |= (1 << VIR_DOMAIN_FEATURE_VIRIDIAN);
+            def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] = VIR_DOMAIN_FEATURE_STATE_ON;
 
         if (xenXMConfigGetBool(conf, "hpet", &val, -1) < 0)
             goto cleanup;
@@ -1569,29 +1569,29 @@ virConfPtr xenFormatXM(virConnectPtr conn,
             goto cleanup;
 
         if (xenXMConfigSetInt(conf, "pae",
-                              (def->features &
-                               (1 << VIR_DOMAIN_FEATURE_PAE)) ? 1 : 0) < 0)
+                              (def->features[VIR_DOMAIN_FEATURE_PAE] ==
+                               VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0)
             goto cleanup;
 
         if (xenXMConfigSetInt(conf, "acpi",
-                              (def->features &
-                               (1 << VIR_DOMAIN_FEATURE_ACPI)) ? 1 : 0) < 0)
+                              (def->features[VIR_DOMAIN_FEATURE_ACPI] ==
+                               VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0)
             goto cleanup;
 
         if (xenXMConfigSetInt(conf, "apic",
-                              (def->features &
-                               (1 << VIR_DOMAIN_FEATURE_APIC)) ? 1 : 0) < 0)
+                              (def->features[VIR_DOMAIN_FEATURE_APIC] ==
+                               VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0)
             goto cleanup;
 
         if (xendConfigVersion >= XEND_CONFIG_VERSION_3_0_4) {
             if (xenXMConfigSetInt(conf, "hap",
-                                  (def->features &
-                                   (1 << VIR_DOMAIN_FEATURE_HAP)) ? 1 : 0) < 0)
+                                  (def->features[VIR_DOMAIN_FEATURE_HAP] ==
+                                   VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0)
                 goto cleanup;
 
             if (xenXMConfigSetInt(conf, "viridian",
-                                  (def->features &
-                                   (1 << VIR_DOMAIN_FEATURE_VIRIDIAN)) ? 1 : 0) < 0)
+                                  (def->features[VIR_DOMAIN_FEATURE_VIRIDIAN] ==
+                                   VIR_DOMAIN_FEATURE_STATE_ON) ? 1 : 0) < 0)
                 goto cleanup;
         }
 
-- 
1.8.4.2