--- docs/schemas/domaincommon.rng | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index fb6a26c..94a72f8 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4118,17 +4118,17 @@ <interleave> <optional> <element name="relaxed"> - <ref name="hypervtristate"/> + <ref name="featurestate"/> </element> </optional> <optional> <element name="vapic"> - <ref name="hypervtristate"/> + <ref name="featurestate"/> </element> </optional> <optional> <element name="spinlocks"> - <ref name="hypervtristate"/> + <ref name="featurestate"/> <optional> <attribute name="retries"> <data type="unsignedInt"/> @@ -4140,7 +4140,7 @@ </element> </define> - <define name="hypervtristate"> + <define name="featurestate"> <attribute name="state"> <choice> <value>on</value> -- 1.8.3.2

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; + } } /* APIC EOI */ @@ -13380,7 +13395,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: @@ -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); + 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 9414ebf..9ddd706 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1987,7 +1987,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 d4226b8..79cf2b6 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -603,11 +603,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 c60f5d8..2479310 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -1865,8 +1865,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; @@ -1947,7 +1947,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 e6239c9..6f3b3cb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6608,7 +6608,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; @@ -7943,7 +7943,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"); } @@ -10726,7 +10726,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, @@ -10981,7 +10981,7 @@ 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; } } @@ -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; \ } @@ -11289,7 +11289,7 @@ 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")) { @@ -11407,7 +11407,7 @@ 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 5e5ea85..99e1446 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2363,7 +2363,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 @@ -2371,21 +2370,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 6209c68..c61336c 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -1201,15 +1201,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 */ @@ -2332,15 +2332,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.3.2

The linux kernel recently added support for paravirtual spinlock handling to avoid performance regressions on overcomitted hosts. This feature needs to be turned in the hypervisor so that the guest OS is notified about the possible support. This patch adds a new feature "paravirt-spinlock" to the XML and supporting code to enable the "kvm_pv_unhalt" pseudoCPU feature in qemu. https://bugzilla.redhat.com/show_bug.cgi?id=1008989 --- This patch is RFC as qemu didn't add this to the upstream repo yet: Pull request: http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03385.html Original thread: http://lists.nongnu.org/archive/html/qemu-devel/2013-09/msg03095.html I'm sending it to get review feedback but will push it only after qemu will have it. docs/formatdomain.html.in | 7 ++++++ docs/schemas/domaincommon.rng | 10 ++++++++- src/conf/domain_conf.c | 20 ++++++++++++++++- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 14 ++++++++++++ .../qemuxml2argv-pv-spinlock-disabled.args | 5 +++++ .../qemuxml2argv-pv-spinlock-disabled.xml | 26 ++++++++++++++++++++++ .../qemuxml2argv-pv-spinlock-enabled.args | 5 +++++ .../qemuxml2argv-pv-spinlock-enabled.xml | 26 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 2 ++ 11 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 3689399..aa0076c 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1182,6 +1182,7 @@ &lt;vapic state='on'/&gt; &lt;spinlocks state='on' retries='4096'/&gt; &lt;/hyperv&gt; + &lt;paravirt-spinlock/&gt; &lt;/features&gt; ...</pre> @@ -1253,6 +1254,12 @@ </tr> </table> </dd> + <dt><code>paravirtual-spinlock</code></dt> + <dd>Notify the guest that the host supports paravirtual spinlocks + for example by exposing the pvticketlocks mechanism. This feature + can be forced of by using a "state='off'" attribute. + </dd> + </dl> <h3><a name="elementsTime">Time keeping</a></h3> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 94a72f8..4c494cb 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3560,7 +3560,7 @@ </define> <!-- A set of optional features: PAE, APIC, ACPI, - HyperV Enlightenment and HAP support + HyperV Enlightenment, paravirtual spinlocks and HAP support --> <define name="features"> <optional> @@ -3606,6 +3606,14 @@ <empty/> </element> </optional> + <optional> + <element name="paravirt-spinlock"> + <optional> + <ref name="featurestate"/> + </optional> + <empty/> + </element> + </optional> </interleave> </element> </optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e1a1d6d..919adc1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -142,7 +142,8 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST, "hap", "viridian", "privnet", - "hyperv") + "hyperv", + "paravirt-spinlock") VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, "default", @@ -11394,6 +11395,22 @@ virDomainDefParseXML(xmlDocPtr xml, def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; break; + case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK: + node = ctxt->node; + ctxt->node = nodes[i]; + if ((tmp = virXPathString("string(./@state)", ctxt))) { + if ((def->features[val] = virDomainFeatureStateTypeFromString(tmp)) == -1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown state atribute '%s' of feature '%s'"), + tmp, virDomainFeatureTypeToString(val)); + goto error; + } + } else { + def->features[val] = VIR_DOMAIN_FEATURE_STATE_ON; + } + ctxt->node = node; + break; + case VIR_DOMAIN_FEATURE_LAST: break; } @@ -16742,6 +16759,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, case VIR_DOMAIN_FEATURE_HAP: case VIR_DOMAIN_FEATURE_VIRIDIAN: case VIR_DOMAIN_FEATURE_PRIVNET: + case VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK: switch ((enum virDomainFeatureState) def->features[i]) { case VIR_DOMAIN_FEATURE_STATE_ON: /* output just the element */ diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 9ddd706..4d69c4f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1618,6 +1618,7 @@ enum virDomainFeature { VIR_DOMAIN_FEATURE_VIRIDIAN, VIR_DOMAIN_FEATURE_PRIVNET, VIR_DOMAIN_FEATURE_HYPERV, + VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK, VIR_DOMAIN_FEATURE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6f3b3cb..01de440 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6608,6 +6608,20 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, have_cpu = true; } + if (def->features[VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK]) { + char sign; + if (def->features[VIR_DOMAIN_FEATURE_PARAVIRT_SPINLOCK] == + VIR_DOMAIN_FEATURE_STATE_ON) + sign = '+'; + else + sign = '-'; + + virBufferAsprintf(&buf, "%s,%ckvm_pv_unhalt", + have_cpu ? "" : default_model, + sign); + have_cpu = true; + } + if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_DOMAIN_FEATURE_STATE_ON) { if (!have_cpu) { virBufferAdd(&buf, default_model, -1); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args new file mode 100644 index 0000000..80047f9 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc \ +-cpu qemu32,-kvm_pv_unhalt -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -usb -net none -serial \ +none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml new file mode 100644 index 0000000..afb0b41 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-disabled.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <features> + <acpi/> + <pae/> + <paravirt-spinlock state='off'/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args new file mode 100644 index 0000000..70db173 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.args @@ -0,0 +1,5 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S -M pc \ +-cpu qemu32,+kvm_pv_unhalt -m 214 -smp 6 -nographic -monitor \ +unix:/tmp/test-monitor,server,nowait -boot n -usb -net none -serial \ +none -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml new file mode 100644 index 0000000..f29cade --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pv-spinlock-enabled.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>6</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='network'/> + </os> + <features> + <acpi/> + <pae/> + <paravirt-spinlock/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ec4a6e5..72b6b2a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -435,6 +435,8 @@ mymain(void) QEMU_CAPS_CHARDEV_SPICEVMC, QEMU_CAPS_SPICE, QEMU_CAPS_HDA_DUPLEX); DO_TEST("eoi-disabled", NONE); DO_TEST("eoi-enabled", NONE); + DO_TEST("pv-spinlock-disabled", NONE); + DO_TEST("pv-spinlock-enabled", NONE); DO_TEST("kvmclock+eoi-disabled", QEMU_CAPS_ENABLE_KVM); DO_TEST("hyperv", NONE); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index c661573..0e83dcf 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -156,6 +156,8 @@ mymain(void) DO_TEST("cpu-eoi-enabled"); DO_TEST("eoi-disabled"); DO_TEST("eoi-enabled"); + DO_TEST("pv-spinlock-disabled"); + DO_TEST("pv-spinlock-enabled"); DO_TEST("hyperv"); DO_TEST("hyperv-off"); -- 1.8.3.2

On Tue, Sep 24, 2013 at 3:43 AM, Peter Krempa <pkrempa(a)redhat.com&gt; wrote: You call it paravirt-spinlock everywhere else so this looks like a typo. Just one typo that in addition to Jan's review. -- Doug Goldstein