[libvirt] [PATCH v4 11/11] qemu: Add swtpm to emulator cgroup

Stefan Berger posted 11 patches 7 years ago
There is a newer version of this series
[libvirt] [PATCH v4 11/11] qemu: Add swtpm to emulator cgroup
Posted by Stefan Berger 7 years ago
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@linux.vnet.ibm.com>
---
 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       | 134 +++++++++++++++++++++++++++++++++++++++++++++-
 src/qemu/qemu_tpm.h       |   6 +++
 7 files changed, 208 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1a5adca..c51062d 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"
@@ -1146,6 +1147,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)
 {
     qemuDomainObjPrivatePtr priv = vm->privateData;
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 3b8ff60..c2fca7f 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 790b19b..dd66420 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;
+}
+
+
+int
+qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
+                          virDomainDefPtr def,
+                          virCgroupPtr cgroup)
+{
+    int ret = 0;
+
+    if (def->tpm)
+        ret = qemuExtTPMSetupCgroup(driver, def, cgroup);
+
+    return ret;
+}
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
index 6de858b..c557778 100644
--- a/src/qemu/qemu_extdevice.h
+++ b/src/qemu/qemu_extdevice.h
@@ -50,4 +50,10 @@ void qemuExtDevicesStop(virQEMUDriverPtr driver,
                         virDomainDefPtr def)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
+bool qemuExtDevicesHasDevice(virDomainDefPtr def);
+
+int qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
+                              virDomainDefPtr def,
+                              virCgroupPtr cgroup);
+
 #endif /* __QEMU_EXTDEVICE_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 9370de3..35a78f3 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6075,6 +6075,10 @@ qemuProcessLaunch(virConnectPtr conn,
     if (qemuProcessSetupEmulator(vm) < 0)
         goto cleanup;
 
+    VIR_DEBUG("Setting cgroup for external devices (if required)");
+    if (qemuSetupCgroupForExtDevices(vm, driver) < 0)
+        goto cleanup;
+
     VIR_DEBUG("Setting up resctrl");
     if (qemuProcessResctrlCreate(driver, vm) < 0)
         goto cleanup;
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 26cc572..da6acbf 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -39,6 +39,7 @@
 #include "viruuid.h"
 #include "virfile.h"
 #include "virstring.h"
+#include "virpidfile.h"
 #include "configmake.h"
 #include "qemu_tpm.h"
 
@@ -346,6 +347,57 @@ qemuTPMEmulatorInitPaths(virDomainTPMDefPtr tpm,
 
 
 /*
+ * qemuTPMCreatePidFilename
+ */
+static char *
+qemuTPMEmulatorCreatePidFilename(const char *swtpmStateDir,
+                                 const char *shortName)
+{
+    char *pidfile = NULL;
+    char *devicename = NULL;
+
+    if (virAsprintf(&devicename, "%s-swtpm", shortName) < 0)
+        return NULL;
+
+    pidfile = virPidFileBuildPath(swtpmStateDir, devicename);
+
+    VIR_FREE(devicename);
+
+    return pidfile;
+}
+
+
+/*
+ * qemuTPMEmulatorGetPid
+ *
+ * @swtpmStateDir: the directory where swtpm writes the pidfile into
+ * @shortName: short name of the domain
+ * @pid: pointer to pid
+ *
+ * Return -errno upon error, or zero on successful reading of the pidfile.
+ * If the PID was not still alive, zero will be returned, and @pid will be
+ * set to -1;
+ */
+static int
+qemuTPMEmulatorGetPid(const char *swtpmStateDir,
+                      const char *shortName,
+                      pid_t *pid)
+{
+    int ret;
+    char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
+                                                     shortName);
+    if (!pidfile)
+        return -ENOMEM;
+
+    ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm_path);
+
+    VIR_FREE(pidfile);
+
+    return ret;
+}
+
+
+/*
  * qemuTPMEmulatorPrepareHost:
  *
  * @tpm: tpm definition
@@ -514,6 +566,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
  * @privileged: whether we are running in privileged mode
  * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root)
  * @swtpm_group: The gid for the swtpm to run as
+ * @swtpmStateDir: the directory where swtpm writes the pid file and creates the
+ *                 Unix socket
+ * @shortName: the short name of the VM
  *
  * Create the virCommand use for starting the emulator
  * Do some initializations on the way, such as creation of storage
@@ -525,10 +580,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
                             const unsigned char *vmuuid,
                             bool privileged,
                             uid_t swtpm_user,
-                            gid_t swtpm_group)
+                            gid_t swtpm_group,
+                            const char *swtpmStateDir,
+                            const char *shortName)
 {
     virCommandPtr cmd = NULL;
     bool created = false;
+    char *pidfile;
 
     if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath,
                                      &created, swtpm_user, swtpm_group) < 0)
@@ -570,6 +628,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
         break;
     }
 
+    if (!(pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, shortName)))
+        goto error;
+
+    virCommandAddArg(cmd, "--pid");
+    virCommandAddArgFormat(cmd, "file=%s", pidfile);
+    VIR_FREE(pidfile);
+
     return cmd;
 
  error:
@@ -721,6 +786,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
     virQEMUDriverConfigPtr cfg;
     virDomainTPMDefPtr tpm = def->tpm;
     char *shortName = virDomainDefGetShortName(def);
+    int timeout, rc;
+    pid_t pid;
 
     if (!shortName)
         return -1;
@@ -733,7 +800,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
     if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid,
                                             driver->privileged,
                                             cfg->swtpm_user,
-                                            cfg->swtpm_group)))
+                                            cfg->swtpm_group,
+                                            cfg->swtpmStateDir, shortName)))
         goto cleanup;
 
     if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
@@ -769,6 +837,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;
+        break;
+    }
+    if (timeout <= 0)
+        goto error;
+
     ret = 0;
 
  cleanup:
@@ -781,6 +865,11 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
     virObjectUnref(cfg);
 
     return ret;
+
+ error:
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                   _("swtpm failed to start"));
+    goto cleanup;
 }
 
 
@@ -830,3 +919,44 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
     VIR_FREE(shortName);
     virObjectUnref(cfg);
 }
+
+
+int
+qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
+                      virDomainDefPtr def,
+                      virCgroupPtr cgroup)
+{
+    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    char *pidfile = NULL;
+    char *shortName = NULL;
+    int ret = -1, rc;
+    pid_t pid;
+
+    switch (def->tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        shortName = virDomainDefGetShortName(def);
+        if (!shortName)
+            goto cleanup;
+        rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid);
+        if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("Could not get process id of swtpm"));
+            goto cleanup;
+        }
+        if (virCgroupAddTask(cgroup, pid) < 0)
+            goto cleanup;
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        break;
+    }
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(pidfile);
+    VIR_FREE(shortName);
+    virObjectUnref(cfg);
+
+    return ret;
+}
diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
index 20f3a9c..6eb1294 100644
--- a/src/qemu/qemu_tpm.h
+++ b/src/qemu/qemu_tpm.h
@@ -47,4 +47,10 @@ void qemuExtTPMStop(virQEMUDriverPtr driver,
                     virDomainDefPtr def)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
 
+int qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
+                          virDomainDefPtr def,
+                          virCgroupPtr cgroup)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+    ATTRIBUTE_RETURN_CHECK;
+
 #endif /* __QEMU_TPM_H__ */
-- 
2.5.5

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 11/11] qemu: Add swtpm to emulator cgroup
Posted by Stefan Berger 6 years, 12 months ago
On 05/10/2018 05:57 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.

I haven't made any changes to this yet. A possibility would be to put 
swtpm into its own tpm-emulator cgroup and extend the XML for the TPM to 
also have 'period' and 'quota':

     <tpm model='tpm-tis'>
       <backend type='emulator'>
         <period>1000</period>
         <quota>500</quota>
       </backend>
     </tpm>

Or we add the following to cputune:

         <tpm_emulator_period>1000</tpm_emulator_period>
         <tpm_emulator_quota>500</tpm_emulator_quota>

The latter would be more consistent, though i would prefer the former.

    Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 11/11] qemu: Add swtpm to emulator cgroup
Posted by Daniel P. Berrangé 6 years, 12 months ago
On Tue, May 15, 2018 at 11:25:58AM -0400, Stefan Berger wrote:
> On 05/10/2018 05:57 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.
> 
> I haven't made any changes to this yet. A possibility would be to put swtpm
> into its own tpm-emulator cgroup and extend the XML for the TPM to also have
> 'period' and 'quota':
> 
>     <tpm model='tpm-tis'>
>       <backend type='emulator'>
>         <period>1000</period>
>         <quota>500</quota>
>       </backend>
>     </tpm>
> 
> Or we add the following to cputune:
> 
>         <tpm_emulator_period>1000</tpm_emulator_period>
>         <tpm_emulator_quota>500</tpm_emulator_quota>
> 
> The latter would be more consistent, though i would prefer the former.

I'm not really seeing a compelling reason to need to set tunables on
the swtpm directly. IMHO we should just consider it part of the
"emulator" tunables - the fact that it is a separate binary/process
rather than inside QEMU is just a private impl detail.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 11/11] qemu: Add swtpm to emulator cgroup
Posted by Stefan Berger 6 years, 12 months ago
On 05/15/2018 11:34 AM, Daniel P. Berrangé wrote:
> On Tue, May 15, 2018 at 11:25:58AM -0400, Stefan Berger wrote:
>> On 05/10/2018 05:57 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.
>> I haven't made any changes to this yet. A possibility would be to put swtpm
>> into its own tpm-emulator cgroup and extend the XML for the TPM to also have
>> 'period' and 'quota':
>>
>>      <tpm model='tpm-tis'>
>>        <backend type='emulator'>
>>          <period>1000</period>
>>          <quota>500</quota>
>>        </backend>
>>      </tpm>
>>
>> Or we add the following to cputune:
>>
>>          <tpm_emulator_period>1000</tpm_emulator_period>
>>          <tpm_emulator_quota>500</tpm_emulator_quota>
>>
>> The latter would be more consistent, though i would prefer the former.
> I'm not really seeing a compelling reason to need to set tunables on
> the swtpm directly. IMHO we should just consider it part of the
> "emulator" tunables - the fact that it is a separate binary/process
> rather than inside QEMU is just a private impl detail.

One reason I could think of is to approximate the real world a little 
closer where a TPM is typically its own chip.

     Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 11/11] qemu: Add swtpm to emulator cgroup
Posted by Daniel P. Berrangé 6 years, 12 months ago
On Tue, May 15, 2018 at 11:43:10AM -0400, Stefan Berger wrote:
> On 05/15/2018 11:34 AM, Daniel P. Berrangé wrote:
> > On Tue, May 15, 2018 at 11:25:58AM -0400, Stefan Berger wrote:
> > > On 05/10/2018 05:57 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.
> > > I haven't made any changes to this yet. A possibility would be to put swtpm
> > > into its own tpm-emulator cgroup and extend the XML for the TPM to also have
> > > 'period' and 'quota':
> > > 
> > >      <tpm model='tpm-tis'>
> > >        <backend type='emulator'>
> > >          <period>1000</period>
> > >          <quota>500</quota>
> > >        </backend>
> > >      </tpm>
> > > 
> > > Or we add the following to cputune:
> > > 
> > >          <tpm_emulator_period>1000</tpm_emulator_period>
> > >          <tpm_emulator_quota>500</tpm_emulator_quota>
> > > 
> > > The latter would be more consistent, though i would prefer the former.
> > I'm not really seeing a compelling reason to need to set tunables on
> > the swtpm directly. IMHO we should just consider it part of the
> > "emulator" tunables - the fact that it is a separate binary/process
> > rather than inside QEMU is just a private impl detail.
> 
> One reason I could think of is to approximate the real world a little closer
> where a TPM is typically its own chip.

I don't think that's a real use case. You can look at QEMU's machine types
and say they have 10-20 separate chips in the real world, but we don't need
to have control over CPU usage of those chips.

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 11/11] qemu: Add swtpm to emulator cgroup
Posted by Boris Fiuczynski 6 years, 12 months ago
On 05/10/2018 11:57 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@linux.vnet.ibm.com>
> ---
>   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       | 134 +++++++++++++++++++++++++++++++++++++++++++++-
>   src/qemu/qemu_tpm.h       |   6 +++
>   7 files changed, 208 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index 1a5adca..c51062d 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"
> @@ -1146,6 +1147,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:
make syntax-check wants a blank in front of the label

> +    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..c2fca7f 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 790b19b..dd66420 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;
> +}
> +
> +
> +int
> +qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
> +                          virDomainDefPtr def,
> +                          virCgroupPtr cgroup)
> +{
> +    int ret = 0;
> +
> +    if (def->tpm)
> +        ret = qemuExtTPMSetupCgroup(driver, def, cgroup);
> +
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
> index 6de858b..c557778 100644
> --- a/src/qemu/qemu_extdevice.h
> +++ b/src/qemu/qemu_extdevice.h
> @@ -50,4 +50,10 @@ void qemuExtDevicesStop(virQEMUDriverPtr driver,
>                           virDomainDefPtr def)
>       ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>   
> +bool qemuExtDevicesHasDevice(virDomainDefPtr def);
> +
> +int qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
> +                              virDomainDefPtr def,
> +                              virCgroupPtr cgroup);
> +
>   #endif /* __QEMU_EXTDEVICE_H__ */
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 9370de3..35a78f3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6075,6 +6075,10 @@ qemuProcessLaunch(virConnectPtr conn,
>       if (qemuProcessSetupEmulator(vm) < 0)
>           goto cleanup;
>   
> +    VIR_DEBUG("Setting cgroup for external devices (if required)");
> +    if (qemuSetupCgroupForExtDevices(vm, driver) < 0)
> +        goto cleanup;
> +
>       VIR_DEBUG("Setting up resctrl");
>       if (qemuProcessResctrlCreate(driver, vm) < 0)
>           goto cleanup;
> diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
> index 26cc572..da6acbf 100644
> --- a/src/qemu/qemu_tpm.c
> +++ b/src/qemu/qemu_tpm.c
> @@ -39,6 +39,7 @@
>   #include "viruuid.h"
>   #include "virfile.h"
>   #include "virstring.h"
> +#include "virpidfile.h"
>   #include "configmake.h"
>   #include "qemu_tpm.h"
>   
> @@ -346,6 +347,57 @@ qemuTPMEmulatorInitPaths(virDomainTPMDefPtr tpm,
>   
>   
>   /*
> + * qemuTPMCreatePidFilename
> + */
> +static char *
> +qemuTPMEmulatorCreatePidFilename(const char *swtpmStateDir,
> +                                 const char *shortName)
> +{
> +    char *pidfile = NULL;
> +    char *devicename = NULL;
> +
> +    if (virAsprintf(&devicename, "%s-swtpm", shortName) < 0)
> +        return NULL;
> +
> +    pidfile = virPidFileBuildPath(swtpmStateDir, devicename);
> +
> +    VIR_FREE(devicename);
> +
> +    return pidfile;
> +}
> +
> +
> +/*
> + * qemuTPMEmulatorGetPid
> + *
> + * @swtpmStateDir: the directory where swtpm writes the pidfile into
> + * @shortName: short name of the domain
> + * @pid: pointer to pid
> + *
> + * Return -errno upon error, or zero on successful reading of the pidfile.
> + * If the PID was not still alive, zero will be returned, and @pid will be
> + * set to -1;
> + */
> +static int
> +qemuTPMEmulatorGetPid(const char *swtpmStateDir,
> +                      const char *shortName,
> +                      pid_t *pid)
> +{
> +    int ret;
> +    char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
> +                                                     shortName);
> +    if (!pidfile)
> +        return -ENOMEM;
> +
> +    ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm_path);
> +
> +    VIR_FREE(pidfile);
> +
> +    return ret;
> +}
> +
> +
> +/*
>    * qemuTPMEmulatorPrepareHost:
>    *
>    * @tpm: tpm definition
> @@ -514,6 +566,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath,
>    * @privileged: whether we are running in privileged mode
>    * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root)
>    * @swtpm_group: The gid for the swtpm to run as
> + * @swtpmStateDir: the directory where swtpm writes the pid file and creates the
> + *                 Unix socket
> + * @shortName: the short name of the VM
>    *
>    * Create the virCommand use for starting the emulator
>    * Do some initializations on the way, such as creation of storage
> @@ -525,10 +580,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
>                               const unsigned char *vmuuid,
>                               bool privileged,
>                               uid_t swtpm_user,
> -                            gid_t swtpm_group)
> +                            gid_t swtpm_group,
> +                            const char *swtpmStateDir,
> +                            const char *shortName)
>   {
>       virCommandPtr cmd = NULL;
>       bool created = false;
> +    char *pidfile;
>   
>       if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath,
>                                        &created, swtpm_user, swtpm_group) < 0)
> @@ -570,6 +628,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
>           break;
>       }
>   
> +    if (!(pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, shortName)))
> +        goto error;
> +
> +    virCommandAddArg(cmd, "--pid");
> +    virCommandAddArgFormat(cmd, "file=%s", pidfile);
> +    VIR_FREE(pidfile);
> +
>       return cmd;
>   
>    error:
> @@ -721,6 +786,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>       virQEMUDriverConfigPtr cfg;
>       virDomainTPMDefPtr tpm = def->tpm;
>       char *shortName = virDomainDefGetShortName(def);
> +    int timeout, rc;
> +    pid_t pid;
>   
>       if (!shortName)
>           return -1;
> @@ -733,7 +800,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>       if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid,
>                                               driver->privileged,
>                                               cfg->swtpm_user,
> -                                            cfg->swtpm_group)))
> +                                            cfg->swtpm_group,
> +                                            cfg->swtpmStateDir, shortName)))
>           goto cleanup;
>   
>       if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0)
> @@ -769,6 +837,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;
> +        break;
> +    }
> +    if (timeout <= 0)
> +        goto error;
> +
>       ret = 0;
>   
>    cleanup:
> @@ -781,6 +865,11 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>       virObjectUnref(cfg);
>   
>       return ret;
> +
> + error:
> +    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                   _("swtpm failed to start"));
> +    goto cleanup;
>   }
>   
>   
> @@ -830,3 +919,44 @@ qemuExtTPMStop(virQEMUDriverPtr driver,
>       VIR_FREE(shortName);
>       virObjectUnref(cfg);
>   }
> +
> +
> +int
> +qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
> +                      virDomainDefPtr def,
> +                      virCgroupPtr cgroup)
> +{
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> +    char *pidfile = NULL;
> +    char *shortName = NULL;
> +    int ret = -1, rc;
> +    pid_t pid;
> +
> +    switch (def->tpm->type) {
> +    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
> +        shortName = virDomainDefGetShortName(def);
> +        if (!shortName)
> +            goto cleanup;
> +        rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid);
> +        if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Could not get process id of swtpm"));
> +            goto cleanup;
> +        }
> +        if (virCgroupAddTask(cgroup, pid) < 0)
> +            goto cleanup;
> +        break;
> +    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
> +    case VIR_DOMAIN_TPM_TYPE_LAST:
> +        break;
> +    }
> +
> +    ret = 0;
> +
> + cleanup:
> +    VIR_FREE(pidfile);
> +    VIR_FREE(shortName);
> +    virObjectUnref(cfg);
> +
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
> index 20f3a9c..6eb1294 100644
> --- a/src/qemu/qemu_tpm.h
> +++ b/src/qemu/qemu_tpm.h
> @@ -47,4 +47,10 @@ void qemuExtTPMStop(virQEMUDriverPtr driver,
>                       virDomainDefPtr def)
>       ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
>   
> +int qemuExtTPMSetupCgroup(virQEMUDriverPtr driver,
> +                          virDomainDefPtr def,
> +                          virCgroupPtr cgroup)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
> +    ATTRIBUTE_RETURN_CHECK;
> +
>   #endif /* __QEMU_TPM_H__ */
> 


-- 
Mit freundlichen Grüßen/Kind regards
    Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

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