Re: [PATCH v5 11/11] qemu: Add swtpm to emulator cgroup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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?

> +}
> +
> +
> +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.

> +
> +    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.

> +        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...

Tks -

Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>

John

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux