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

[Stefan Berger]

On 05/15/2018 08:13 AM, Marc Hartmayer wrote:
> 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?

Yes, it can. One can undefine a domain while it is running. Though the 
placement of this call isn't necessary (anymore, maybe was some time 
ago). It's being cleaned up in qemuDomainRemoveInactive(). Thanks for 
pointing this out.

    Stefan