[libvirt] [PATCH v3 14/14] qemu: Add swtpm to emulator cgroup

John Ferlan jferlan at redhat.com
Tue May 8 21:44:24 UTC 2018



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 at 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 at 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 at linux.vnet.ibm.com>
> ---
>  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);
>  }
> 




More information about the libvir-list mailing list