On 05/21/2018 07:27 PM, John Ferlan wrote:
On 05/15/2018 08:26 PM, Stefan Berger wrote:
Add the external swtpm to the emulator cgroup so that upper limits of CPU
usage can be enforced on the emulated TPM.
To enable this we need to have the swtpm write its process id (pid) into a
file. We then read it from the file to configure the emulator cgroup.
The PID file is created in /var/run/libvirt/qemu/swtpm:
[root@localhost swtpm]# ls -lZ /var/run/libvirt/qemu/swtpm/
total 4
-rw-r--r--. 1 tss tss system_u:object_r:qemu_var_run_t:s0 5 Apr 10 12:26 1-testvm-swtpm.pid
srw-rw----. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632 0 Apr 10 12:26 1-testvm-swtpm.sock
The swtpm command line now looks as follows:
root@localhost testvm]# ps auxZ | grep swtpm | grep socket | grep -v grep
system_u:system_r:virtd_t:s0:c597,c632 tss 18697 0.0 0.0 28172 3892 ? Ss 16:46 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/swtpm/485d0004-a48f-436a-8457-8a3b73e28568/tpm1.2/ --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --pid file=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.pid
Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
---
src/qemu/qemu_cgroup.c | 35 ++++++++++++
src/qemu/qemu_cgroup.h | 2 +
src/qemu/qemu_extdevice.c | 23 ++++++++
src/qemu/qemu_extdevice.h | 6 +++
src/qemu/qemu_process.c | 4 ++
src/qemu/qemu_tpm.c | 135 ++++++++++++++++++++++++++++++++++++++++++++--
src/qemu/qemu_tpm.h | 6 +++
7 files changed, 208 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 54b00a5da5..12b3f3bf40 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -26,6 +26,7 @@
#include "qemu_cgroup.h"
#include "qemu_domain.h"
#include "qemu_process.h"
+#include "qemu_extdevice.h"
#include "vircgroup.h"
#include "virlog.h"
#include "viralloc.h"
@@ -1172,6 +1173,40 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup,
}
+int
+qemuSetupCgroupForExtDevices(virDomainObjPtr vm,
+ virQEMUDriverPtr driver)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ virCgroupPtr cgroup_temp = NULL;
+ int ret = -1;
+
+ if (!qemuExtDevicesHasDevice(vm->def) ||
+ priv->cgroup == NULL)
+ return 0; /* Not supported, so claim success */
+
+ /*
+ * If CPU cgroup controller is not initialized here, then we need
+ * neither period nor quota settings. And if CPUSET controller is
+ * not initialized either, then there's nothing to do anyway.
+ */
+ if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) &&
+ !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET))
+ return 0;
+
+ if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0,
+ false, &cgroup_temp) < 0)
+ goto cleanup;
+
+ ret = qemuExtDevicesSetupCgroup(driver, vm->def, cgroup_temp);
+
+ cleanup:
+ virCgroupFree(&cgroup_temp);
+
+ return ret;
+}
+
+
int
qemuSetupGlobalCpuCgroup(virDomainObjPtr vm)
{
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 3b8ff6055d..c2fca7fc1d 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -69,6 +69,8 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
long long quota);
int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask);
int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm);
+int qemuSetupCgroupForExtDevices(virDomainObjPtr vm,
+ virQEMUDriverPtr driver);
int qemuRemoveCgroup(virDomainObjPtr vm);
typedef struct _qemuCgroupEmulatorAllNodesData qemuCgroupEmulatorAllNodesData;
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 790b19be9e..dd664208df 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -30,6 +30,8 @@
#include "virlog.h"
#include "virstring.h"
#include "virtime.h"
+#include "virtpm.h"
+#include "virpidfile.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -152,3 +154,24 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
if (def->tpm)
qemuExtTPMStop(driver, def);
}
+
+
+bool
+qemuExtDevicesHasDevice(virDomainDefPtr def)
+{
+ return def->tpm != NULL;
I think this should be:
if (def->tpm && def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
return true;
return false;
Since we only need this for the emulator process, right?
Correct. It would do too much in case of passthrough.
+}
+
+
+int
+qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
+ virDomainDefPtr def,
+ virCgroupPtr cgroup)
+{
+ int ret = 0;
+
+ if (def->tpm)
+ ret = qemuExtTPMSetupCgroup(driver, def, cgroup);
The def->tpm would seem to be superfluous based on
qemuSetupCgroupForExtDevices calling qemuExtDevicesHasDevice first anyway.
For as long as there's not other device, this is of course true.
+
+ return ret;
+}
[...]
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 508685c455..9f50d9b9b2 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
[...]
if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
@@ -756,6 +823,22 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
goto cleanup;
}
+ /* check that the swtpm has written its pid into the file */
+ timeout = 1000; /* ms */
+ while (timeout > 0) {
+ rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid);
+ if (rc < 0) {
+ timeout -= 50;
+ usleep(50 * 1000);
+ continue;
+ }
+ if (rc == 0 && pid == (pid_t)-1)
+ goto error;
s/ goto/goto/
e.g. there's one extra space.
Fixed.
One more improvement would be to push this into
qemuTPMEmulatorPollPid(cfg->swtpmStateDir, shortName, &pid, 50, 1000);
I'd do that later, though, since this may end up touching virpidfile.c
again as well with a function to Poll.
+ break;
+ }
+ if (timeout <= 0)
+ goto error;
+
ret = 0;
[...]
Things look good to me... Let me know about patch 6 and thoughts here. I
don't mind making the adjustments before pushing.
I do need a patch 12 which alters docs/news.xml to briefly describe the
changes to support TPM/emulator...
Ok. I will post a v6. And then I'll post the AppArmor related patches. I
think auditing also needs some work. It's not being called at all at the
moment.
Tks -
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
Applied.
John
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
