On 05/04/2018 04:21 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/conf/domain_conf.c | 1 +
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_cgroup.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_cgroup.h | 1 +
> src/qemu/qemu_extdevice.c | 19 +++++++++++++++++
> src/qemu/qemu_process.c | 4 ++++
> src/util/virtpm.c | 33 +++++++++++++++++++++++++++++
> 7 files changed, 112 insertions(+)
>
I have to run for the day, I'll look again in the morning at this one...
I did make one note below as I ran the series through Coverity.
John
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index c98d26a..f542c8e 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2624,6 +2624,7 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def)
> VIR_FREE(def->data.emulator.source.data.nix.path);
> VIR_FREE(def->data.emulator.storagepath);
> VIR_FREE(def->data.emulator.logfile);
> + VIR_FREE(def->data.emulator.pidfile);
> break;
> case VIR_DOMAIN_TPM_TYPE_LAST:
> break;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 826ff26..49c77f7 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1311,6 +1311,7 @@ struct _virDomainTPMDef {
> virDomainChrSourceDef source;
> char *storagepath;
> char *logfile;
> + char *pidfile;
> } emulator;
> } data;
> };
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 1a5adca..f86ac3f 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -38,6 +38,7 @@
> #include "virnuma.h"
> #include "virsystemd.h"
> #include "virdevmapper.h"
> +#include "virpidfile.h"
>
> #define VIR_FROM_THIS VIR_FROM_QEMU
>
> @@ -1146,6 +1147,58 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup,
>
>
> int
> +qemuSetupCgroupForExtDevices(virDomainObjPtr vm)
> +{
> + qemuDomainObjPrivatePtr priv = vm->privateData;
> + virDomainTPMDefPtr tpm = vm->def->tpm;
> + virCgroupPtr cgroup_temp = NULL;
> + pid_t pid;
> + int ret = -1;
> +
> + if (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;
> +
> + if (tpm) {
> + switch (tpm->type) {
> + case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> + if (virPidFileReadPath(tpm->data.emulator.pidfile, &pid) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Could not read swtpm's pidfile %s"),
> + tpm->data.emulator.pidfile);
> + goto cleanup;
> + }
> + if (virCgroupAddTask(cgroup_temp, pid) < 0)
> + goto cleanup;
> + break;
> + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> + case VIR_DOMAIN_TPM_TYPE_LAST:
> + break;
> + }
> + }
> +
> + ret = 0;
> +
> +cleanup:
> + virCgroupFree(&cgroup_temp);
> +
> + return ret;
> +}
> +
> +
> +int
> qemuSetupGlobalCpuCgroup(virDomainObjPtr vm)
> {
> qemuDomainObjPrivatePtr priv = vm->privateData;
> diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
> index 3b8ff60..478bf7e 100644
> --- a/src/qemu/qemu_cgroup.h
> +++ b/src/qemu/qemu_cgroup.h
> @@ -69,6 +69,7 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup,
> long long quota);
> int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask);
> int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm);
> +int qemuSetupCgroupForExtDevices(virDomainObjPtr vm);
> int qemuRemoveCgroup(virDomainObjPtr vm);
>
> typedef struct _qemuCgroupEmulatorAllNodesData qemuCgroupEmulatorAllNodesData;
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index eb7220d..779e759 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -148,6 +148,9 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
> virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> virDomainTPMDefPtr tpm = def->tpm;
> char *shortName = virDomainDefGetShortName(def);
> + char *pidfiledata = NULL;
> + int timeout;
> + int len;
>
> if (!shortName)
> return -1;
> @@ -195,6 +198,22 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
> goto error;
> }
>
> + /* check that the swtpm has written its pid into the file */
> + timeout = 1000; /* ms */
> + while ((len = virFileReadHeaderQuiet(tpm->data.emulator.pidfile,
> + 10, &pidfiledata)) <= 0) {
> + if (len == 0 && timeout > 0) {
> + timeout -= 50;
> + usleep(50 * 1000);
> + continue;
> + }
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("swtpm did not write pidfile '%s'"),
> + tpm->data.emulator.pidfile);
> + goto error;
leaks @pidfiledata...
> + }
> + VIR_FREE(pidfiledata);
Just move this to after cleanup: and you fix the leak.
> +
> ret = 0;
>
> cleanup:
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 2b07530..bf0e4da 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6076,6 +6076,10 @@ qemuProcessLaunch(virConnectPtr conn,
> if (qemuProcessSetupEmulator(vm) < 0)
> goto cleanup;
>
> + VIR_DEBUG("Setting cgroup for external devices (if required)");
> + if (qemuSetupCgroupForExtDevices(vm) < 0)
> + goto cleanup;
> +
> VIR_DEBUG("Setting up resctrl");
> if (qemuProcessResctrlCreate(driver, vm) < 0)
> goto cleanup;
> diff --git a/src/util/virtpm.c b/src/util/virtpm.c
> index 0617326..3d8a195 100644
> --- a/src/util/virtpm.c
> +++ b/src/util/virtpm.c
> @@ -39,6 +39,7 @@
> #include "virlog.h"
> #include "virtpm.h"
> #include "virutil.h"
> +#include "virpidfile.h"
> #include "configmake.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
> @@ -376,6 +377,25 @@ int virTPMEmulatorInitPaths(virDomainTPMDefPtr tpm,
> }
>
> /*
> + * virTPMCreatePidFilename
> + */
> +static char *virTPMCreatePidFilename(const char *swtpmStateDir,
> + const char *shortName)
> +{
> + char *pidfile = NULL;
> + char *devname = NULL;
> +
> + if (virAsprintf(&devname, "%s-swtpm", shortName) < 0)
> + return NULL;
> +
> + pidfile = virPidFileBuildPath(swtpmStateDir, devname);
> +
> + VIR_FREE(devname);
> +
> + return pidfile;
> +}
> +
> +/*
> * virTPMEmulatorPrepareHost:
> *
> * @tpm: tpm definition
> @@ -442,6 +462,10 @@ int virTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm,
> goto cleanup;
> tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
>
> + if (!(tpm->data.emulator.pidfile =
> + virTPMCreatePidFilename(swtpmStateDir, shortName)))
> + goto cleanup;
> +
> ret = 0;
>
> cleanup:
> @@ -619,6 +643,9 @@ virTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, const char *vmname,
> break;
> }
>
> + virCommandAddArg(cmd, "--pid");
> + virCommandAddArgFormat(cmd, "file=%s", tpm->data.emulator.pidfile);
> +
> return cmd;
>
> error:
> @@ -646,6 +673,7 @@ virTPMEmulatorStop(const char *swtpmStateDir, const char *shortName)
> virCommandPtr cmd;
> char *pathname;
> char *errbuf = NULL;
> + char *pidfile;
>
> if (virTPMEmulatorInit() < 0)
> return;
> @@ -674,6 +702,11 @@ virTPMEmulatorStop(const char *swtpmStateDir, const char *shortName)
> unlink(pathname);
>
> cleanup:
> + /* clean up the PID file */
> + if ((pidfile = virTPMCreatePidFilename(swtpmStateDir, shortName))) {
> + unlink(pidfile);
> + VIR_FREE(pidfile);
> + }
> VIR_FREE(pathname);
> VIR_FREE(errbuf);
> }
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
