[PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration

[Michal Prívozník]

On 10/18/22 14:23, Stefan Berger wrote:
> 
> 
> On 10/18/22 04:15, Daniel P. Berrangé wrote:
>> On Mon, Oct 17, 2022 at 11:17:56AM -0400, Stefan Berger wrote:
>>>
>>>
>>> On 10/17/22 09:48, Daniel P. Berrangé wrote:
>>>> On Mon, Oct 17, 2022 at 09:39:52AM -0400, Stefan Berger wrote:
>>>>>
>>>>>
>>>
>>>>
>>>> The key is in qemuMigrationSrcIsSafe(), and how it determines if a
>>>> migration is safe.
>>>>
>>>>     * Disk on local storage, no flags  => unsafe, migration error
>>>>     * Disk on local storage, VIR_MIGRATE_NON_SHARED_DISK => ok,
>>>> copies disk storage
>>>>     * Disk on shared storage, no flags => safe
>>>>     * Disk on shared storage, VIR_MIGRATE_NON_SHARED_DISK => ok, but
>>>> needlessly copies disk storage
>>>>
>>>> The key helper methods are virFileIsSharedFS and virFileIsClusterFS
>>>> which check the filesystem type for the path against a known list
>>>> of shared/cluster FS.
>>>>
>>>>> So we won't do it this way for TPM state migration. Instead we can
>>>>> try to write on the source migration side
>>>>>
>>>>> a) a simple file and detect whether the file is at the destination
>>>>> b) a file with either a name or content that only the source and
>>>>>      destination libvirts would know at this point
>>>>>
>>>>> b) is to prevent stale files from becoming indicators for shared
>>>>> storage.
>>>>
>>>> No, please use the virFileIsSharedFS/ClusterFS helpers.
>>>>
>>>
>>> I tried to use virFileIsSharedFS on the source and destination side of
>>> my NFS setup to see how they work. The issue here is that the NFS server
>>> side, that exports /var/lib/libvirt/swtpm and is also the migration
>>> source at some point, says this:
>>
>> We expect both sides to have the same storage configurtion. ie both
>> must be NFS. I'm pretty sure our code for handling disks does not
>> work when you have a local FS on one side, which is exported to the
>> other side as NFS. Conceptally that's not something someone would
>> do in production, since they you make the target host dependant
>> on the src compute host, which is poor for resilience.
>>
> 
> Ok, so then we can replace (accesses to) VIR_MIGARTE_TPM_SHARED_STORAGE
> with calls to virFilesIsSharedFS() and simply assume that this function
> will return the same results on both sides of the migration (without
> testing it) and if it doesn't and failures occur then it's an
> unsupported shared storage setup (that is documented somewhere). I hope
> to not receive reports from ppl that don't describe their setup
> appropriately but see some odd failures because of this.


Just FYI, I'm testing the following patches:

https://gitlab.com/MichalPrivoznik/libvirt/-/commits/tpm_migration_mine_v1

There're still some parts missing. but I'm continuing to work on them.

Michal