On 05/08/2018 05:07 PM, John Ferlan wrote:

On 05/04/2018 04:21 PM, Stefan Berger wrote:
This patch adds support for an external swtpm TPM emulator. The XML for
this type of TPM looks as follows:

  <tpm model='tpm-tis'>
    <backend type='emulator'/>
  </tpm>

The XML will currently only start a TPM 1.2.

Upon first start, libvirt will run `swtpm_setup`, which will simulate the
manufacturing of a TPM and create certificates for it and write them into
NVRAM locations of the emulated TPM.

After that libvirt starts the swtpm TPM emulator using the `swtpm` executable.

Once the VM terminates, libvirt uses the swtpm_ioctl executable to gracefully
shut down the `swtpm` in case it is still running (QEMU did not send shutdown)
or clean up the socket file.

The above mentioned executables must be found in the PATH.

The executables can either be run as root or started as root and switch to
the tss user. The requirement for the tss user comes through 'tcsd', which
is used for the simulation of the manufacturing. Which user is used can be
configured through qemu.conf. By default 'tss' is used.

The swtpm writes out state into files. The state is kept in 
/var/lib/libvirt/swtpm:

[root@localhost libvirt]# ls -lZ | grep swtpm

drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 Apr  5 
16:22 swtpm

The directory /var/lib/libvirt/swtpm maintains per-TPM state directories.
(Using the uuid of the VM for that since the name can change per VM renaming but
  we need a stable directory name.)

[root@localhost swtpm]# ls -lZ
total 4
drwx------. 2 tss  tss  system_u:object_r:virt_var_lib_t:s0          4096 Apr  
5 16:46 485d0004-a48f-436a-8457-8a3b73e28568

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28568]# ls -lZ
total 4
drwx------. 2 tss tss system_u:object_r:virt_var_lib_t:s0 4096 Apr 10 21:34 
tpm1.2

[root@localhost tpm1.2]# ls -lZ
total 8
-rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 3648 Apr  5 16:46 
tpm-00.permall

The directory /var/run/libvirt/qemu/swtpm/ hosts the swtpm.sock that
QEMU uses to communicate with the swtpm:

root@localhost domain-1-testvm]# ls -lZ
total 0
srw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632  0 Apr  6 
10:24 1-testvm-swtpm.sock

The logfile for the swtpm is in /var/log/swtpm/libvirt/qemu:

[root@localhost-3 qemu]# ls -lZ
total 4
-rw-------. 1 tss tss unconfined_u:object_r:var_log_t:s0 2199 Apr  6 14:01 
testvm-swtpm.log

The processes are labeled as follows:

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | 
grep socket | grep -v grep
system_u:system_r:virtd_t:s0-s0:c0.c1023 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

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | 
grep tpm | grep -v grep
system_u:system_r:svirt_t:s0:c413,c430 qemu 18702 2.5  0.0 3036052 48676 ?     
Sl   16:46   0:08 /bin/qemu-system-x86_64 [...]

Signed-off-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
  src/conf/domain_conf.c   | 22 ++++++++++++++++++++++
  src/libvirt_private.syms |  1 +
  src/qemu/qemu_command.c  | 39 +++++++++++++++++++++++++++++++++------
  src/qemu/qemu_domain.c   |  3 +++
  src/qemu/qemu_driver.c   |  7 +++++++
  5 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d9945dd..a42574a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2593,6 +2593,24 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
      }
  }
2 blank lines and
void
virDomainTPMDelete(virDomainDefPtr def)

+void virDomainTPMDelete(virDomainDefPtr def)
+{
+    virDomainTPMDefPtr tpm = def->tpm;
+
+    if (!tpm)
+        return;
+
+    switch (tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        virTPMDeleteEmulatorStorage(tpm);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_LAST:
+        /* nothing to do */
+        break;
+    }
+}
+
  void virDomainTPMDefFree(virDomainTPMDefPtr def)
  {
      if (!def)
@@ -27614,6 +27632,10 @@ virDomainDeleteConfig(const char *configDir,
          goto cleanup;
      }
+ /* in case domain is NOT running, remove any TPM storage */
+    if (!dom->persistent)
+        virDomainTPMDelete(dom->def);
+
      ret = 0;
cleanup:
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index eebfc72..e533b95 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -559,6 +559,7 @@ virDomainTimerTrackTypeToString;
  virDomainTPMBackendTypeFromString;
  virDomainTPMBackendTypeToString;
  virDomainTPMDefFree;
+virDomainTPMDelete;
  virDomainTPMModelTypeFromString;
  virDomainTPMModelTypeToString;
  virDomainUSBDeviceDefForeach;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index bb330bf..c02b783 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9425,21 +9425,31 @@ qemuBuildTPMDevStr(const virDomainDef *def,
static char *
-qemuBuildTPMBackendStr(const virDomainDef *def,
+qemuBuildTPMBackendStr(virDomainDef *def,
Don't lose the "const"

a left-over from some previous call that made changes to 'tpm'. fixed.


                         virCommandPtr cmd,
                         virQEMUCapsPtr qemuCaps,
                         int *tpmfd,
-                       int *cancelfd)
+                       int *cancelfd,
+                       char **chardev)
  {
-    const virDomainTPMDef *tpm = def->tpm;
+    virDomainTPMDef *tpm = def->tpm;
Don't lose the "const"

      virBuffer buf = VIR_BUFFER_INITIALIZER;
-    const char *type = virDomainTPMBackendTypeToString(tpm->type);
+    const char *type = NULL;
      char *cancel_path = NULL, *devset = NULL;
      const char *tpmdev;
*tpmfd = -1;
      *cancelfd = -1;
+ switch (tpm->type) {
+    case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+    case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        type = virDomainTPMBackendTypeToString(tpm->type);
+        break;
+    case VIR_DOMAIN_TPM_TYPE_LAST:
     default:
         virReportEnumRangeError(virDomainTPMBackendType, tpm->type);

We need some sort of error message otherwise we get failed for some
reason which is never fun to diagnose.

All other cases I see use the same function without error message. Not sure what you mean. We seem to follow a pattern with this now.


+        goto error;
+    }
+
      virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias);
switch (tpm->type) {
@@ -9491,6 +9501,16 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
break;
      case VIR_DOMAIN_TPM_TYPE_EMULATOR:
+        if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR))
+            goto no_support;
+
+        virBufferAddLit(&buf, ",chardev=chrtpm");
+
+        if (virAsprintf(chardev, "socket,id=chrtpm,path=%s",
+                        tpm->data.emulator.source.data.nix.path) < 0)
+            goto error;
+
+        break;
      case VIR_DOMAIN_TPM_TYPE_LAST:
          goto error;
      }
@@ -9517,10 +9537,11 @@ qemuBuildTPMBackendStr(const virDomainDef *def,
static int
  qemuBuildTPMCommandLine(virCommandPtr cmd,
-                        const virDomainDef *def,
+                        virDomainDef *def,
Don't lose the "const"

                          virQEMUCapsPtr qemuCaps)
  {
      char *optstr;
+    char *chardev = NULL;
      int tpmfd = -1;
      int cancelfd = -1;
      char *fdset;
@@ -9529,12 +9550,18 @@ qemuBuildTPMCommandLine(virCommandPtr cmd,
          return 0;
if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps,
-                                          &tpmfd, &cancelfd)))
+                                          &tpmfd, &cancelfd,
+                                          &chardev)))
          return -1;
virCommandAddArgList(cmd, "-tpmdev", optstr, NULL);
      VIR_FREE(optstr);
+ if (chardev) {
+        virCommandAddArgList(cmd, "-chardev", chardev, NULL);
+        VIR_FREE(chardev);
+    }
+
      if (tpmfd >= 0) {
          fdset = qemuVirCommandGetFDSet(cmd, tpmfd);
          if (!fdset)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3eac43..57a82dc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -34,6 +34,7 @@
  #include "qemu_migration.h"
  #include "qemu_migration_params.h"
  #include "qemu_security.h"
+#include "qemu_extdevice.h"
  #include "viralloc.h"
  #include "virlog.h"
  #include "virerror.h"
@@ -7166,6 +7167,8 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
              VIR_WARN("unable to remove snapshot directory %s", snapDir);
          VIR_FREE(snapDir);
      }
+    if (!qemuExtDevicesInitPaths(driver, vm->def))
I know it's more or less functionally equivalent, but it's better to use
"if (qemuExtDevicesInitPaths(driver, vm->def) == 0)" since the function
is not a boolean or pointer returning function.

With suggested adjustments,

Done. Thanks.


Reviewed-by: John Ferlan <jfer...@redhat.com>

John

+        virDomainTPMDelete(vm->def);
virObjectRef(vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9ce97ea..f496f89 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -60,6 +60,7 @@
  #include "qemu_migration_params.h"
  #include "qemu_blockjob.h"
  #include "qemu_security.h"
+#include "qemu_extdevice.h"
#include "virerror.h"
  #include "virlog.h"
@@ -7349,6 +7350,9 @@ qemuDomainCreateWithFlags(virDomainPtr dom, unsigned int 
flags)
          goto endjob;
      }
+ if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
+        goto endjob;
+
      if (qemuDomainObjStart(dom->conn, driver, vm, flags,
                             QEMU_ASYNC_JOB_START) < 0)
          goto endjob;
@@ -7494,6 +7498,9 @@ qemuDomainUndefineFlags(virDomainPtr dom,
      if (!(vm = qemuDomObjFromDomain(dom)))
          return -1;
+ if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
+        return -1;
+
      cfg = virQEMUDriverGetConfig(driver);
if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0)


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

Reply via email to