[PATCH v3 4/6] qemu: tpm: Pass --migration option to swtpm if supported and needed

[Michal Prívozník]

On 10/21/22 15:31, Michal Prívozník wrote:
> On 10/21/22 13:36, Stefan Berger wrote:
>>
>>
>> On 10/21/22 06:55, Michal Prívozník wrote:
>>> On 10/18/22 19:04, Stefan Berger wrote:
>>>> Pass the --migration option to swtpm if swptm supports it (starting
>>>> with v0.8) and if the TPM's state is written on shared storage. If this
>>>> is the case apply the 'release-lock-outgoing' parameter with this
>>>> option and apply the 'incoming' parameter for incoming migration so that
>>>> swtpm releases the file lock on the source side when the state is
>>>> migrated
>>>> and locks the file on the destination side when the state is received.
>>>>
>>>> If a started swtpm instance is running with the necessary options of
>>>> migrating with share storage then remember this with a flag in the
>>>> virDomainTPMPrivateDef.
>>>>
>>>> Report an error if swtpm does not support the --migration option and an
>>>> incoming migration across shared storage is requested.
>>>>
>>>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>>>> ---
>>>>   src/qemu/qemu_migration.c |  8 +++++
>>>>   src/qemu/qemu_tpm.c       | 66 ++++++++++++++++++++++++++++++++++++++-
>>>>   src/qemu/qemu_tpm.h       |  6 ++++
>>>>   3 files changed, 79 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
>>>> index 33105cf07b..5b4f4615ee 100644
>>>> --- a/src/qemu/qemu_migration.c
>>>> +++ b/src/qemu/qemu_migration.c
>>>> @@ -38,6 +38,7 @@
>>>>   #include "qemu_security.h"
>>>>   #include "qemu_slirp.h"
>>>>   #include "qemu_block.h"
>>>> +#include "qemu_tpm.h"
>>>>     #include "domain_audit.h"
>>>>   #include "virlog.h"
>>>> @@ -2789,6 +2790,13 @@ qemuMigrationSrcBegin(virConnectPtr conn,
>>>>           goto cleanup;
>>>>       }
>>>>   +    if (qemuTPMHasSharedStorage(vm->def) &&
>>>> +        !qemuTPMCanMigrateSharedStorage(vm->def)) {
>>>
>>> This works, but only because qemuValidateDomainDefTPMs() denies two
>>> emulated TPMs. I think we can call just qemuTPMCanMigrateSharedStorage()
>>> since it already iterates over all TPMs and checks each one individually.
>>
>> I am only starting swtpm with the necessary command line options for
>> shared storage migration if a) shared storage is detected at the time of
>> the start of the VM and b) if swtpm supports shared storage migration at
>> all. If it doesn't support it I let you start the VM but won't let you
>> migrate.
>>
>>
>> qemuTPMCanMigrateSharedStorage() checks whether the running swtpm
>> instance can do b). It doesn't check the swtpm executable that you may
>> have upgraded or downgraded in the meantime. That's why I need the
>> private data.
>>
>> So the check above tests whether there is shared storage detected for
>> the TPM_EMULATOR (swtpm; only one allowed) and if so whether the
>> *running swtpm instance* supports shared storage migration. I think it
>> is correct this way.
> 
> Ah, good point. So let me fix the other small nits I've raised and merge
> these.

Actually, I'll investigate whether we need the very last patch. I mean,
we definitely do not want to remove the swtpm state from the source, BUT
I wonder whether we can set @flags passed to
qemuDomainRemoveInactiveCommon() (e.g. to VIR_DOMAIN_UNDEFINE_KEEP_TPM)
instead of introducing another argument with basically the same
functionality.

Michal