11 a.m.
Hi,
First of all, kudos to Peter Krempa for his fast review!
In this v2, I've addressed the following points:
- The socket is *not* mandatory and my code totally confused Peter.
Sorry about that!
here a snippet of the QEMU code to understand:
/* Compute the socket path if necessary */
if (s->msr_energy.socket_path == NULL) {
s->msr_energy.socket_path = vmsr_compute_default_paths();
}
So I made all the modification to make it not necessary.
- Change the socket name to "rapl_helper_socket"
- Change the socket to be absFilePath
- I did not add anything to honour the _OFF state, because it is not
necessary to explicitly disable it.
That's about it.
Regards,
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 | 11 +++++++++++
tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
.../kvm-features.x86_64-latest.args | 2 +-
tests/qemuxmlconfdata/kvm-features.xml | 1 +
8 files changed, 46 insertions(+), 1 deletion(-)
--
2.46.0
11 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
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/0418f90809aea5b375c859e744c...
Signed-off-by: Anthony Harivel <aharivel(a)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 | 11 +++++++++++
tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
.../kvm-features.x86_64-latest.args | 2 +-
tests/qemuxmlconfdata/kvm-features.xml | 1 +
8 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index e1e219bcb5c5..5c66e41e940f 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -1996,6 +1996,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'/>
@@ -2111,6 +2112,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 d950921667a2..32a3d09fe175 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -220,6 +220,7 @@ VIR_ENUM_IMPL(virDomainKVM,
"poll-control",
"pv-ipi",
"dirty-ring",
+ "rapl",
);
VIR_ENUM_IMPL(virDomainXen,
@@ -16693,6 +16694,11 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
return -1;
}
}
+
+ /* rapl feature should parse socket property */
+ if (feature == VIR_DOMAIN_KVM_RAPL && value == VIR_TRISTATE_SWITCH_ON) {
+ kvm->rapl_helper_socket = virXMLPropString(feat, "socket");
+ }
}
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
@@ -21108,6 +21114,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'"),
@@ -27840,6 +27847,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 eae621f900b9..37295f1cfddb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2213,6 +2213,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;
@@ -2400,6 +2401,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 05ba69792470..20d0ce556ecf 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -8016,6 +8016,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 28914c9c346a..b1b0bb9c63a6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6562,6 +6562,9 @@ qemuBuildCpuCommandLine(virCommand *cmd,
case VIR_DOMAIN_KVM_DIRTY_RING:
break;
+ case VIR_DOMAIN_KVM_RAPL:
+ break;
+
case VIR_DOMAIN_KVM_LAST:
break;
}
@@ -7176,6 +7179,14 @@ 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.46.0
8:09 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
On Thu, Aug 22, 2024 at 17:59:47 +0200, 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/0418f90809aea5b375c859e744c...
Signed-off-by: Anthony Harivel <aharivel(a)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 | 11 +++++++++++
tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
.../kvm-features.x86_64-latest.args | 2 +-
tests/qemuxmlconfdata/kvm-features.xml | 1 +
8 files changed, 46 insertions(+), 1 deletion(-)
[...]
@@ -7176,6 +7179,14 @@ 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);
}
This should be separated via a newline.
+ 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:
Apart from that the rest looks good providing the following:
I suppose that the 'rapl-helper-socket' is a shared (multiple qemu's use
it) resource set up beforehand by the admin. Right?
If that is the case that means the lifecycle of the daemon and
permissions (including selinux) for accessing the socket are not
something that libvirt needs to deal with.
If either of them isn't true please outline how that socket is to be
used to see how libvirt will need to approach it.
4:53 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
Peter Krempa, Sep 02, 2024 at 15:09:
On Thu, Aug 22, 2024 at 17:59:47 +0200, 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/0418f90809aea5b375c859e744c...
>
> Signed-off-by: Anthony Harivel <aharivel(a)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 | 11 +++++++++++
> tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
> .../kvm-features.x86_64-latest.args | 2 +-
> tests/qemuxmlconfdata/kvm-features.xml | 1 +
> 8 files changed, 46 insertions(+), 1 deletion(-)
[...]
> @@ -7176,6 +7179,14 @@ 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);
> }
This should be separated via a newline.
Will be done for next iteration thanks.
> + 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:
Apart from that the rest looks good providing the following:
I suppose that the 'rapl-helper-socket' is a shared (multiple qemu's use
it) resource set up beforehand by the admin. Right?
Indeed this is shared between all qemu instances.
If that is the case that means the lifecycle of the daemon and
permissions (including selinux) for accessing the socket are not
something that libvirt needs to deal with.
If either of them isn't true please outline how that socket is to be
used to see how libvirt will need to approach it.
This is a very good question, I'm trying to solve at the moment.
The daemon is called "qemu-vmsr-helper". The source is available in
qemu/tools/i386/* (qemu 9.1) and is built with projet options "tools"
enable.
There is also a systemd files available in qemu source code
qemu/contrib/systemd/
with the file qemu-vmsr-helper.service and qemu-vmsr-helper.socket that
respectively start the daemon and start the socket to listen to.
If libvirt can solve the installation of this daemon, then I'll be happy
to add a patch to my patch queue.
Looking forward you thought about it.
Many thanks,
Anthony
5:08 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
On Thu, Aug 22, 2024 at 17:59:47 +0200, 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/0418f90809aea5b375c859e744c...
>
> Signed-off-by: Anthony Harivel <aharivel(a)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 | 11 +++++++++++
> tests/qemuxmlconfdata/kvm-features-off.xml | 1 +
> .../kvm-features.x86_64-latest.args | 2 +-
> tests/qemuxmlconfdata/kvm-features.xml | 1 +
> 8 files changed, 46 insertions(+), 1 deletion(-)
[...]
> @@ -7176,6 +7179,14 @@ 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);
> }
This should be separated via a newline.
> + 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:
Apart from that the rest looks good providing the following:
I suppose that the 'rapl-helper-socket' is a shared (multiple qemu's use
it) resource set up beforehand by the admin. Right?
The qemu-pr-helper could be run as a single instnce, or it could be
run per-QEMU instance. The latter would give us better security
isolation, for what is a privileged daemon. On the other hand, I
wonder about the CPU overhead of having 100's of copies of the
process running on a host.
If that is the case that means the lifecycle of the daemon and
permissions (including selinux) for accessing the socket are not
something that libvirt needs to deal with.
If either of them isn't true please outline how that socket is to be
used to see how libvirt will need to approach it.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6:29 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
Daniel P. Berrangé, Sep 03, 2024 at 12:08:
On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
> On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel wrote:
> > Add the support in libvirt to activate the RAPL feature in QEMU.
> I suppose that the 'rapl-helper-socket' is a shared (multiple qemu's
use
> it) resource set up beforehand by the admin. Right?
The qemu-pr-helper could be run as a single instnce, or it could be
run per-QEMU instance. The latter would give us better security
isolation, for what is a privileged daemon. On the other hand, I
wonder about the CPU overhead of having 100's of copies of the
process running on a host.
If it runs on a single instance, then the socket needs to be chmod/chown
to something like qemu / libvirt group with access only to root and
group.
Running one helper instance per-QEMU instance would mean that every
instance read 1 MSR / Package every second. The socket is left open
(thanks to Daniel suggestion in QEMU review). The impact would be quite
low I guess on the housekeeping CPU.
When I designed the daemon with Paolo, the first solution was the main
idea but I'm open to any solution that leads to a better adoption of the
feature.
Thanks,
Anthony
> If that is the case that means the lifecycle of the daemon and
> permissions (including selinux) for accessing the socket are not
> something that libvirt needs to deal with.
>
> If either of them isn't true please outline how that socket is to be
> used to see how libvirt will need to approach it.
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:16 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
On Tue, Sep 03, 2024 at 13:29:28 +0200, Anthony Harivel wrote:
Daniel P. Berrangé, Sep 03, 2024 at 12:08:
> On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
> > On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel wrote:
> > > Add the support in libvirt to activate the RAPL feature in QEMU.
> > I suppose that the 'rapl-helper-socket' is a shared (multiple
qemu's use
> > it) resource set up beforehand by the admin. Right?
>
> The qemu-pr-helper could be run as a single instnce, or it could be
> run per-QEMU instance. The latter would give us better security
> isolation, for what is a privileged daemon. On the other hand, I
> wonder about the CPU overhead of having 100's of copies of the
> process running on a host.
So when I was originally skimming trhough the docs I didn't properly
understand that the reason for the helper daemon is that there was a
security issue with accessing the measurement counters and thought it
was strictly for performance reasons.
If it runs on a single instance, then the socket needs to be
chmod/chown
to something like qemu / libvirt group with access only to root and
group.
Another alternative for a shared instance to be used by multiple qemu
instances is that libvirt can open the socket and pass it to qemu, which
avoids the potential issue at-least with DAC security labels as the
socket can be owned by root:root with mode 600.
I'm not sure how the selinux policy for that daemon looks though.
Running one helper instance per-QEMU instance would mean that every
instance read 1 MSR / Package every second. The socket is left open
(thanks to Daniel suggestion in QEMU review). The impact would be quite
low I guess on the housekeeping CPU.
When I designed the daemon with Paolo, the first solution was the main
idea but I'm open to any solution that leads to a better adoption of the
feature.
Libvirt can obviously manage also a per VM instance, which should be
straightforward, but not as simple as this patch. This can theoretically
also be added later, e.g. by adding a 'managed' property enabling the
libvirt-managed daemon.
7:24 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
On Tue, Sep 03, 2024 at 02:16:58PM +0200, Peter Krempa wrote:
On Tue, Sep 03, 2024 at 13:29:28 +0200, Anthony Harivel wrote:
> Daniel P. Berrangé, Sep 03, 2024 at 12:08:
> > On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
> > > On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel wrote:
> > > > Add the support in libvirt to activate the RAPL feature in QEMU.
> > > I suppose that the 'rapl-helper-socket' is a shared (multiple
qemu's use
> > > it) resource set up beforehand by the admin. Right?
> >
> > The qemu-pr-helper could be run as a single instnce, or it could be
> > run per-QEMU instance. The latter would give us better security
> > isolation, for what is a privileged daemon. On the other hand, I
> > wonder about the CPU overhead of having 100's of copies of the
> > process running on a host.
So when I was originally skimming trhough the docs I didn't properly
understand that the reason for the helper daemon is that there was a
security issue with accessing the measurement counters and thought it
was strictly for performance reasons.
Yep, this is only for security reasons.
> If it runs on a single instance, then the socket needs to be
chmod/chown
> to something like qemu / libvirt group with access only to root and
> group.
Another alternative for a shared instance to be used by multiple qemu
instances is that libvirt can open the socket and pass it to qemu, which
avoids the potential issue at-least with DAC security labels as the
socket can be owned by root:root with mode 600.
I'm not sure how the selinux policy for that daemon looks though.
I don't believe any policy exists yet, since this is a brand new
service.
> Running one helper instance per-QEMU instance would mean that
every
> instance read 1 MSR / Package every second. The socket is left open
> (thanks to Daniel suggestion in QEMU review). The impact would be quite
> low I guess on the housekeeping CPU.
>
> When I designed the daemon with Paolo, the first solution was the main
> idea but I'm open to any solution that leads to a better adoption of the
> feature.
Libvirt can obviously manage also a per VM instance, which should be
straightforward, but not as simple as this patch. This can theoretically
also be added later, e.g. by adding a 'managed' property enabling the
libvirt-managed daemon.
A parallel would be the qemu-pr-helper which this new service was initially
derived from in terms of archicture. It also runs privileged for security
reasons and can be single shared instance, or per-VM instance.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
7:41 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
Daniel P. Berrangé, Sep 03, 2024 at 14:24:
On Tue, Sep 03, 2024 at 02:16:58PM +0200, Peter Krempa wrote:
> On Tue, Sep 03, 2024 at 13:29:28 +0200, Anthony Harivel wrote:
> > Daniel P. Berrangé, Sep 03, 2024 at 12:08:
> > > On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
> > > > On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel wrote:
> > > > > Add the support in libvirt to activate the RAPL feature in
QEMU.
> > > > I suppose that the 'rapl-helper-socket' is a shared (multiple
qemu's use
> > > > it) resource set up beforehand by the admin. Right?
> > >
> > > The qemu-pr-helper could be run as a single instnce, or it could be
> > > run per-QEMU instance. The latter would give us better security
> > > isolation, for what is a privileged daemon. On the other hand, I
> > > wonder about the CPU overhead of having 100's of copies of the
> > > process running on a host.
>
> So when I was originally skimming trhough the docs I didn't properly
> understand that the reason for the helper daemon is that there was a
> security issue with accessing the measurement counters and thought it
> was strictly for performance reasons.
Yep, this is only for security reasons.
TL;DR:
"A malicious user application may use RAPL to infer confidential
information of another unprivileged application running on the same or
a different CPU core by observing the power-related behavior of the
victim application."
The answer from Intel was to put the interface being privileged
permission.
Link:
https://www.intel.com/content/www/us/en/developer/articles/technical/soft...
> > If it runs on a single instance, then the socket needs to
be chmod/chown
> > to something like qemu / libvirt group with access only to root and
> > group.
>
> Another alternative for a shared instance to be used by multiple qemu
> instances is that libvirt can open the socket and pass it to qemu, which
> avoids the potential issue at-least with DAC security labels as the
> socket can be owned by root:root with mode 600.
>
> I'm not sure how the selinux policy for that daemon looks though.
I don't believe any policy exists yet, since this is a brand new
service.
> > Running one helper instance per-QEMU instance would mean that every
> > instance read 1 MSR / Package every second. The socket is left open
> > (thanks to Daniel suggestion in QEMU review). The impact would be quite
> > low I guess on the housekeeping CPU.
> >
> > When I designed the daemon with Paolo, the first solution was the main
> > idea but I'm open to any solution that leads to a better adoption of the
> > feature.
>
> Libvirt can obviously manage also a per VM instance, which should be
> straightforward, but not as simple as this patch. This can theoretically
> also be added later, e.g. by adding a 'managed' property enabling the
> libvirt-managed daemon.
A parallel would be the qemu-pr-helper which this new service was initially
derived from in terms of archicture. It also runs privileged for security
reasons and can be single shared instance, or per-VM instance.
Which actually leads to the question to the following:
Why do I "simply" not do like the qemu-pr-helper in libvirt ?
see qemuProcessStartManagedPRDaemon() in src/qemu/qemu_process.c +2808
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
6:01 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
Hi,
Anthony Harivel, Sep 03, 2024 at 14:41:
Daniel P. Berrangé, Sep 03, 2024 at 14:24:
> On Tue, Sep 03, 2024 at 02:16:58PM +0200, Peter Krempa wrote:
> > On Tue, Sep 03, 2024 at 13:29:28 +0200, Anthony Harivel wrote:
> > > Daniel P. Berrangé, Sep 03, 2024 at 12:08:
> > > > On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
> > > > > On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel wrote:
> > > > > > Add the support in libvirt to activate the RAPL feature in
QEMU.
> > > > > I suppose that the 'rapl-helper-socket' is a shared
(multiple qemu's use
> > > > > it) resource set up beforehand by the admin. Right?
> > > >
> > > > The qemu-pr-helper could be run as a single instnce, or it could be
> > > > run per-QEMU instance. The latter would give us better security
> > > > isolation, for what is a privileged daemon. On the other hand, I
> > > > wonder about the CPU overhead of having 100's of copies of the
> > > > process running on a host.
> >
> > So when I was originally skimming trhough the docs I didn't properly
> > understand that the reason for the helper daemon is that there was a
> > security issue with accessing the measurement counters and thought it
> > was strictly for performance reasons.
>
> Yep, this is only for security reasons.
>
TL;DR:
"A malicious user application may use RAPL to infer confidential
information of another unprivileged application running on the same or
a different CPU core by observing the power-related behavior of the
victim application."
The answer from Intel was to put the interface being privileged
permission.
Link:
https://www.intel.com/content/www/us/en/developer/articles/technical/soft...
> > > If it runs on a single instance, then the socket needs to be chmod/chown
> > > to something like qemu / libvirt group with access only to root and
> > > group.
> >
> > Another alternative for a shared instance to be used by multiple qemu
> > instances is that libvirt can open the socket and pass it to qemu, which
> > avoids the potential issue at-least with DAC security labels as the
> > socket can be owned by root:root with mode 600.
> >
> > I'm not sure how the selinux policy for that daemon looks though.
>
> I don't believe any policy exists yet, since this is a brand new
> service.
>
> > > Running one helper instance per-QEMU instance would mean that every
> > > instance read 1 MSR / Package every second. The socket is left open
> > > (thanks to Daniel suggestion in QEMU review). The impact would be quite
> > > low I guess on the housekeeping CPU.
> > >
> > > When I designed the daemon with Paolo, the first solution was the main
> > > idea but I'm open to any solution that leads to a better adoption of
the
> > > feature.
> >
> > Libvirt can obviously manage also a per VM instance, which should be
> > straightforward, but not as simple as this patch. This can theoretically
> > also be added later, e.g. by adding a 'managed' property enabling the
> > libvirt-managed daemon.
>
> A parallel would be the qemu-pr-helper which this new service was initially
> derived from in terms of archicture. It also runs privileged for security
> reasons and can be single shared instance, or per-VM instance.
>
Which actually leads to the question to the following:
Why do I "simply" not do like the qemu-pr-helper in libvirt ?
see qemuProcessStartManagedPRDaemon() in src/qemu/qemu_process.c +2808
> With regards,
> Daniel
> --
> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o- https://fstop138.berrange.com :|
> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
If I may resume the conversation:
1) The helper daemon is primarily needed for security reasons to prevent
potential leaks of confidential information through RAPL.
2) There are two main ways to handle the helper daemon:
Single instance for all QEMU processes:
This requires
adjusting socket permissions (chmod/chown) to control
access securely.
One instance per QEMU process: This offers better isolation but
might increase CPU overhead slightly if the number of instances
running get high.
3) Libvirt can manage both approaches, but no existing SELinux policy
covers this yet, so we’ll need to consider that.
4) qemu-pr-helper is very similar to the qemu-vmsr-helper in terms of
architecture and in terms of privileged needs. It can be single
shared instance or per-vm instance.
I cannot find the original patch that has added qemu-pr-helper into
libvirt. The latest commit in src/qemu/qemu_process is 7eead248c65f
and it doesn't look right IMHO.
If you have a reference to the original patch, I can use it as a guide
to implement this helper in the same way, if you believe that is the
best solution.
Thanks,
Anthony
5:11 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
Hi,
Anthony Harivel, Sep 05, 2024 at 13:01:
Hi,
Anthony Harivel, Sep 03, 2024 at 14:41:
> Daniel P. Berrangé, Sep 03, 2024 at 14:24:
> > On Tue, Sep 03, 2024 at 02:16:58PM +0200, Peter Krempa wrote:
> > > On Tue, Sep 03, 2024 at 13:29:28 +0200, Anthony Harivel wrote:
> > > > Daniel P. Berrangé, Sep 03, 2024 at 12:08:
> > > > > On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
> > > > > > On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel
wrote:
> > > > > > > Add the support in libvirt to activate the RAPL
feature in QEMU.
> > > > > > I suppose that the 'rapl-helper-socket' is a shared
(multiple qemu's use
> > > > > > it) resource set up beforehand by the admin. Right?
> > > > >
> > > > > The qemu-pr-helper could be run as a single instnce, or it could
be
> > > > > run per-QEMU instance. The latter would give us better security
> > > > > isolation, for what is a privileged daemon. On the other hand,
I
> > > > > wonder about the CPU overhead of having 100's of copies of
the
> > > > > process running on a host.
> > >
> > > So when I was originally skimming trhough the docs I didn't properly
> > > understand that the reason for the helper daemon is that there was a
> > > security issue with accessing the measurement counters and thought it
> > > was strictly for performance reasons.
> >
> > Yep, this is only for security reasons.
> >
>
> TL;DR:
> "A malicious user application may use RAPL to infer confidential
> information of another unprivileged application running on the same or
> a different CPU core by observing the power-related behavior of the
> victim application."
>
> The answer from Intel was to put the interface being privileged
> permission.
>
> Link:
>
https://www.intel.com/content/www/us/en/developer/articles/technical/soft...
>
> > > > If it runs on a single instance, then the socket needs to be
chmod/chown
> > > > to something like qemu / libvirt group with access only to root and
> > > > group.
> > >
> > > Another alternative for a shared instance to be used by multiple qemu
> > > instances is that libvirt can open the socket and pass it to qemu, which
> > > avoids the potential issue at-least with DAC security labels as the
> > > socket can be owned by root:root with mode 600.
> > >
> > > I'm not sure how the selinux policy for that daemon looks though.
> >
> > I don't believe any policy exists yet, since this is a brand new
> > service.
> >
> > > > Running one helper instance per-QEMU instance would mean that every
> > > > instance read 1 MSR / Package every second. The socket is left open
> > > > (thanks to Daniel suggestion in QEMU review). The impact would be
quite
> > > > low I guess on the housekeeping CPU.
> > > >
> > > > When I designed the daemon with Paolo, the first solution was the
main
> > > > idea but I'm open to any solution that leads to a better adoption
of the
> > > > feature.
> > >
> > > Libvirt can obviously manage also a per VM instance, which should be
> > > straightforward, but not as simple as this patch. This can theoretically
> > > also be added later, e.g. by adding a 'managed' property enabling
the
> > > libvirt-managed daemon.
> >
> > A parallel would be the qemu-pr-helper which this new service was initially
> > derived from in terms of archicture. It also runs privileged for security
> > reasons and can be single shared instance, or per-VM instance.
> >
>
> Which actually leads to the question to the following:
> Why do I "simply" not do like the qemu-pr-helper in libvirt ?
>
> see qemuProcessStartManagedPRDaemon() in src/qemu/qemu_process.c +2808
>
>
> > With regards,
> > Daniel
> > --
> > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
> > |: https://libvirt.org -o- https://fstop138.berrange.com :|
> > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
If I may resume the conversation:
1) The helper daemon is primarily needed for security reasons to prevent
potential leaks of confidential information through RAPL.
2) There are two main ways to handle the helper daemon:
> Single instance for all QEMU processes:
This requires adjusting socket permissions (chmod/chown) to control
access securely.
> One instance per QEMU process: This offers better isolation but
might increase CPU overhead slightly if the number of instances
running get high.
3) Libvirt can manage both approaches, but no existing SELinux policy
covers this yet, so we’ll need to consider that.
4) qemu-pr-helper is very similar to the qemu-vmsr-helper in terms of
architecture and in terms of privileged needs. It can be single
shared instance or per-vm instance.
I cannot find the original patch that has added qemu-pr-helper into
libvirt. The latest commit in src/qemu/qemu_process is 7eead248c65f
and it doesn't look right IMHO.
If you have a reference to the original patch, I can use it as a guide
to implement this helper in the same way, if you believe that is the
best solution.
Thanks,
Anthony
Gentle ping for the above discussion.
Thanks,
Anthony
10:33 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
On Thu, Sep 05, 2024 at 13:01:53 +0200, Anthony Harivel wrote:
Hi,
Anthony Harivel, Sep 03, 2024 at 14:41:
> Daniel P. Berrangé, Sep 03, 2024 at 14:24:
> > On Tue, Sep 03, 2024 at 02:16:58PM +0200, Peter Krempa wrote:
> > > On Tue, Sep 03, 2024 at 13:29:28 +0200, Anthony Harivel wrote:
> > > > Daniel P. Berrangé, Sep 03, 2024 at 12:08:
> > > > > On Mon, Sep 02, 2024 at 03:09:42PM +0200, Peter Krempa wrote:
> > > > > > On Thu, Aug 22, 2024 at 17:59:47 +0200, Anthony Harivel
wrote:
[...]
If I may resume the conversation:
1) The helper daemon is primarily needed for security reasons to prevent
potential leaks of confidential information through RAPL.
2) There are two main ways to handle the helper daemon:
> Single instance for all QEMU processes:
This requires adjusting socket permissions (chmod/chown) to control
access securely.
> One instance per QEMU process: This offers better isolation but
might increase CPU overhead slightly if the number of instances
running get high.
3) Libvirt can manage both approaches, but no existing SELinux policy
covers this yet, so we’ll need to consider that.
The question is also which of the approaches will actually be used.
Implementing both is possible, it's just whether the managed approach
will be used in the end.
4) qemu-pr-helper is very similar to the qemu-vmsr-helper in terms of
architecture and in terms of privileged needs. It can be single
shared instance or per-vm instance.
I cannot find the original patch that has added qemu-pr-helper into
libvirt. The latest commit in src/qemu/qemu_process is 7eead248c65f
and it doesn't look right IMHO.
If you have a reference to the original patch, I can use it as a guide
to implement this helper in the same way, if you believe that is the
best solution.
The qemu-pr-helper was introduced in libvirt in commits
b0cd8045f012af78e863cd19f74e9db6c1b5dfdd and few prior ones. Note that
it was a very long time ago so the code will likely no longer be state
of the art. Also note that for 'qemu-pr-helper' it's much more
complicated as it needs to be handled also for hotplug of disks when a
new instance might need to be started.
Here you'll only ever have one instance, that shares the lifetime with
the VM, which makes few things much simpler (much less wiring up weird
corner cases).
In case we do in fact want a libvirt-managed version of this the
question is also how to handle potential cases e.g. when the
libvirt-managed daemon gets killed/crashes.
The PR manager daemon has some form of event handlign which will restart
and reconnect it. Is that needed here as well?
5:03 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
Hi,
Peter Krempa, Oct 03, 2024 at 17:33:
> Anthony Harivel, Sep 03, 2024 at 14:41:
[...]
> If I may resume the conversation:
>
> 1) The helper daemon is primarily needed for security reasons to prevent
> potential leaks of confidential information through RAPL.
>
> 2) There are two main ways to handle the helper daemon:
> > Single instance for all QEMU processes:
> This requires adjusting socket permissions (chmod/chown) to control
> access securely.
> > One instance per QEMU process: This offers better isolation but
> might increase CPU overhead slightly if the number of instances
> running get high.
>
> 3) Libvirt can manage both approaches, but no existing SELinux policy
> covers this yet, so we’ll need to consider that.
The question is also which of the approaches will actually be used.
Implementing both is possible, it's just whether the managed approach
will be used in the end.
In the end, the one that will be used is the one that makes things the
more transparent/easier for the user.
If libvirt does not manage the daemon, who is going to do it ?
The vmsr daemon could also be managed by systemd. But again who is
going to setup this ?
My goal is to make this feature the easiest possible to deploy:
- Install qemu (the daemon is installed on the system)
- Install libvirt (the daemon is managed by it)
- add the feature to the VM XML
And it works.
Users need this ideally as simple as this.
> 4) qemu-pr-helper is very similar to the qemu-vmsr-helper in
terms of
> architecture and in terms of privileged needs. It can be single
> shared instance or per-vm instance.
>
>
> I cannot find the original patch that has added qemu-pr-helper into
> libvirt. The latest commit in src/qemu/qemu_process is 7eead248c65f
> and it doesn't look right IMHO.
>
> If you have a reference to the original patch, I can use it as a guide
> to implement this helper in the same way, if you believe that is the
> best solution.
The qemu-pr-helper was introduced in libvirt in commits
b0cd8045f012af78e863cd19f74e9db6c1b5dfdd and few prior ones. Note that
it was a very long time ago so the code will likely no longer be state
of the art. Also note that for 'qemu-pr-helper' it's much more
complicated as it needs to be handled also for hotplug of disks when a
new instance might need to be started.
Here you'll only ever have one instance, that shares the lifetime with
the VM, which makes few things much simpler (much less wiring up weird
corner cases).
In case we do in fact want a libvirt-managed version of this the
question is also how to handle potential cases e.g. when the
libvirt-managed daemon gets killed/crashes.
The PR manager daemon has some form of event handlign which will restart
and reconnect it. Is that needed here as well?
It's a stateless daemon so it should be pretty simple.
- The daemon crash: the MSR value will be 0 during the reboot of the
daemon, the VM does not crash.
- The VM crash: it doesn't matter for the daemon. The VM reboot, request
the MSR, it works.
I found the commit 053d9e30e7a515e7fbddc598e91fc08158fa1329 quite
interesting to continue the dev.
Thanks,
Anthony
8:42 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
On Mon, Oct 07, 2024 at 12:03:17 +0200, Anthony Harivel wrote:
Hi,
Peter Krempa, Oct 03, 2024 at 17:33:
>> Anthony Harivel, Sep 03, 2024 at 14:41:
> [...]
>
>> If I may resume the conversation:
>>
>> 1) The helper daemon is primarily needed for security reasons to prevent
>> potential leaks of confidential information through RAPL.
>>
>> 2) There are two main ways to handle the helper daemon:
>> > Single instance for all QEMU processes:
>> This requires adjusting socket permissions (chmod/chown) to control
>> access securely.
>> > One instance per QEMU process: This offers better isolation but
>> might increase CPU overhead slightly if the number of instances
>> running get high.
>>
>> 3) Libvirt can manage both approaches, but no existing SELinux policy
>> covers this yet, so we’ll need to consider that.
>
> The question is also which of the approaches will actually be used.
> Implementing both is possible, it's just whether the managed approach
> will be used in the end.
>
In the end, the one that will be used is the one that makes things the
more transparent/easier for the user.
If libvirt does not manage the daemon, who is going to do it ?
The vmsr daemon could also be managed by systemd. But again who is
going to setup this ?
My goal is to make this feature the easiest possible to deploy:
- Install qemu (the daemon is installed on the system)
- Install libvirt (the daemon is managed by it)
- add the feature to the VM XML
And it works.
Users need this ideally as simple as this.
Okay that was really not obvious from the initial patch that you wanted
to achieve the above.
As noted it makes sense to have the managed version if it's going to be
used this way, but the initial implementation didn't make it seem as
such.
>> 4) qemu-pr-helper is very similar to the qemu-vmsr-helper in
terms of
>> architecture and in terms of privileged needs. It can be single
>> shared instance or per-vm instance.
>>
>>
>> I cannot find the original patch that has added qemu-pr-helper into
>> libvirt. The latest commit in src/qemu/qemu_process is 7eead248c65f
>> and it doesn't look right IMHO.
>>
>> If you have a reference to the original patch, I can use it as a guide
>> to implement this helper in the same way, if you believe that is the
>> best solution.
>
> The qemu-pr-helper was introduced in libvirt in commits
> b0cd8045f012af78e863cd19f74e9db6c1b5dfdd and few prior ones. Note that
> it was a very long time ago so the code will likely no longer be state
> of the art. Also note that for 'qemu-pr-helper' it's much more
> complicated as it needs to be handled also for hotplug of disks when a
> new instance might need to be started.
>
> Here you'll only ever have one instance, that shares the lifetime with
> the VM, which makes few things much simpler (much less wiring up weird
> corner cases).
>
> In case we do in fact want a libvirt-managed version of this the
> question is also how to handle potential cases e.g. when the
> libvirt-managed daemon gets killed/crashes.
>
> The PR manager daemon has some form of event handlign which will restart
> and reconnect it. Is that needed here as well?
It's a stateless daemon so it should be pretty simple.
- The daemon crash: the MSR value will be 0 during the reboot of the
daemon, the VM does not crash.
- The VM crash: it doesn't matter for the daemon. The VM reboot, request
the MSR, it works.
Fair enough. If the VM keeps running and it just breaks this feature
which looks mostly like statistics it should be fine if it simply stops
working in such cases.
11:13 a.m.
New subject: [PATCH v2 1/1] qemu: Add support for RAPL MSRs feature
Peter Krempa, Oct 07, 2024 at 15:42:
On Mon, Oct 07, 2024 at 12:03:17 +0200, Anthony Harivel wrote:
> Hi,
>
> Peter Krempa, Oct 03, 2024 at 17:33:
> >> Anthony Harivel, Sep 03, 2024 at 14:41:
> > [...]
> >
> >> If I may resume the conversation:
> >>
> >> 1) The helper daemon is primarily needed for security reasons to prevent
> >> potential leaks of confidential information through RAPL.
> >>
> >> 2) There are two main ways to handle the helper daemon:
> >> > Single instance for all QEMU processes:
> >> This requires adjusting socket permissions (chmod/chown) to control
> >> access securely.
> >> > One instance per QEMU process: This offers better isolation but
> >> might increase CPU overhead slightly if the number of instances
> >> running get high.
> >>
> >> 3) Libvirt can manage both approaches, but no existing SELinux policy
> >> covers this yet, so we’ll need to consider that.
> >
> > The question is also which of the approaches will actually be used.
> > Implementing both is possible, it's just whether the managed approach
> > will be used in the end.
> >
>
> In the end, the one that will be used is the one that makes things the
> more transparent/easier for the user.
>
> If libvirt does not manage the daemon, who is going to do it ?
> The vmsr daemon could also be managed by systemd. But again who is
> going to setup this ?
>
> My goal is to make this feature the easiest possible to deploy:
> - Install qemu (the daemon is installed on the system)
> - Install libvirt (the daemon is managed by it)
> - add the feature to the VM XML
>
> And it works.
>
> Users need this ideally as simple as this.
Okay that was really not obvious from the initial patch that you wanted
to achieve the above.
As noted it makes sense to have the managed version if it's going to be
used this way, but the initial implementation didn't make it seem as
such.
And you are absolutely right ! Sorry about the confusion.
My initial patch was only about enabling the feature in the XML file that
creates the right QEMU command line.
Daniel came into the discussion and gave me some thought about the
daemon. Now I've decided that it should be better to have it this way:
one commit to enable the feature in libvirt and another one to manage
the daemon, both coming from the same patch queue.
Bear with me while I'm doing this for the v3.
Thanks,
Anthony
> >> 4) qemu-pr-helper is very similar to the
qemu-vmsr-helper in terms of
> >> architecture and in terms of privileged needs. It can be single
> >> shared instance or per-vm instance.
> >>
> >>
> >> I cannot find the original patch that has added qemu-pr-helper into
> >> libvirt. The latest commit in src/qemu/qemu_process is 7eead248c65f
> >> and it doesn't look right IMHO.
> >>
> >> If you have a reference to the original patch, I can use it as a guide
> >> to implement this helper in the same way, if you believe that is the
> >> best solution.
> >
> > The qemu-pr-helper was introduced in libvirt in commits
> > b0cd8045f012af78e863cd19f74e9db6c1b5dfdd and few prior ones. Note that
> > it was a very long time ago so the code will likely no longer be state
> > of the art. Also note that for 'qemu-pr-helper' it's much more
> > complicated as it needs to be handled also for hotplug of disks when a
> > new instance might need to be started.
> >
> > Here you'll only ever have one instance, that shares the lifetime with
> > the VM, which makes few things much simpler (much less wiring up weird
> > corner cases).
> >
> > In case we do in fact want a libvirt-managed version of this the
> > question is also how to handle potential cases e.g. when the
> > libvirt-managed daemon gets killed/crashes.
> >
> > The PR manager daemon has some form of event handlign which will restart
> > and reconnect it. Is that needed here as well?
>
> It's a stateless daemon so it should be pretty simple.
>
> - The daemon crash: the MSR value will be 0 during the reboot of the
> daemon, the VM does not crash.
>
> - The VM crash: it doesn't matter for the daemon. The VM reboot, request
> the MSR, it works.
Fair enough. If the VM keeps running and it just breaks this feature
which looks mostly like statistics it should be fine if it simply stops
working in such cases.
45
days inactive
91
days old
14 comments
3 participants
participants (3)
-
Anthony Harivel -
Daniel P. Berrangé -
Peter Krempa