1. All hyperv features are tristate ones. So make tristate parsing code common. 2. Reindent switch statement. 3. Reduce nesting in spinlocks parsing. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; --- src/conf/domain_conf.c | 96 +++++++++++++++++++++----------------------------- 1 file changed, 40 insertions(+), 56 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 314a584..1040217 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15584,68 +15584,52 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = nodes[i]; - switch ((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 " - "HyperV Enlightenment feature '%s'"), - nodes[i]->name); - goto error; - } - - if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid value of state argument " - "for HyperV Enlightenment feature '%s'"), - nodes[i]->name); - goto error; - } + if (!(tmp = virXPathString("string(./@state)", ctxt))) { + virReportError(VIR_ERR_XML_ERROR, + _("missing 'state' attribute for " + "HyperV Enlightenment feature '%s'"), + nodes[i]->name); + goto error; + } - VIR_FREE(tmp); - def->hyperv_features[feature] = value; - break; + if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value of state argument " + "for HyperV Enlightenment feature '%s'"), + nodes[i]->name); + goto error; + } - case VIR_DOMAIN_HYPERV_SPINLOCKS: - if (!(tmp = virXPathString("string(./@state)", ctxt))) { - virReportError(VIR_ERR_XML_ERROR, - _("missing 'state' attribute for " - "HyperV Enlightenment feature '%s'"), - nodes[i]->name); - goto error; - } + VIR_FREE(tmp); + def->hyperv_features[feature] = value; - if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid value of state argument " - "for HyperV Enlightenment feature '%s'"), - nodes[i]->name); - goto error; - } + switch ((virDomainHyperv) feature) { + case VIR_DOMAIN_HYPERV_RELAXED: + case VIR_DOMAIN_HYPERV_VAPIC: + break; - VIR_FREE(tmp); - if (value == VIR_TRISTATE_SWITCH_ON) { - if (virXPathUInt("string(./@retries)", ctxt, - &def->hyperv_spinlocks) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("invalid HyperV spinlock retry count")); - goto error; - } - - if (def->hyperv_spinlocks < 0xFFF) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("HyperV spinlock retry count must be " - "at least 4095")); - goto error; - } - } - def->hyperv_features[feature] = value; + case VIR_DOMAIN_HYPERV_SPINLOCKS: + if (value != VIR_TRISTATE_SWITCH_ON) break; - /* coverity[dead_error_begin] */ - case VIR_DOMAIN_HYPERV_LAST: - break; + if (virXPathUInt("string(./@retries)", ctxt, + &def->hyperv_spinlocks) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("invalid HyperV spinlock retry count")); + goto error; + } + + if (def->hyperv_spinlocks < 0xFFF) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("HyperV spinlock retry count must be " + "at least 4095")); + goto error; + } + break; + + /* coverity[dead_error_begin] */ + case VIR_DOMAIN_HYPERV_LAST: + break; } } VIR_FREE(nodes); -- 1.8.3.1

From: Maxim Nestratov <mnestratov(a)virtuozzo.com&gt; This patch adds support for "vpindex", "runtime", "synic" and "stimer" features available in qemu 2.5+. - When Hyper-V "vpindex" is on, guest can use MSR HV_X64_MSR_VP_INDEX to get virtual processor ID. - Hyper-V "runtime" enlightement feature allows to use MSR HV_X64_MSR_VP_RUNTIME to get the time the virtual processor consumes running guest code, as well as the time the hypervisor spends running code on behalf of that guest. - Hyper-V "synic" stands for Synthetic Interrupt Controller, which is lapic extension controlled via MSRs. - Hyper-V "stimer" switches on Hyper-V SynIC timers MSR's support. Guest can setup and use fired by host events (SynIC interrupt and appropriate timer expiration message) as guest clock events - Hyper-V "reset" allows guest to reset VM. - Hyper-V "vendor_id" exposes hypervisor vendor id to guest. Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy(a)virtuozzo.com&gt; --- docs/formatdomain.html.in | 41 +++++++++++++ docs/schemas/domaincommon.rng | 37 ++++++++++++ src/conf/domain_conf.c | 69 +++++++++++++++++++++- src/conf/domain_conf.h | 9 +++ src/qemu/qemu_command.c | 11 ++++ src/qemu/qemu_parse_command.c | 20 +++++++ tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml | 6 ++ tests/qemuxml2argvdata/qemuxml2argv-hyperv.args | 3 +- tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml | 6 ++ .../qemuxml2xmlout-hyperv-off.xml | 6 ++ tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv.xml | 6 ++ 11 files changed, 212 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4e49228..ce8b43a 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1460,6 +1460,11 @@ &lt;relaxed state='on'/&gt; &lt;vapic state='on'/&gt; &lt;spinlocks state='on' retries='4096'/&gt; + &lt;vpindex state='on'/&gt; + &lt;runtime state='on'/&gt; + &lt;synic state='on'/&gt; + &lt;reset state='on'/&gt; + &lt;vendor_id state='on' value='KVM Hv'/&gt; &lt;/hyperv&gt; &lt;kvm&gt; &lt;hidden state='on'/&gt; @@ -1535,6 +1540,42 @@ <td>on, off; retries - at least 4095</td> <td><span class="since">1.1.0 (QEMU only)</span></td> </tr> + <tr> + <td>vpindex</td> + <td>Virtual processor index</td> + <td> on, off</td> + <td><span class="since">1.3.3 (QEMU only)</span></td> + </tr> + <tr> + <td>runtime</td> + <td>Processor time spent on running guest code and on behalf of guest code</td> + <td> on, off</td> + <td><span class="since">1.3.3 (QEMU only)</span></td> + </tr> + <tr> + <td>synic</td> + <td>Enable Synthetic Interrupt Controller (SyNIC)</td> + <td> on, off</td> + <td><span class="since">1.3.3 (QEMU only)</span></td> + </tr> + <tr> + <td>stimer</td> + <td>Enable SyNIC timers</td> + <td> on, off</td> + <td><span class="since">1.3.3 (QEMU only)</span></td> + </tr> + <tr> + <td>reset</td> + <td>Enable hypervisor reset</td> + <td> on, off</td> + <td><span class="since">1.3.3 (QEMU only)</span></td> + </tr> + <tr> + <td>vendor_id</td> + <td>Set hypervisor vendor id</td> + <td>on, off; value - string, up to 12 characters</td> + <td><span class="since">1.3.3 (QEMU only)</span></td> + </tr> </table> </dd> <dt><code>pvspinlock</code></dt> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6ca937c..8861a22 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4885,6 +4885,43 @@ </optional> </element> </optional> + <optional> + <element name="vpindex"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="runtime"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="synic"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="stimer"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="reset"> + <ref name="featurestate"/> + </element> + </optional> + <optional> + <element name="vendor_id"> + <ref name="featurestate"/> + <optional> + <attribute name="value"> + <data type="string"> + <param name='pattern'>[^,]{0,12}</param> + </data> + </attribute> + </optional> + </element> + </optional> </interleave> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6799b86..8ba2e2e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -145,7 +145,13 @@ VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST, VIR_ENUM_IMPL(virDomainHyperv, VIR_DOMAIN_HYPERV_LAST, "relaxed", "vapic", - "spinlocks") + "spinlocks", + "vpindex", + "runtime", + "synic", + "stimer", + "reset", + "vendor_id") VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden") @@ -2594,6 +2600,7 @@ void virDomainDefFree(virDomainDefPtr def) VIR_FREE(def->emulator); VIR_FREE(def->description); VIR_FREE(def->title); + VIR_FREE(def->hyperv_vendor_id); virBlkioDeviceArrayClear(def->blkio.devices, def->blkio.ndevices); @@ -15606,6 +15613,11 @@ virDomainDefParseXML(xmlDocPtr xml, switch ((virDomainHyperv) feature) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: + case VIR_DOMAIN_HYPERV_RESET: break; case VIR_DOMAIN_HYPERV_SPINLOCKS: @@ -15627,6 +15639,33 @@ virDomainDefParseXML(xmlDocPtr xml, } break; + case VIR_DOMAIN_HYPERV_VENDOR_ID: + if (value != VIR_TRISTATE_SWITCH_ON) + break; + + if (!(def->hyperv_vendor_id = virXPathString("string(./@value)", + ctxt))) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing 'value' attribute for " + "HyperV feature 'vendor_id'")); + goto error; + } + + if (strlen(def->hyperv_vendor_id) > VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) { + virReportError(VIR_ERR_XML_ERROR, + _("HyperV vendor_id value must not be more " + "than %d charachters long."), + VIR_DOMAIN_HYPERV_VENDOR_ID_MAX); + goto error; + } + + /* ensure that the string can be passed to qemu*/ + if (strchr(def->hyperv_vendor_id, ',')) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("HyperV vendor_id value is invalid")); + goto error; + } + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; @@ -17629,6 +17668,11 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: + case VIR_DOMAIN_HYPERV_RESET: if (src->hyperv_features[i] != dst->hyperv_features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of HyperV enlightenment " @@ -17654,6 +17698,17 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } break; + case VIR_DOMAIN_HYPERV_VENDOR_ID: + if (STRNEQ_NULLABLE(src->hyperv_vendor_id, dst->hyperv_vendor_id)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("HyperV vendor_id differs: " + "source: '%s', destination: '%s'"), + src->hyperv_vendor_id, + dst->hyperv_vendor_id); + return false; + } + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; @@ -22322,6 +22377,11 @@ virDomainDefFormatInternal(virDomainDefPtr def, switch ((virDomainHyperv) j) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: + case VIR_DOMAIN_HYPERV_RESET: break; case VIR_DOMAIN_HYPERV_SPINLOCKS: @@ -22331,6 +22391,13 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->hyperv_spinlocks); break; + case VIR_DOMAIN_HYPERV_VENDOR_ID: + if (def->hyperv_features[j] != VIR_TRISTATE_SWITCH_ON) + break; + virBufferEscapeString(buf, " value='%s'", + def->hyperv_vendor_id); + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 832ca4f..79ff55e 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1699,10 +1699,18 @@ typedef enum { VIR_DOMAIN_FEATURE_LAST } virDomainFeature; +# define VIR_DOMAIN_HYPERV_VENDOR_ID_MAX 12 + typedef enum { VIR_DOMAIN_HYPERV_RELAXED = 0, VIR_DOMAIN_HYPERV_VAPIC, VIR_DOMAIN_HYPERV_SPINLOCKS, + VIR_DOMAIN_HYPERV_VPINDEX, + VIR_DOMAIN_HYPERV_RUNTIME, + VIR_DOMAIN_HYPERV_SYNIC, + VIR_DOMAIN_HYPERV_STIMER, + VIR_DOMAIN_HYPERV_RESET, + VIR_DOMAIN_HYPERV_VENDOR_ID, VIR_DOMAIN_HYPERV_LAST } virDomainHyperv; @@ -2238,6 +2246,7 @@ struct _virDomainDef { int kvm_features[VIR_DOMAIN_KVM_LAST]; unsigned int hyperv_spinlocks; virGICVersion gic_version; + char *hyperv_vendor_id; /* These options are of type virTristateSwitch: ON = keep, OFF = drop */ int caps_features[VIR_DOMAIN_CAPS_FEATURE_LAST]; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 000c29d..f87511c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5039,6 +5039,11 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, switch ((virDomainHyperv) i) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: + case VIR_DOMAIN_HYPERV_RESET: if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) virBufferAsprintf(&buf, ",hv_%s", virDomainHypervTypeToString(i)); @@ -5050,6 +5055,12 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, def->hyperv_spinlocks); break; + case VIR_DOMAIN_HYPERV_VENDOR_ID: + if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) + virBufferAsprintf(&buf, ",hv_vendor_id=%s", + def->hyperv_vendor_id); + break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 60e3d69..6635190 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -1539,6 +1539,11 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, switch ((virDomainHyperv) f) { case VIR_DOMAIN_HYPERV_RELAXED: case VIR_DOMAIN_HYPERV_VAPIC: + case VIR_DOMAIN_HYPERV_VPINDEX: + case VIR_DOMAIN_HYPERV_RUNTIME: + case VIR_DOMAIN_HYPERV_SYNIC: + case VIR_DOMAIN_HYPERV_STIMER: + case VIR_DOMAIN_HYPERV_RESET: if (value) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("HyperV feature '%s' should not " @@ -1566,6 +1571,21 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, dom->hyperv_spinlocks = 0xFFF; break; + case VIR_DOMAIN_HYPERV_VENDOR_ID: + dom->hyperv_features[f] = VIR_TRISTATE_SWITCH_ON; + if (!value) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("missing HyperV vendor_id value")); + goto cleanup; + } + + if (VIR_STRDUP(dom->hyperv_vendor_id, value) < 0) + goto cleanup; + + if (strlen(dom->hyperv_vendor_id) > VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) + dom->hyperv_vendor_id[VIR_DOMAIN_HYPERV_VENDOR_ID_MAX] = '\0'; + break; + case VIR_DOMAIN_HYPERV_LAST: break; } diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml index 1067f64..fe08463 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv-off.xml @@ -14,6 +14,12 @@ <relaxed state='off'/> <vapic state='off'/> <spinlocks state='off'/> + <vpindex state='off'/> + <runtime state='off'/> + <synic state='off'/> + <stimer state='off'/> + <reset state='off'/> + <vendor_id state='off'/> </hyperv> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args index 141844a..32846a2 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.args @@ -8,7 +8,8 @@ QEMU_AUDIO_DRV=none \ -name QEMUGuest1 \ -S \ -M pc \ --cpu qemu32,hv_relaxed,hv_vapic,hv_spinlocks=0x2fff \ +-cpu 'qemu32,hv_relaxed,hv_vapic,hv_spinlocks=0x2fff,hv_vpindex,hv_runtime,\ +hv_synic,hv_stimer,hv_reset,hv_vendor_id=KVM Hv' \ -m 214 \ -smp 6 \ -uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml index 2b8f332..a47013b 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-hyperv.xml @@ -14,6 +14,12 @@ <relaxed state='on'/> <vapic state='on'/> <spinlocks state='on' retries='12287'/> + <vpindex state='on'/> + <runtime state='on'/> + <synic state='on'/> + <stimer state='on'/> + <reset state='on'/> + <vendor_id state='on' value='KVM Hv'/> </hyperv> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv-off.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv-off.xml index b09c447..6163a5d 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv-off.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv-off.xml @@ -14,6 +14,12 @@ <relaxed state='off'/> <vapic state='off'/> <spinlocks state='off'/> + <vpindex state='off'/> + <runtime state='off'/> + <synic state='off'/> + <stimer state='off'/> + <reset state='off'/> + <vendor_id state='off'/> </hyperv> </features> <clock offset='utc'/> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv.xml index a79115c..c11c273 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hyperv.xml @@ -14,6 +14,12 @@ <relaxed state='on'/> <vapic state='on'/> <spinlocks state='on' retries='12287'/> + <vpindex state='on'/> + <runtime state='on'/> + <synic state='on'/> + <stimer state='on'/> + <reset state='on'/> + <vendor_id state='on' value='KVM Hv'/> </hyperv> </features> <clock offset='utc'/> -- 1.8.3.1

28.03.2016 17:55, John Ferlan пишет: yes, sure let it be the first one (one character shorter :) ) just because it's already in libvirt (commit 59fc0d06) same reason (commit 600bca59) I'm OK with all the comments, push please

[...] OK - on the first pass my eyes didn't see what my fingers typed (1.5, when it should have been 2.5 ;-))... time to punish the fingers. I'll drop the requires too since the column header is "Since"... Hmm. I see... These two were added a bit differently, but the feature bits get added. I was being myopic and just comparing the for loop. Those are bits the user would have no need to provide so better to not document since they're handled by other XML. I'll push this shortly... John [...]