[libvirt] [PATCH v3 0/2] Add new HyperV features
v1: https://www.redhat.com/archives/libvir-list/2013-May/msg00173.html v2: https://www.redhat.com/archives/libvir-list/2013-May/msg00184.html uses virStringSplit instead of strsep added a missing break to virDomainDefParseXML v3: rebased to use VIR_STRDUP fixed a crash on parsing '-cpu qemu32,hv_' Ján Tomko (2): qemu: simplify CPU command line parsing qemu: add hv_vapic and hv_spinlocks support docs/formatdomain.html.in | 17 ++- docs/schemas/domaincommon.rng | 12 ++ src/conf/domain_conf.c | 40 +++++- src/conf/domain_conf.h | 3 + src/qemu/qemu_command.c | 161 ++++++++++++++---------- tests/qemuxml2argvdata/qemuxml2argv-hyperv.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml | 2 + 7 files changed, 170 insertions(+), 67 deletions(-) -- 1.8.1.5
Use virStringSplit. Change the 'error' label to 'cleanup' to prevent memory leaks on error. --- src/qemu/qemu_command.c | 117 +++++++++++++++++++++--------------------------- 1 file changed, 52 insertions(+), 65 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c4a162a..5513e28 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9622,73 +9622,68 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, const char *val) { virCPUDefPtr cpu = NULL; - const char *p = val; - const char *next; + char **tokens; char *model = NULL; + int ret = -1; + int i; - do { - if (*p == '\0' || *p == ',') - goto syntax; + if (!(tokens = virStringSplit(val, ",", 0))) + goto cleanup; - if ((next = strchr(p, ','))) - next++; + if (tokens[0] == NULL) + goto syntax; - if (p == val) { - if (VIR_STRNDUP(model, p, next ? next - p - 1 : -1) < 0) - goto error; + for (i = 0; tokens[i] != NULL; i++) { + if (*tokens[i] == '\0') + goto syntax; + + if (i == 0) { + if (VIR_STRDUP(model, tokens[i]) < 0) + goto cleanup; if (!STREQ(model, "qemu32") && !STREQ(model, "qemu64")) { if (!(cpu = qemuInitGuestCPU(dom))) - goto error; + goto cleanup; cpu->model = model; model = NULL; } - } else if (*p == '+' || *p == '-') { - char *feature; + } else if (*tokens[i] == '+' || *tokens[i] == '-') { + const char *feature = tokens[i] + 1; /* '+' or '-' */ int policy; - int ret = 0; - if (*p == '+') + if (*tokens[i] == '+') policy = VIR_CPU_FEATURE_REQUIRE; else policy = VIR_CPU_FEATURE_DISABLE; - p++; - if (*p == '\0' || *p == ',') + if (*feature == '\0') goto syntax; - if (VIR_STRNDUP(feature, p, next ? next - p - 1 : -1) < 0) - goto error; - if (STREQ(feature, "kvmclock")) { bool present = (policy == VIR_CPU_FEATURE_REQUIRE); - int i; + int j; - for (i = 0; i < dom->clock.ntimers; i++) { - if (dom->clock.timers[i]->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK) { + for (j = 0; j < dom->clock.ntimers; j++) { + if (dom->clock.timers[j]->name == VIR_DOMAIN_TIMER_NAME_KVMCLOCK) break; - } } - if (i == dom->clock.ntimers) { - if (VIR_REALLOC_N(dom->clock.timers, i+1) < 0 || - VIR_ALLOC(dom->clock.timers[i]) < 0) + if (j == dom->clock.ntimers) { + if (VIR_REALLOC_N(dom->clock.timers, j + 1) < 0 || + VIR_ALLOC(dom->clock.timers[j]) < 0) goto no_memory; - dom->clock.timers[i]->name = VIR_DOMAIN_TIMER_NAME_KVMCLOCK; - dom->clock.timers[i]->present = -1; - dom->clock.timers[i]->tickpolicy = -1; - dom->clock.timers[i]->track = -1; + dom->clock.timers[j]->name = VIR_DOMAIN_TIMER_NAME_KVMCLOCK; + dom->clock.timers[j]->present = present; + dom->clock.timers[j]->tickpolicy = -1; + dom->clock.timers[j]->track = -1; dom->clock.ntimers++; - } - - if (dom->clock.timers[i]->present != -1 && - dom->clock.timers[i]->present != present) { + } else if (dom->clock.timers[j]->present != -1 && + dom->clock.timers[j]->present != present) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("conflicting occurrences of kvmclock feature")); - goto error; + goto cleanup; } - dom->clock.timers[i]->present = present; } else if (STREQ(feature, "kvm_pv_eoi")) { if (policy == VIR_CPU_FEATURE_REQUIRE) dom->apic_eoi = VIR_DOMAIN_FEATURE_STATE_ON; @@ -9697,36 +9692,29 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, } else { if (!cpu) { if (!(cpu = qemuInitGuestCPU(dom))) - goto error; + goto cleanup; cpu->model = model; model = NULL; } - ret = virCPUDefAddFeature(cpu, feature, policy); + if (virCPUDefAddFeature(cpu, feature, policy) < 0) + goto cleanup; } - - VIR_FREE(feature); - if (ret < 0) - goto error; - } else if (STRPREFIX(p, "hv_")) { - char *feature; + } else if (STRPREFIX(tokens[i], "hv_")) { + const char *feature = tokens[i] + 3; /* "hv_" */ int f; - p += 3; /* "hv_" */ - if (*p == '\0' || *p == ',') + if (*feature == '\0') goto syntax; - if (VIR_STRNDUP(feature, p, next ? next - p - 1 : -1) < 0) - goto error; - dom->features |= (1 << VIR_DOMAIN_FEATURE_HYPERV); if ((f = virDomainHypervTypeFromString(feature)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unsupported HyperV Enlightenment feature " "'%s'"), feature); - goto error; + goto cleanup; } switch ((enum virDomainHyperv) f) { @@ -9737,21 +9725,17 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, case VIR_DOMAIN_HYPERV_LAST: break; } - - VIR_FREE(feature); } - } while ((p = next)); + } if (dom->os.arch == VIR_ARCH_X86_64) { bool is_32bit = false; if (cpu) { union cpuData *cpuData = NULL; - int ret; - ret = cpuEncode(VIR_ARCH_X86_64, cpu, NULL, &cpuData, - NULL, NULL, NULL, NULL); - if (ret < 0) - goto error; + if (cpuEncode(VIR_ARCH_X86_64, cpu, NULL, &cpuData, + NULL, NULL, NULL, NULL) < 0) + goto cleanup; is_32bit = (cpuHasFeature(VIR_ARCH_X86_64, cpuData, "lm") != 1); cpuDataFree(VIR_ARCH_X86_64, cpuData); @@ -9759,22 +9743,25 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, is_32bit = STREQ(model, "qemu32"); } - if (is_32bit) { + if (is_32bit) dom->os.arch = VIR_ARCH_I686; - } } + + ret = 0; + +cleanup: VIR_FREE(model); - return 0; + virStringFreeList(tokens); + return ret; syntax: virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown CPU syntax '%s'"), val); - goto error; + goto cleanup; no_memory: virReportOOMError(); -error: - return -1; + goto cleanup; } -- 1.8.1.5
On 06/05/13 17:50, Ján Tomko wrote:
Use virStringSplit. Change the 'error' label to 'cleanup' to prevent memory leaks on error. --- src/qemu/qemu_command.c | 117 +++++++++++++++++++++--------------------------- 1 file changed, 52 insertions(+), 65 deletions(-)
ACK. Peter
On 06/06/2013 11:58 AM, Peter Krempa wrote:
On 06/05/13 17:50, Ján Tomko wrote:
Use virStringSplit. Change the 'error' label to 'cleanup' to prevent memory leaks on error. --- src/qemu/qemu_command.c | 117 +++++++++++++++++++++--------------------------- 1 file changed, 52 insertions(+), 65 deletions(-)
ACK.
Thanks, pushed now. Jan
Add new CPU flags for HyperV: hv_vapic for virtual APIC support hv_spinlocks for spinlock support XML: <features> <hyperv> <vapic state='on'/> <spinlocks>0xFFFF</spinlocks> </hyperv> </features> results in the following QEMU command line: qemu -cpu <cpu_model>,hv_vapic,hv_spinlocks=0xffff https://bugzilla.redhat.com/show_bug.cgi?id=784836 --- docs/formatdomain.html.in | 17 ++++++++- docs/schemas/domaincommon.rng | 12 +++++++ src/conf/domain_conf.c | 40 ++++++++++++++++++++- src/conf/domain_conf.h | 3 ++ src/qemu/qemu_command.c | 48 ++++++++++++++++++++++++- tests/qemuxml2argvdata/qemuxml2argv-hyperv.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml | 2 ++ 7 files changed, 120 insertions(+), 4 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 755d084..983e070 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1147,6 +1147,8 @@ <privnet/> <hyperv> <relaxed state='on'/> + <vapic state='on'/> + <spinlocks>0xFFFF</spinlocks> </hyperv> </features> @@ -1197,14 +1199,27 @@ <th>Feature</th> <th>Description</th> <th>Value</th> + <th>Since</th> </tr> <tr> <td>relaxed</td> <td>Relax contstraints on timers</td> <td> on, off</td> + <td><span class="since">1.0.0 (QEMU only)</span></td> + </tr> + <tr> + <td>vapic</td> + <td>Enable virtual APIC</td> + <td>on, off</td> + <td><span class="since">1.0.6 (QEMU only)</span></td> + </tr> + <tr> + <td>spinlocks</td> + <td>Enable spinlock support</td> + <td>hexadecimal number of retries, at least 0xFFF</td> + <td><span class="since">1.0.6 (QEMU only)</span></td> </tr> </table> - <span class="since">Since 1.0.0 (QEMU only)</span> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3cace35..ff14f34 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4017,6 +4017,18 @@ <ref name="hypervtristate"/> </element> </optional> + <optional> + <element name="vapic"> + <ref name="hypervtristate"/> + </element> + </optional> + <optional> + <element name="spinlocks"> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{0,8}</param> + </data> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a16ebd1..3d231a5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -148,7 +148,9 @@ VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, "off") VIR_ENUM_IMPL(virDomainHyperv, VIR_DOMAIN_HYPERV_LAST, - "relaxed") + "relaxed", + "vapic", + "spinlocks") VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, "destroy", @@ -11061,6 +11063,7 @@ virDomainDefParseXML(xmlDocPtr xml, switch ((enum virDomainHyperv) feature) { case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: if (!(tmp = virXPathString("string(./@state)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -11081,6 +11084,32 @@ virDomainDefParseXML(xmlDocPtr xml, def->hyperv_features[feature] = value; break; + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (!(tmp = virXPathString("string(.)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing HyperV spinlock retry count")); + goto error; + } + + if (virStrToLong_ui(tmp, NULL, 0, + &def->hyperv_spinlocks) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Cannot parse HyperV spinlock retry " + "count")); + goto error; + } + + if (def->hyperv_spinlocks < 0xFFF) { + virReportError(VIR_ERR_XML_ERROR, + _("HyperV spinlock retry count must be " + "at least 0xFFF")); + goto error; + } + VIR_FREE(tmp); + def->hyperv_features[VIR_DOMAIN_HYPERV_SPINLOCKS] = + VIR_DOMAIN_FEATURE_STATE_ON; + break; + case VIR_DOMAIN_HYPERV_LAST: break; } @@ -16175,12 +16204,21 @@ virDomainDefFormatInternal(virDomainDefPtr def, 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])); break; + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (def->hyperv_features[i]) + virBufferAsprintf(buf, " <%s>0x%x</%s>\n", + virDomainHypervTypeToString(i), + def->hyperv_spinlocks, + virDomainHypervTypeToString(i)); + break; + case VIR_DOMAIN_HYPERV_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a71d6c..cce1189 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1583,6 +1583,8 @@ enum virDomainFeatureState { enum virDomainHyperv { VIR_DOMAIN_HYPERV_RELAXED = 0, + VIR_DOMAIN_HYPERV_VAPIC, + VIR_DOMAIN_HYPERV_SPINLOCKS, VIR_DOMAIN_HYPERV_LAST }; @@ -1920,6 +1922,7 @@ struct _virDomainDef { int apic_eoi; /* These options are of type virDomainFeatureState */ int hyperv_features[VIR_DOMAIN_HYPERV_LAST]; + unsigned int hyperv_spinlocks; virDomainClockDef clock; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5513e28..b0119d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5782,11 +5782,18 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, 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] == VIR_DOMAIN_FEATURE_STATE_ON) virBufferAsprintf(&buf, ",hv_%s", virDomainHypervTypeToString(i)); break; + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (def->hyperv_features[i] == VIR_DOMAIN_FEATURE_STATE_ON) + virBufferAsprintf(&buf,",hv_spinlocks=0x%x", + def->hyperv_spinlocks); + break; + case VIR_DOMAIN_HYPERV_LAST: break; } @@ -9623,6 +9630,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, { virCPUDefPtr cpu = NULL; char **tokens; + char **hv_tokens = NULL; char *model = NULL; int ret = -1; int i; @@ -9702,9 +9710,19 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, goto cleanup; } } else if (STRPREFIX(tokens[i], "hv_")) { - const char *feature = tokens[i] + 3; /* "hv_" */ + const char *token = tokens[i] + 3; /* "hv_" */ + const char *feature, *value; int f; + if (*token == '\0') + goto syntax; + + if (!(hv_tokens = virStringSplit(token, "=", 2))) + goto cleanup; + + feature = hv_tokens[0]; + value = hv_tokens[1]; + if (*feature == '\0') goto syntax; @@ -9719,12 +9737,39 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, switch ((enum virDomainHyperv) f) { case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: + if (value) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("HyperV feature '%s' should not " + "have a value"), feature); + goto cleanup; + } dom->hyperv_features[f] = VIR_DOMAIN_FEATURE_STATE_ON; break; + case VIR_DOMAIN_HYPERV_SPINLOCKS: + dom->hyperv_features[f] = VIR_DOMAIN_FEATURE_STATE_ON; + if (!hv_tokens[1]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("missing HyperV spinlock retry count")); + goto cleanup; + } + + if (virStrToLong_ui(value, NULL, 0, &dom->hyperv_spinlocks) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("cannot parse HyperV spinlock retry count")); + goto cleanup; + } + + if (dom->hyperv_spinlocks < 0xFFF) + dom->hyperv_spinlocks = 0xFFF; + break; + case VIR_DOMAIN_HYPERV_LAST: break; } + virStringFreeList(hv_tokens); + hv_tokens = NULL; } } @@ -9752,6 +9797,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, cleanup: VIR_FREE(model); virStringFreeList(tokens); + virStringFreeList(hv_tokens); return ret; syntax: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args index fac4d5f..df6b207 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ --cpu qemu32,hv_relaxed -m 214 -smp 6 -nographic -monitor \ +-cpu qemu32,hv_relaxed,hv_vapic,hv_spinlocks=0x2fff -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-hyperv.xml b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml index 0d5d0c7..9601645 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml @@ -12,6 +12,8 @@ <acpi/> <hyperv> <relaxed state='on'/> + <vapic state='on'/> + <spinlocks>0x2fff</spinlocks> </hyperv> </features> <clock offset='utc'/> -- 1.8.1.5
On 06/05/13 17:50, Ján Tomko wrote:
Add new CPU flags for HyperV: hv_vapic for virtual APIC support hv_spinlocks for spinlock support
XML: <features> <hyperv> <vapic state='on'/> <spinlocks>0xFFFF</spinlocks>
Is the Microsoft documentation for this feature use hex values?
</hyperv> </features>
results in the following QEMU command line: qemu -cpu <cpu_model>,hv_vapic,hv_spinlocks=0xffff
https://bugzilla.redhat.com/show_bug.cgi?id=784836 --- docs/formatdomain.html.in | 17 ++++++++- docs/schemas/domaincommon.rng | 12 +++++++ src/conf/domain_conf.c | 40 ++++++++++++++++++++- src/conf/domain_conf.h | 3 ++ src/qemu/qemu_command.c | 48 ++++++++++++++++++++++++- tests/qemuxml2argvdata/qemuxml2argv-hyperv.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml | 2 ++ 7 files changed, 120 insertions(+), 4 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 755d084..983e070 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1147,6 +1147,8 @@ <privnet/> <hyperv> <relaxed state='on'/> + <vapic state='on'/> + <spinlocks>0xFFFF</spinlocks> </hyperv>
</features> @@ -1197,14 +1199,27 @@ <th>Feature</th> <th>Description</th> <th>Value</th> + <th>Since</th> </tr> <tr> <td>relaxed</td> <td>Relax contstraints on timers</td> <td> on, off</td> + <td><span class="since">1.0.0 (QEMU only)</span></td> + </tr> + <tr> + <td>vapic</td> + <td>Enable virtual APIC</td> + <td>on, off</td> + <td><span class="since">1.0.6 (QEMU only)</span></td> + </tr> + <tr> + <td>spinlocks</td> + <td>Enable spinlock support</td> + <td>hexadecimal number of retries, at least 0xFFF</td> + <td><span class="since">1.0.6 (QEMU only)</span></td> </tr> </table> - <span class="since">Since 1.0.0 (QEMU only)</span> </dd> </dl>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3cace35..ff14f34 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4017,6 +4017,18 @@ <ref name="hypervtristate"/> </element> </optional> + <optional> + <element name="vapic"> + <ref name="hypervtristate"/> + </element> + </optional> + <optional> + <element name="spinlocks"> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{0,8}</param> + </data> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a16ebd1..3d231a5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -148,7 +148,9 @@ VIR_ENUM_IMPL(virDomainFeatureState, VIR_DOMAIN_FEATURE_STATE_LAST, "off")
VIR_ENUM_IMPL(virDomainHyperv, VIR_DOMAIN_HYPERV_LAST, - "relaxed") + "relaxed", + "vapic", + "spinlocks")
VIR_ENUM_IMPL(virDomainLifecycle, VIR_DOMAIN_LIFECYCLE_LAST, "destroy", @@ -11061,6 +11063,7 @@ virDomainDefParseXML(xmlDocPtr xml,
switch ((enum virDomainHyperv) feature) { case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: if (!(tmp = virXPathString("string(./@state)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, _("missing 'state' attribute for " @@ -11081,6 +11084,32 @@ virDomainDefParseXML(xmlDocPtr xml, def->hyperv_features[feature] = value; break;
+ case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (!(tmp = virXPathString("string(.)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing HyperV spinlock retry count")); + goto error; + } + + if (virStrToLong_ui(tmp, NULL, 0, + &def->hyperv_spinlocks) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Cannot parse HyperV spinlock retry " + "count"));
Missing formatting literal "%s" (breaks syntax-check)
+ goto error; + } + + if (def->hyperv_spinlocks < 0xFFF) { + virReportError(VIR_ERR_XML_ERROR, + _("HyperV spinlock retry count must be " + "at least 0xFFF"));
Missing formatting literal "%s" (breaks syntax-check)
+ goto error; + } + VIR_FREE(tmp); + def->hyperv_features[VIR_DOMAIN_HYPERV_SPINLOCKS] = + VIR_DOMAIN_FEATURE_STATE_ON; + break; + case VIR_DOMAIN_HYPERV_LAST: break; } @@ -16175,12 +16204,21 @@ virDomainDefFormatInternal(virDomainDefPtr def, 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])); break;
+ case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (def->hyperv_features[i]) + virBufferAsprintf(buf, " <%s>0x%x</%s>\n", + virDomainHypervTypeToString(i),
The spinlocks case statement doesn't seem to be universal. You can avoid a function call by directly using the XML field name instead of doing virDomainHypervTypeToString().
+ def->hyperv_spinlocks, + virDomainHypervTypeToString(i));
Twice.
+ break; + case VIR_DOMAIN_HYPERV_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3a71d6c..cce1189 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1583,6 +1583,8 @@ enum virDomainFeatureState {
enum virDomainHyperv { VIR_DOMAIN_HYPERV_RELAXED = 0, + VIR_DOMAIN_HYPERV_VAPIC, + VIR_DOMAIN_HYPERV_SPINLOCKS,
VIR_DOMAIN_HYPERV_LAST }; @@ -1920,6 +1922,7 @@ struct _virDomainDef { int apic_eoi; /* These options are of type virDomainFeatureState */ int hyperv_features[VIR_DOMAIN_HYPERV_LAST]; + unsigned int hyperv_spinlocks;
virDomainClockDef clock;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5513e28..b0119d8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5782,11 +5782,18 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, 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] == VIR_DOMAIN_FEATURE_STATE_ON) virBufferAsprintf(&buf, ",hv_%s", virDomainHypervTypeToString(i)); break;
+ case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (def->hyperv_features[i] == VIR_DOMAIN_FEATURE_STATE_ON) + virBufferAsprintf(&buf,",hv_spinlocks=0x%x", + def->hyperv_spinlocks); + break; + case VIR_DOMAIN_HYPERV_LAST: break; } @@ -9623,6 +9630,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, { virCPUDefPtr cpu = NULL; char **tokens; + char **hv_tokens = NULL; char *model = NULL; int ret = -1; int i; @@ -9702,9 +9710,19 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, goto cleanup; } } else if (STRPREFIX(tokens[i], "hv_")) { - const char *feature = tokens[i] + 3; /* "hv_" */ + const char *token = tokens[i] + 3; /* "hv_" */ + const char *feature, *value; int f;
+ if (*token == '\0') + goto syntax; + + if (!(hv_tokens = virStringSplit(token, "=", 2))) + goto cleanup; + + feature = hv_tokens[0]; + value = hv_tokens[1]; + if (*feature == '\0') goto syntax;
@@ -9719,12 +9737,39 @@ qemuParseCommandLineCPU(virDomainDefPtr dom,
switch ((enum virDomainHyperv) f) { case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: + if (value) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("HyperV feature '%s' should not " + "have a value"), feature); + goto cleanup; + } dom->hyperv_features[f] = VIR_DOMAIN_FEATURE_STATE_ON; break;
+ case VIR_DOMAIN_HYPERV_SPINLOCKS: + dom->hyperv_features[f] = VIR_DOMAIN_FEATURE_STATE_ON; + if (!hv_tokens[1]) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
Missing formatting literal "%s" (breaks syntax-check)
+ _("missing HyperV spinlock retry count")); + goto cleanup; + } + + if (virStrToLong_ui(value, NULL, 0, &dom->hyperv_spinlocks) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
Missing formatting literal "%s" (breaks syntax-check)
+ _("cannot parse HyperV spinlock retry count")); + goto cleanup; + } + + if (dom->hyperv_spinlocks < 0xFFF) + dom->hyperv_spinlocks = 0xFFF; + break; + case VIR_DOMAIN_HYPERV_LAST: break; } + virStringFreeList(hv_tokens); + hv_tokens = NULL; } }
@@ -9752,6 +9797,7 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, cleanup: VIR_FREE(model); virStringFreeList(tokens); + virStringFreeList(hv_tokens); return ret;
syntax: diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args index fac4d5f..df6b207 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args @@ -1,4 +1,4 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc \ --cpu qemu32,hv_relaxed -m 214 -smp 6 -nographic -monitor \ +-cpu qemu32,hv_relaxed,hv_vapic,hv_spinlocks=0x2fff -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-hyperv.xml b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml index 0d5d0c7..9601645 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml @@ -12,6 +12,8 @@ <acpi/> <hyperv> <relaxed state='on'/> + <vapic state='on'/> + <spinlocks>0x2fff</spinlocks> </hyperv> </features> <clock offset='utc'/>
Peter
On 06/06/2013 11:01 AM, Peter Krempa wrote:
On 06/05/13 17:50, Ján Tomko wrote:
Add new CPU flags for HyperV: hv_vapic for virtual APIC support hv_spinlocks for spinlock support
XML: <features> <hyperv> <vapic state='on'/> <spinlocks>0xFFFF</spinlocks>
Is the Microsoft documentation for this feature use hex values?
It uses 0xFFFFFFFF as the special value meaning "no retries" [1] QEMU uses hex values too, but it parses the number with strtoul and base 0 (just as we do), meaning decimal and octal values are allowed too.
</hyperv> </features>
results in the following QEMU command line: qemu -cpu <cpu_model>,hv_vapic,hv_spinlocks=0xffff
Jan [1] http://www.microsoft.com/en-us/download/details.aspx?displaylang=en&id=18673
participants (2)
-
Ján Tomko -
Peter Krempa