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 <stefanb@xxxxxxxxxxxxxxxxxx>
> ---
> 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"
> 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.
> + 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,
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
