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@xxxxxxxxxxxxx>
>>>> ---
>>>> 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
