[libvirt] [PATCH 1/2] qemu: support for kvm-hint-dedicated performance hint
Michal Privoznik
mprivozn at redhat.com
Mon Aug 12 09:23:16 UTC 2019
On 8/9/19 5:19 PM, Menno Lageman wrote:
> From: Wim ten Have <wim.ten.have at oracle.com>
>
> QEMU version 2.12.1 introduced a performance feature under commit
> be7773268d98 ("target-i386: add KVM_HINTS_DEDICATED performance hint")
>
> This patch adds a new KVM feature 'hint-dedicated' to set this performance
> hint for KVM guests. The feature is off by default.
>
> To enable this hint and have libvirt add "-cpu host,kvm-hint-dedicated=on"
> to the QEMU command line, the following XML code needs to be added to the
> guest's domain description in conjunction with CPU mode='host-passthrough'.
>
> <features>
> <kvm>
> <hint-dedicated state='on'/>
> </kvm>
> </features>
> ...
> <cpu mode='host-passthrough ... />
>
> Signed-off-by: Wim ten Have <wim.ten.have at oracle.com>
> Signed-off-by: Menno Lageman <menno.lageman at oracle.com>
> ---
> docs/formatdomain.html.in | 7 +++++++
> docs/schemas/domaincommon.rng | 5 +++++
> src/conf/domain_conf.c | 4 ++++
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_command.c | 13 +++++++++++++
> 5 files changed, 30 insertions(+)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 6d084d7c0472..c9b18b57e1fc 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2044,6 +2044,7 @@
> </hyperv>
> <kvm>
> <hidden state='on'/>
> + <hint-dedicated state='on'/>
> </kvm>
> <pvspinlock state='on'/>
> <gic version='2'/>
> @@ -2217,6 +2218,12 @@
> <td>on, off</td>
> <td><span class="since">1.2.8 (QEMU 2.1.0)</span></td>
> </tr>
> + <tr>
> + <td>hint-dedicated</td>
> + <td>Set the "dedicated physical CPU" performance hint ("-cpu kvm-hint-dedicated=on")</td>
I'd rather not document the command line libvirt generates here. Not only it's an internal thing of libvirt, it'd also be a promise we might not keep up (if qemu changes way how this is configured for instance).
But what we should document is what this feature actually is. I've tried to dig out KVM patches and now I have a faint idea, but we can't expect our users to go through patches when deciding whether to turn this on or not.
> + <td>on, off</td>
> + <td><span class="since">5.7.0 (QEMU 2.12.1)</span></td>
> + </tr>
> </table>
> </dd>
> <dt><code>pmu</code></dt>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index a0771da45b5d..08853f9d9e92 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -5965,6 +5965,11 @@
> <ref name="featurestate"/>
> </element>
> </optional>
> + <optional>
> + <element name="hint-dedicated">
> + <ref name="featurestate"/>
> + </element>
> + </optional>
> </interleave>
> </element>
> </define>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 03312afaaff8..3907fcf6e5ca 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -202,6 +202,7 @@ VIR_ENUM_IMPL(virDomainHyperv,
> VIR_ENUM_IMPL(virDomainKVM,
> VIR_DOMAIN_KVM_LAST,
> "hidden",
> + "hint-dedicated",
> );
>
> VIR_ENUM_IMPL(virDomainMsrsUnknown,
> @@ -20412,6 +20413,7 @@ virDomainDefParseXML(xmlDocPtr xml,
>
> switch ((virDomainKVM) feature) {
> case VIR_DOMAIN_KVM_HIDDEN:
> + case VIR_DOMAIN_KVM_DEDICATED:
> if (!(tmp = virXMLPropString(nodes[i], "state"))) {
> virReportError(VIR_ERR_XML_ERROR,
> _("missing 'state' attribute for "
> @@ -22624,6 +22626,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src,
> for (i = 0; i < VIR_DOMAIN_KVM_LAST; i++) {
> switch ((virDomainKVM) i) {
> case VIR_DOMAIN_KVM_HIDDEN:
> + case VIR_DOMAIN_KVM_DEDICATED:
> if (src->kvm_features[i] != dst->kvm_features[i]) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> _("State of KVM feature '%s' differs: "
> @@ -28124,6 +28127,7 @@ virDomainDefFormatFeatures(virBufferPtr buf,
> for (j = 0; j < VIR_DOMAIN_KVM_LAST; j++) {
> switch ((virDomainKVM) j) {
> case VIR_DOMAIN_KVM_HIDDEN:
> + case VIR_DOMAIN_KVM_DEDICATED:
> if (def->kvm_features[j])
> virBufferAsprintf(&childBuf, "<%s state='%s'/>\n",
> virDomainKVMTypeToString(j),
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index bce47443c858..f7423b1b6f89 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1765,6 +1765,7 @@ typedef enum {
>
> typedef enum {
> VIR_DOMAIN_KVM_HIDDEN = 0,
> + VIR_DOMAIN_KVM_DEDICATED,
>
> VIR_DOMAIN_KVM_LAST
> } virDomainKVM;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 71a36ff63a81..ab535af5efb6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -7212,6 +7212,19 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
> virBufferAddLit(&buf, ",kvm=off");
> break;
>
> + case VIR_DOMAIN_KVM_DEDICATED:
> + if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) {
> + if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
> + virBufferAddLit(&buf, ",kvm-hint-dedicated=on");
> + } else {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("kvm-hint-dedicated=on is only applicable when "
> + "<cpu mode=\"host-passthrough\" .../> is in effect"));
> + goto cleanup;
> + }
> + }
> + break;
Ou, I don't think this is the correct place to check for valid domain config. How about, you move the check somewhere to qemuDomainDefValidate() and leave here just the cmd line generator part?
Also, the error message is kind of hairy.
Long story short, this is what I have on mind:
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index ab535af5ef..f096e8f27e 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -7213,16 +7213,8 @@ qemuBuildCpuCommandLine(virCommandPtr cmd,
break;
case VIR_DOMAIN_KVM_DEDICATED:
- if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON) {
- if (def->cpu && def->cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
- virBufferAddLit(&buf, ",kvm-hint-dedicated=on");
- } else {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("kvm-hint-dedicated=on is only applicable when "
- "<cpu mode=\"host-passthrough\" .../> is in effect"));
- goto cleanup;
- }
- }
+ if (def->kvm_features[i] == VIR_TRISTATE_SWITCH_ON)
+ virBufferAddLit(&buf, ",kvm-hint-dedicated=on");
break;
/* coverity[dead_error_begin] */
diff --git i/src/qemu/qemu_domain.c w/src/qemu/qemu_domain.c
index 937b461a8b..e1c31d58a4 100644
--- i/src/qemu/qemu_domain.c
+++ w/src/qemu/qemu_domain.c
@@ -4698,6 +4698,14 @@ qemuDomainDefValidate(const virDomainDef *def,
}
}
+ if (def->kvm_features[VIR_DOMAIN_KVM_DEDICATED] == VIR_TRISTATE_SWITCH_ON &&
+ (!def->cpu || def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("kvm-hint-dedicated=on is only applicable "
+ "for cpu host-passthrough"));
+ goto cleanup;
+ }
+
ret = 0;
cleanup:
Michal
More information about the libvir-list
mailing list