[libvirt] [PATCH v4 06/11] qemu: Extend QEMU with external TPM support

[Marc Hartmayer]

On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb at linux.vnet.ibm.com> wrote:
> Implement functions for managing the storage of the external swtpm as well
> as starting and stopping it. Also implement functions to use swtpm_setup,
> which simulates the manufacturing of a TPM, which includes creation of
> certificates for the device.
>
> Further, the external TPM needs storage on the host that we need to set
> up before it can be run. We can clean up the host once the domain is
> undefined.
>
> This patch also implements a small layer for external device support that
> calls into the TPM device layer if a domain has an attached TPM. This is
> the layer we will wire up later on.
>
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> ---
>  src/qemu/Makefile.inc.am  |   4 +
>  src/qemu/qemu_domain.c    |   2 +
>  src/qemu/qemu_driver.c    |   5 +
>  src/qemu/qemu_extdevice.c | 154 ++++++++++
>  src/qemu/qemu_extdevice.h |  53 ++++
>  src/qemu/qemu_migration.c |   3 +
>  src/qemu/qemu_process.c   |  12 +
>  src/qemu/qemu_tpm.c       | 753 ++++++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_tpm.h       |  50 +++
>  9 files changed, 1036 insertions(+)
>  create mode 100644 src/qemu/qemu_extdevice.c
>  create mode 100644 src/qemu/qemu_extdevice.h
>  create mode 100644 src/qemu/qemu_tpm.c
>  create mode 100644 src/qemu/qemu_tpm.h
>
> diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> index 7f50501..46797af 100644
> --- a/src/qemu/Makefile.inc.am
> +++ b/src/qemu/Makefile.inc.am
> @@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \
>  	qemu/qemu_domain_address.h \
>  	qemu/qemu_cgroup.c \
>  	qemu/qemu_cgroup.h \
> +	qemu/qemu_extdevice.c \
> +	qemu/qemu_extdevice.h \
>  	qemu/qemu_hostdev.c \
>  	qemu/qemu_hostdev.h \
>  	qemu/qemu_hotplug.c \
> @@ -51,6 +53,8 @@ QEMU_DRIVER_SOURCES = \
>  	qemu/qemu_security.h \
>  	qemu/qemu_qapi.c \
>  	qemu/qemu_qapi.h \
> +	qemu/qemu_tpm.c \
> +	qemu/qemu_tpm.h \
>  	$(NULL)
>
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 774a102..e2d2a24 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"
> @@ -7174,6 +7175,7 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
>              VIR_WARN("unable to remove snapshot directory %s", snapDir);
>          VIR_FREE(snapDir);
>      }
> +    qemuExtDevicesCleanupHost(driver, vm->def);
>
>      virDomainObjListRemove(driver->domains, vm);
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b03eb30..b576a4d 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"
> @@ -7558,6 +7559,10 @@ qemuDomainUndefineFlags(virDomainPtr dom,
>      if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0)
>          goto endjob;
>
> +    /* in case domain is NOT running, remove any TPM storage */
> +    if (!vm->persistent)
       ^^^^^^^^^^^^^^^^^^^^
       Can this really happen since there is a guard against this
       situation in the code?

“
 if (!vm->persistent) {
    virReportError(VIR_ERR_OPERATION_INVALID,
                   "%s", _("cannot undefine transient domain"));
    …
”

You’re validating that the domain is not persistent… but your comment
says 'not running'.

And why are you doing this in qemuDomainUndefineFlags and not in
something like qemuDomainDestroyFlags and processMonitorEOFEvent?

[…snip]

Haven’t looked into more detail right now.

Beste Grüße / Kind regards
   Marc Hartmayer

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