[PATCH v3 0/1] add RAPL feature in libvirt
Hi, Since Fedora 41, RAPL feature in QEMU is available. The tested version with this patch is: qemu-9.1.2-2.fc41. Incorporating this feature into libvirt would significantly streamline the process of testing, adopting, and further developing it, thereby enhancing overall productivity and user experience. Thank you for your consideration. Anthony Anthony Harivel (1): qemu: Add support for RAPL MSRs feature docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 10 ++++++++++ src/qemu/qemu_command.c | 12 ++++++++++++ tests/qemuxmlconfdata/kvm-features-off.xml | 1 + .../kvm-features.x86_64-latest.args | 2 +- tests/qemuxmlconfdata/kvm-features.xml | 1 + 8 files changed, 47 insertions(+), 1 deletion(-) -- 2.48.1
Add the support in libvirt to activate the RAPL feature in QEMU. This feature is activated with -accel kvm,rapl=true,path=/path/sock.sock in QEMU. The feature is activated in libvirt with the following XML configuration: <kvm> [...] <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/> [...] </kvm> See: https://gitlab.com/qemu-project/qemu/-/commit/0418f90809aea5b375c859e744c8e8... Signed-off-by: Anthony Harivel <aharivel@redhat.com> --- docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 10 ++++++++++ src/qemu/qemu_command.c | 12 ++++++++++++ tests/qemuxmlconfdata/kvm-features-off.xml | 1 + .../kvm-features.x86_64-latest.args | 2 +- tests/qemuxmlconfdata/kvm-features.xml | 1 + 8 files changed, 47 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 83aeaa32c296..eceee1531085 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2016,6 +2016,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <poll-control state='on'/> <pv-ipi state='off'/> <dirty-ring state='on' size='4096'/> + <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/> </kvm> <xen> <e820_host state='on'/> @@ -2135,6 +2136,7 @@ are: poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.10.0 (QEMU 4.2)` pv-ipi Paravirtualized send IPIs on, off :since:`7.10.0 (QEMU 3.1)` dirty-ring Enable dirty ring feature on, off; size - must be power of 2, range [1024,65536] :since:`8.0.0 (QEMU 6.1)` + rapl Enable rapl feature on, off; socket - rapl helper socket :since:`10.8.0 (QEMU 9.1)` ============== ============================================================================ ====================================================== ============================ ``xen`` diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 87f87bbe56b2..84239f38598d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -224,6 +224,7 @@ VIR_ENUM_IMPL(virDomainKVM, "poll-control", "pv-ipi", "dirty-ring", + "rapl", ); VIR_ENUM_IMPL(virDomainXen, @@ -16837,6 +16838,11 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return -1; } } + + /* rapl feature should parse socket property */ + if (feature == VIR_DOMAIN_KVM_RAPL && value == VIR_TRISTATE_SWITCH_ON) { + def->kvm_features->rapl_helper_socket = virXMLPropString(feat, "socket"); + } } def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; @@ -21308,6 +21314,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RAPL: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), @@ -28107,6 +28114,17 @@ virDomainDefFormatFeatures(virBuffer *buf, } break; + case VIR_DOMAIN_KVM_RAPL: + if (def->kvm_features->features[j] != VIR_TRISTATE_SWITCH_ABSENT) { + virBufferAsprintf(&childBuf, "<%s state='%s'", + virDomainKVMTypeToString(j), + virTristateSwitchTypeToString(def->kvm_features->features[j])); + + virBufferEscapeString(&childBuf, " socket='%s'", def->kvm_features->rapl_helper_socket); + virBufferAddLit(&childBuf, "/>\n"); + } + break; + case VIR_DOMAIN_KVM_LAST: break; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e51c74b6d18e..fef0f0a09c61 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2241,6 +2241,7 @@ typedef enum { VIR_DOMAIN_KVM_POLLCONTROL, VIR_DOMAIN_KVM_PVIPI, VIR_DOMAIN_KVM_DIRTY_RING, + VIR_DOMAIN_KVM_RAPL, VIR_DOMAIN_KVM_LAST } virDomainKVM; @@ -2440,6 +2441,7 @@ struct _virDomainFeatureKVM { int features[VIR_DOMAIN_KVM_LAST]; unsigned int dirty_ring_size; /* size of dirty ring for each vCPU, no units */ + char *rapl_helper_socket; /* rapl helper socket */ }; typedef struct _virDomainFeatureTCG virDomainFeatureTCG; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 96cedb85e867..c910df8c75a0 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -8143,6 +8143,16 @@ </optional> </element> </optional> + <optional> + <element name="rapl"> + <ref name="featurestate"/> + <optional> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </optional> + </element> + </optional> </interleave> </element> </define> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7370711918b6..e194a2dd2aa5 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6437,6 +6437,9 @@ qemuBuildCpuCommandLine(virCommand *cmd, case VIR_DOMAIN_KVM_DIRTY_RING: break; + case VIR_DOMAIN_KVM_RAPL: + break; + case VIR_DOMAIN_KVM_LAST: break; } @@ -7062,6 +7065,15 @@ qemuBuildAccelCommandLine(virCommand *cmd, def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->kvm_features->dirty_ring_size); } + + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON && + def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&buf, ",rapl=true"); + + if (def->kvm_features->rapl_helper_socket != NULL) { + virBufferAsprintf(&buf, ",rapl-helper-socket=%s", def->kvm_features->rapl_helper_socket); + } + } break; case VIR_DOMAIN_VIRT_HVF: diff --git a/tests/qemuxmlconfdata/kvm-features-off.xml b/tests/qemuxmlconfdata/kvm-features-off.xml index 3cd4ff18f283..3e8dbea4b177 100644 --- a/tests/qemuxmlconfdata/kvm-features-off.xml +++ b/tests/qemuxmlconfdata/kvm-features-off.xml @@ -16,6 +16,7 @@ <poll-control state='off'/> <pv-ipi state='off'/> <dirty-ring state='off'/> + <rapl state='off'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> diff --git a/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args b/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args index 955db67eb4b7..2dd613ab338d 100644 --- a/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args +++ b/tests/qemuxmlconfdata/kvm-features.x86_64-latest.args @@ -11,7 +11,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-QEMUGuest1/.config \ -S \ -object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-QEMUGuest1/master-key.aes"}' \ -machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram,acpi=on \ --accel kvm,dirty-ring-size=4096 \ +-accel kvm,dirty-ring-size=4096,rapl=true,rapl-helper-socket=/run/qemu-vmsr-helper.sock \ -cpu host,migratable=off,kvm=off,kvm-hint-dedicated=on,kvm-poll-control=on \ -m size=219136k \ -object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \ diff --git a/tests/qemuxmlconfdata/kvm-features.xml b/tests/qemuxmlconfdata/kvm-features.xml index 78091064b109..ee601e06de75 100644 --- a/tests/qemuxmlconfdata/kvm-features.xml +++ b/tests/qemuxmlconfdata/kvm-features.xml @@ -16,6 +16,7 @@ <poll-control state='on'/> <pv-ipi state='on'/> <dirty-ring state='on' size='4096'/> + <rapl state='on' socket='/run/qemu-vmsr-helper.sock'/> </kvm> </features> <cpu mode='host-passthrough' check='none' migratable='off'/> -- 2.48.1
On Wed, Feb 12, 2025 at 16:36:48 +0100, Anthony Harivel wrote:
Add the support in libvirt to activate the RAPL feature in QEMU.
This feature is activated with -accel kvm,rapl=true,path=/path/sock.sock in QEMU.
The feature is activated in libvirt with the following XML configuration:
<kvm> [...] <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/> [...] </kvm>
See: https://gitlab.com/qemu-project/qemu/-/commit/0418f90809aea5b375c859e744c8e8...
So what are the plans regarding the managed mode here? I don't necessarily need you to commit to anything but if libvirt is ever expected to support the managed mode we should at least have a strategy how to integrate that and document that only non-managed mode is supported. E.g. prepare for <rapl state ='on' mode='unmanaged' socket='/run/qemu-vmsr-helper.sock'/> Or at least imply in the documentation that if socket is provided we expect an manually managed daemon to listen there.
Signed-off-by: Anthony Harivel <aharivel@redhat.com> --- docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 10 ++++++++++ src/qemu/qemu_command.c | 12 ++++++++++++ tests/qemuxmlconfdata/kvm-features-off.xml | 1 + .../kvm-features.x86_64-latest.args | 2 +- tests/qemuxmlconfdata/kvm-features.xml | 1 + 8 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 83aeaa32c296..eceee1531085 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2016,6 +2016,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <poll-control state='on'/> <pv-ipi state='off'/> <dirty-ring state='on' size='4096'/> + <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/> </kvm> <xen> <e820_host state='on'/> @@ -2135,6 +2136,7 @@ are: poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.10.0 (QEMU 4.2)` pv-ipi Paravirtualized send IPIs on, off :since:`7.10.0 (QEMU 3.1)` dirty-ring Enable dirty ring feature on, off; size - must be power of 2, range [1024,65536] :since:`8.0.0 (QEMU 6.1)` + rapl Enable rapl feature on, off; socket - rapl helper socket :since:`10.8.0 (QEMU 9.1)` ============== ============================================================================ ====================================================== ============================
The libvirt version here will need updating. Since this works only on intel: $ qemu-system-x86_64 --accel kvm,rapl=true qemu-system-x86_64: --accel kvm,rapl=true: The RAPL feature can only be enabled on hosts with Intel CPU models qemu-system-x86_64: --accel kvm,rapl=true: kvm : error RAPL feature requirement not met you should document that. I don't think we need a specific CPU check for this niche feature.
@@ -16837,6 +16838,11 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return -1; } } + + /* rapl feature should parse socket property */ + if (feature == VIR_DOMAIN_KVM_RAPL && value == VIR_TRISTATE_SWITCH_ON) { + def->kvm_features->rapl_helper_socket = virXMLPropString(feat, "socket"); + } }
Qemu will fail without a socket. $ qemu-system-x86_64 --accel kvm,rapl=true qemu-system-x86_64: --accel kvm,rapl=true: vmsr socket opening failed If you make the socket mandatory the presence of the socket could later be used to mean un-managed mode.
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; @@ -21308,6 +21314,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RAPL: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), @@ -28107,6 +28114,17 @@ virDomainDefFormatFeatures(virBuffer *buf,
[...]
typedef struct _virDomainFeatureTCG virDomainFeatureTCG; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 96cedb85e867..c910df8c75a0 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -8143,6 +8143,16 @@ </optional> </element> </optional> + <optional> + <element name="rapl"> + <ref name="featurestate"/> + <optional> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </optional> + </element> + </optional> </interleave> </element> </define>
[...]
@@ -7062,6 +7065,15 @@ qemuBuildAccelCommandLine(virCommand *cmd, def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->kvm_features->dirty_ring_size); } + + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON && + def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&buf, ",rapl=true"); + + if (def->kvm_features->rapl_helper_socket != NULL) { + virBufferAsprintf(&buf, ",rapl-helper-socket=%s", def->kvm_features->rapl_helper_socket); + }
As noted above; qemu makes the socket mandatory so the check doesn't make much sense. Also we'll most likely need to use virQEMUBuildBufferEscapeComma here to properly format paths with a comma since they are user-provided.
+ } break;
case VIR_DOMAIN_VIRT_HVF:
The code doesn't also do any security labelling of the socket. That means that the socket needs to be properly labelled manually. I think that should be documetned as well, so that there's no expectation of libvirt to do this.
Hi Peter, Peter Krempa, Feb 12, 2025 at 17:17:
On Wed, Feb 12, 2025 at 16:36:48 +0100, Anthony Harivel wrote:
Add the support in libvirt to activate the RAPL feature in QEMU.
This feature is activated with -accel kvm,rapl=true,path=/path/sock.sock in QEMU.
The feature is activated in libvirt with the following XML configuration:
<kvm> [...] <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/> [...] </kvm>
See: https://gitlab.com/qemu-project/qemu/-/commit/0418f90809aea5b375c859e744c8e8...
So what are the plans regarding the managed mode here?
I think I will go first, the way Paolo suggested, deliver the helper thought a RPM, i.e unmanaged by libvirt. I did not find any relevant justification for this simple stateless helper to be managed by libvirt. Technically, the helper could also be distributed as a container. But I guess mixing RPM & container for an install is not very appreciated. At the moment it comes with qemu RPM. The systemd files are included as well.
I don't necessarily need you to commit to anything but if libvirt is ever expected to support the managed mode we should at least have a strategy how to integrate that and document that only non-managed mode is supported.
E.g. prepare for
<rapl state ='on' mode='unmanaged' socket='/run/qemu-vmsr-helper.sock'/>
Or at least imply in the documentation that if socket is provided we expect an manually managed daemon to listen there.
Signed-off-by: Anthony Harivel <aharivel@redhat.com> --- docs/formatdomain.rst | 2 ++ src/conf/domain_conf.c | 18 ++++++++++++++++++ src/conf/domain_conf.h | 2 ++ src/conf/schemas/domaincommon.rng | 10 ++++++++++ src/qemu/qemu_command.c | 12 ++++++++++++ tests/qemuxmlconfdata/kvm-features-off.xml | 1 + .../kvm-features.x86_64-latest.args | 2 +- tests/qemuxmlconfdata/kvm-features.xml | 1 + 8 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 83aeaa32c296..eceee1531085 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -2016,6 +2016,7 @@ Hypervisors may allow certain CPU / machine features to be toggled on/off. <poll-control state='on'/> <pv-ipi state='off'/> <dirty-ring state='on' size='4096'/> + <rapl state ='on' socket='/run/qemu-vmsr-helper.sock'/> </kvm> <xen> <e820_host state='on'/> @@ -2135,6 +2136,7 @@ are: poll-control Decrease IO completion latency by introducing a grace period of busy waiting on, off :since:`6.10.0 (QEMU 4.2)` pv-ipi Paravirtualized send IPIs on, off :since:`7.10.0 (QEMU 3.1)` dirty-ring Enable dirty ring feature on, off; size - must be power of 2, range [1024,65536] :since:`8.0.0 (QEMU 6.1)` + rapl Enable rapl feature on, off; socket - rapl helper socket :since:`10.8.0 (QEMU 9.1)` ============== ============================================================================ ====================================================== ============================
The libvirt version here will need updating.
Since this works only on intel:
$ qemu-system-x86_64 --accel kvm,rapl=true qemu-system-x86_64: --accel kvm,rapl=true: The RAPL feature can only be enabled on hosts with Intel CPU models qemu-system-x86_64: --accel kvm,rapl=true: kvm : error RAPL feature requirement not met
you should document that. I don't think we need a specific CPU check for this niche feature.
@@ -16837,6 +16838,11 @@ virDomainFeaturesKVMDefParse(virDomainDef *def, return -1; } } + + /* rapl feature should parse socket property */ + if (feature == VIR_DOMAIN_KVM_RAPL && value == VIR_TRISTATE_SWITCH_ON) { + def->kvm_features->rapl_helper_socket = virXMLPropString(feat, "socket"); + } }
Qemu will fail without a socket.
$ qemu-system-x86_64 --accel kvm,rapl=true qemu-system-x86_64: --accel kvm,rapl=true: vmsr socket opening failed
If you make the socket mandatory the presence of the socket could later be used to mean un-managed mode.
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON; @@ -21308,6 +21314,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDef *src, case VIR_DOMAIN_KVM_POLLCONTROL: case VIR_DOMAIN_KVM_PVIPI: case VIR_DOMAIN_KVM_DIRTY_RING: + case VIR_DOMAIN_KVM_RAPL: if (src->kvm_features->features[i] != dst->kvm_features->features[i]) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("State of KVM feature '%1$s' differs: source: '%2$s', destination: '%3$s'"), @@ -28107,6 +28114,17 @@ virDomainDefFormatFeatures(virBuffer *buf,
[...]
typedef struct _virDomainFeatureTCG virDomainFeatureTCG; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 96cedb85e867..c910df8c75a0 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -8143,6 +8143,16 @@ </optional> </element> </optional> + <optional> + <element name="rapl"> + <ref name="featurestate"/> + <optional> + <attribute name="socket"> + <ref name="absFilePath"/> + </attribute> + </optional> + </element> + </optional> </interleave> </element> </define>
[...]
@@ -7062,6 +7065,15 @@ qemuBuildAccelCommandLine(virCommand *cmd, def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->kvm_features->dirty_ring_size); } + + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON && + def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&buf, ",rapl=true"); + + if (def->kvm_features->rapl_helper_socket != NULL) { + virBufferAsprintf(&buf, ",rapl-helper-socket=%s", def->kvm_features->rapl_helper_socket); + }
As noted above; qemu makes the socket mandatory so the check doesn't make much sense.
Also we'll most likely need to use virQEMUBuildBufferEscapeComma here to properly format paths with a comma since they are user-provided.
+ } break;
case VIR_DOMAIN_VIRT_HVF:
The code doesn't also do any security labelling of the socket. That means that the socket needs to be properly labelled manually. I think that should be documetned as well, so that there's no expectation of libvirt to do this.
Thanks for all your feedback. I've added all in my todo list for my next sprint. I understand that it needs way more documentation ! Anthony
Hi Peter,
@@ -7062,6 +7065,15 @@ qemuBuildAccelCommandLine(virCommand *cmd, def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->kvm_features->dirty_ring_size); } + + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON && + def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&buf, ",rapl=true"); + + if (def->kvm_features->rapl_helper_socket != NULL) { + virBufferAsprintf(&buf, ",rapl-helper-socket=%s", def->kvm_features->rapl_helper_socket); + }
As noted above; qemu makes the socket mandatory so the check doesn't make much sense.
I'm not sure to follow the above remark. The socket is mandatory in QEMU yes, so it should also be mandatory in libvirt so that we don't make the QEMU process fails at start ? Or do we just let the user check what QEMU is returning so that the user correct later the XML ? Thanks Anthony
Also we'll most likely need to use virQEMUBuildBufferEscapeComma here to properly format paths with a comma since they are user-provided.
On Thu, Feb 27, 2025 at 12:41:10 +0100, Anthony Harivel wrote:
Hi Peter,
@@ -7062,6 +7065,15 @@ qemuBuildAccelCommandLine(virCommand *cmd, def->kvm_features->features[VIR_DOMAIN_KVM_DIRTY_RING] == VIR_TRISTATE_SWITCH_ON) { virBufferAsprintf(&buf, ",dirty-ring-size=%d", def->kvm_features->dirty_ring_size); } + + if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON && + def->kvm_features->features[VIR_DOMAIN_KVM_RAPL] == VIR_TRISTATE_SWITCH_ON) { + virBufferAddLit(&buf, ",rapl=true"); + + if (def->kvm_features->rapl_helper_socket != NULL) { + virBufferAsprintf(&buf, ",rapl-helper-socket=%s", def->kvm_features->rapl_helper_socket); + }
As noted above; qemu makes the socket mandatory so the check doesn't make much sense.
I'm not sure to follow the above remark. The socket is mandatory in QEMU yes, so it should also be mandatory in libvirt so that we don't make the QEMU process fails at start ?
You should validate that the socket is present; either provided by user or auto-generated once managed mode will be introduced. Also you then don't need to conditionally format it any more once you check that it's present.
Or do we just let the user check what QEMU is returning so that the user correct later the XML ?
Normally we validate config beforehand; the errors from qemu can be iffy sometimes.
Thanks Anthony
Also we'll most likely need to use virQEMUBuildBufferEscapeComma here to
Also don't forget this ^^^
properly format paths with a comma since they are user-provided.
participants (2)
-
Anthony Harivel -
Peter Krempa