Re: [libvirt] [PATCH 4/6] conf: add support for Direct Mode for Hyper-V Synthetic timers
Ján Tomko writes: > On Thu, Jul 25, 2019 at 03:52:16PM +0200, Vitaly Kuznetsov wrote: >>Support 'Direct Mode' for Hyper-V Synthetic Timers in domain config. >>Make it 'stimer' enlightenment option as it is not a separate thing. >> >>Signed-off-by: Vitaly Kuznetsov >>--- >> docs/formatdomain.html.in | 10 ++- >> docs/schemas/domaincommon.rng | 16 +++- >> src/conf/domain_conf.c| 138 +++--- >> src/conf/domain_conf.h| 8 ++ >> src/cpu/cpu_x86.c | 51 +++-- >> src/cpu/cpu_x86_data.h| 2 + >> src/libvirt_private.syms | 2 + >> 7 files changed, 187 insertions(+), 40 deletions(-) >> >>diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in >>index 1aaddb6d9b..a0723edad1 100644 >>--- a/docs/formatdomain.html.in >>+++ b/docs/formatdomain.html.in >>@@ -2033,7 +2033,9 @@ >>>> >> >>- >>+ >>+ >>>>+ >> >> >>@@ -2148,9 +2150,9 @@ >> >> >> stimer >>- Enable SynIC timers >>- on, off >>- 1.3.3 (QEMU 2.6) >>+ Enable SynIC timers, optionally with Direct Mode support >>+ on, off; direct - on,off >>+ 1.3.3 (QEMU 2.6), direct mode 5.6.0 (QEMU >>4.1) >> >> >> reset >>diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng >>index 763480440c..8cf1995748 100644 >>--- a/docs/schemas/domaincommon.rng >>+++ b/docs/schemas/domaincommon.rng >>@@ -5896,7 +5896,7 @@ >> >> >> >>- >>+ >> >> >> >>@@ -5945,6 +5945,20 @@ >> >> >> >>+ >>+ >>+ >>+ >>+ >>+ >>+ >>+ >>+ >>+ >>+ >>+ >>+ >>+ >> >> >> >>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >>index 0574c69a46..779b4ed880 100644 >>--- a/src/conf/domain_conf.c >>+++ b/src/conf/domain_conf.c >>@@ -197,6 +197,11 @@ VIR_ENUM_IMPL(virDomainHyperv, >> "evmcs", >> ); >> >>+VIR_ENUM_IMPL(virDomainHypervStimer, >>+ VIR_DOMAIN_HYPERV_STIMER_LAST, >>+ "direct", >>+); > > Do you anticipate more stimer "sub"-features in the future? > Having an enum with one value just to loop over an array with one > element and then switch()-ing across all the possible value seems > like overkill. > I don't anticipate any sub-features for stimer for the time being, however, I wanted to make code look like what we already have (e.g. virDomainKVM). We can, of course, simplify things if we consider 'direct' being the one and only. >>+ >> VIR_ENUM_IMPL(virDomainKVM, >> VIR_DOMAIN_KVM_LAST, >> "hidden", >>@@ -20359,6 +20364,51 @@ virDomainDefParseXML(xmlDocPtr xml, >> ctxt->node = node; >> } >> >>+if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { >>+int feature; >>+int value; >>+if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, >>&nodes)) < 0) >>+goto error; >>+ >>+for (i = 0; i < n; i++) { >>+feature = virDomainHypervStimerTypeFromString((const char >>*)nodes[i]->name); >>+if (feature < 0) { >>+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >>+ _("unsupported Hyper-V stimer feature: %s"), >>+ nodes[i]->name); >>+goto error; >>+} >>+ >>+switch ((virDomainHypervStimer) feature) { >>+case VIR_DOMAIN_HYPERV_STIMER_DIRECT: >>+if (!(tmp = virXMLPropString(nodes[i], "state"))) { >>+virReportError(VIR_ERR_XML_ERROR, >>+ _("missing 'state' attribute for " >>+ "Hyper-V stimer feature '%s'"), >>+ nodes[i]->name); >>+goto error; >>+} >>+ >>+if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { >>+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, >>+ _("invalid value of state argument " >>+ "for Hyper-V stimer feature '%s'"), >>+ nodes[i]->name); >>+goto error; >>+} >>+ >>+VIR_FREE(tmp); >>+def->hyperv_stimer_features[feature] = value; >>+break; >>+ >>+/* coverity[dead_error_begin] */ >>+case VIR_DOMAIN_HYPERV_STIMER_LAST: >>+break; >>+} >>+
Re: [libvirt] [PATCH 4/6] conf: add support for Direct Mode for Hyper-V Synthetic timers
On Thu, Jul 25, 2019 at 03:52:16PM +0200, Vitaly Kuznetsov wrote: Support 'Direct Mode' for Hyper-V Synthetic Timers in domain config. Make it 'stimer' enlightenment option as it is not a separate thing. Signed-off-by: Vitaly Kuznetsov --- docs/formatdomain.html.in | 10 ++- docs/schemas/domaincommon.rng | 16 +++- src/conf/domain_conf.c| 138 +++--- src/conf/domain_conf.h| 8 ++ src/cpu/cpu_x86.c | 51 +++-- src/cpu/cpu_x86_data.h| 2 + src/libvirt_private.syms | 2 + 7 files changed, 187 insertions(+), 40 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1aaddb6d9b..a0723edad1 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2033,7 +2033,9 @@- + + + @@ -2148,9 +2150,9 @@ stimer - Enable SynIC timers - on, off - 1.3.3 (QEMU 2.6) + Enable SynIC timers, optionally with Direct Mode support + on, off; direct - on,off + 1.3.3 (QEMU 2.6), direct mode 5.6.0 (QEMU 4.1) reset diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 763480440c..8cf1995748 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5896,7 +5896,7 @@ - + @@ -5945,6 +5945,20 @@ + + + + + + + + + + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0574c69a46..779b4ed880 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -197,6 +197,11 @@ VIR_ENUM_IMPL(virDomainHyperv, "evmcs", ); +VIR_ENUM_IMPL(virDomainHypervStimer, + VIR_DOMAIN_HYPERV_STIMER_LAST, + "direct", +); Do you anticipate more stimer "sub"-features in the future? Having an enum with one value just to loop over an array with one element and then switch()-ing across all the possible value seems like overkill. + VIR_ENUM_IMPL(virDomainKVM, VIR_DOMAIN_KVM_LAST, "hidden", @@ -20359,6 +20364,51 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; } +if (def->features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { +int feature; +int value; +if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes)) < 0) +goto error; + +for (i = 0; i < n; i++) { +feature = virDomainHypervStimerTypeFromString((const char *)nodes[i]->name); +if (feature < 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unsupported Hyper-V stimer feature: %s"), + nodes[i]->name); +goto error; +} + +switch ((virDomainHypervStimer) feature) { +case VIR_DOMAIN_HYPERV_STIMER_DIRECT: +if (!(tmp = virXMLPropString(nodes[i], "state"))) { +virReportError(VIR_ERR_XML_ERROR, + _("missing 'state' attribute for " + "Hyper-V stimer feature '%s'"), + nodes[i]->name); +goto error; +} + +if ((value = virTristateSwitchTypeFromString(tmp)) < 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid value of state argument " + "for Hyper-V stimer feature '%s'"), + nodes[i]->name); +goto error; +} + +VIR_FREE(tmp); +def->hyperv_stimer_features[feature] = value; +break; + +/* coverity[dead_error_begin] */ +case VIR_DOMAIN_HYPERV_STIMER_LAST: +break; +} +} +VIR_FREE(nodes); +} + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) { int feature; int value; @@ -22583,6 +22633,29 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } } +if (src->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] == VIR_TRISTATE_SWITCH_ON) { +for (i = 0; i < VIR_DOMAIN_HYPERV_STIMER_LAST; i++) { +switch ((virDomainHypervStimer) i) { +case VIR_DOMAIN_HYPERV_STIMER_DIRECT: +if (src->hyperv_stimer_features[i] != dst->hyperv_stimer_
