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

[Stefan Berger]

On 05/21/2018 06:13 PM, John Ferlan wrote:
>
> On 05/15/2018 08:26 PM, Stefan Berger 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_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       | 751 ++++++++++++++++++++++++++++++++++++++++++++++
>>   src/qemu/qemu_tpm.h       |  50 +++
>>   8 files changed, 1029 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/qemu_migration.c b/src/qemu/qemu_migration.c
>> index f753e42d1b..42e68f3ecb 100644
>> --- a/src/qemu/qemu_migration.c
>> +++ b/src/qemu/qemu_migration.c
>> @@ -39,6 +39,7 @@
>>   #include "qemu_hotplug.h"
>>   #include "qemu_blockjob.h"
>>   #include "qemu_security.h"
>> +#include "qemu_extdevice.h"
>>   
>>   #include "domain_audit.h"
>>   #include "virlog.h"
>> @@ -2917,6 +2918,7 @@ qemuMigrationSrcConfirm(virQEMUDriverPtr driver,
>>       if (!virDomainObjIsActive(vm)) {
>>           if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
>>               virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
>> +            qemuExtDevicesCleanupHost(driver, vm->def);
> I believe ^^ this ^^ will eventually get called by ...
>
>>               vm->persistent = 0;
>>           }
>>           qemuDomainRemoveInactiveJob(driver, vm);
> ... ^^ this ^^
>
> qemuDomainRemoveInactiveJob calls qemuDomainRemoveInactive and seince
> vm->persistent is not set, the call to qemuExtDevicesCleanupHost will be
> done unconditionally.
>
> IDC if it stays, but unless there's a reason to call this outside of a
> job, maybe we should allow the RemoveInactiveJob to do the magic?  Thoughts?
>
>
>> @@ -4515,6 +4517,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
>>       if (!virDomainObjIsActive(vm) && ret == 0) {
>>           if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) {
>>               virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
>> +            qemuExtDevicesCleanupHost(driver, vm->def);
> Similar here.
>
> Which then leaves just qemuDomainRemoveInactive doing that cleanup!

Right. Removed the unnecessary scrubbing calls.

>
>>               vm->persistent = 0;
>>           }
>>           qemuDomainRemoveInactiveJob(driver, vm);
> Beyond that - looks good to me...
>
> Reviewed-by: John Ferlan <jferlan at redhat.com>

Thanks.

>
> John
>