Re: [libvirt] [PATCH 2/3] conf: Refactor storing and usage of feature flags

To allow a finer control for some future flags, refactor the code to store them in an array instead of a bitmap. --- src/conf/domain_conf.c | 166 ++++++++++++++++++++++++++++++--------------- src/conf/domain_conf.h | 2 +- src/libxl/libxl_conf.c | 9 ++- src/lxc/lxc_container.c | 6 +- src/qemu/qemu_command.c | 16 ++--- 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, 191 insertions(+), 135 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8953579..e1a1d6d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -11367,29 +11367,40 @@ virDomainDefParseXML(xmlDocPtr xml, int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name); if (val < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected feature %s"), - nodes[i]->name); + _("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, - _("unknown value for attribute eoi: %s"), + _("unknown value for attribute eoi: '%s'"),

tmp); goto error; } - def->apic_eoi = eoi; + 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; @@ -13361,12 +13372,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; + }

@@ -16703,58 +16718,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_ON: + /* output just the element */ + virBufferAsprintf(buf, " <%s/>\n", name); + break; +

+ case VIR_DOMAIN_FEATURE_STATE_OFF: + /* explicit off state */ + virBufferAsprintf(buf, " <%s state='off'/>\n", name); + break;

+ + case VIR_DOMAIN_FEATURE_STATE_DEFAULT: + case VIR_DOMAIN_FEATURE_STATE_LAST: + /* don't output the element */ break; + }

- case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (def->hyperv_features[i] == 0) - 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);

@@ -10994,13 +10994,13 @@ qemuParseCommandLine(virCapsPtr qemuCaps, if ((def->os.arch == VIR_ARCH_I686) || (def->os.arch == VIR_ARCH_X86_64)) - def->features |= (1 << VIR_DOMAIN_FEATURE_ACPI) + def->features[VIR_DOMAIN_FEATURE_ACPI] = VIR_DOMAIN_FEATURE_STATE_ON; /*| (1 << VIR_DOMAIN_FEATURE_APIC)*/;

#define WANT_VALUE() \ const char *val = progargv[++i]; \ if (!val) { \ - virReportError(VIR_ERR_INTERNAL_ERROR, \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ _("missing value for %s argument"), arg); \ goto error; \ }