Re: [PATCH v2 2/9] qemu: Introduced VIR_MIGRATE_TPM_SHARED_STORAGE for TPM migration
On 10/17/22 11:28, Michal Prívozník wrote:
On 10/17/22 17:17, 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:
>
> /var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is
> shared: 0
>
> the NFS client side then says this:
>
>
> /var/lib/libvirt/swtpm/ecc221c4-6bb9-423f-ac31-72244fdbb1a1/tpm2 is
> shared: 1VIR_MIGRATE_TPM_SHARED_STORAGE
>
Are both sides mounted as NFS or just one of them? I think it's safe to
It's just one of them that is the client, the other is the server exporting the file
system.
So it will be a matter of documentation on how to setup the shared filesystem. For NFS
this means that both sides of the migration must be an NFS client and not one side acting
as the server exporting the filesystem to the client.
assume that either both sides are on a shared FS or none. Otherwise
I'm
quite sure file locking won't work anyways. We can go even further and
I saw an error about the locks on cleanup (due to missing code in the series to avoid
security label cleanup on TPM stop) but otherwise it's working fine.
test whether shared_fs(source) == shared_fs(dst); and throw an error
if
it doesn't.
You mean we would set the (now non-public? or not allowed to be set by user?) flag
VIR_MIGRATE_TPM_SHARED_STORAGE on the sources side if we determine shared storage and
again test on the destination side whether it thinks about shared storage (hopefully with
the source host and not some other host -- presumably an excluded case) in the same way as
the source host did?
And since I've misguided you on the way with flag I am willing to help
you write patches. Please let me know if you need any help.
I am not sure at this point, probably you have a better feeling how to do this correctly
than I do. If we could run virFileIsSharedFS on both sides and get the same result, this
would be easy but I think it's not that simple. It looks like we need to determine the
VIR_MIGRATE_TPM_SHARED_STORAGE on the source side and pass it to the destination side and
possibly test again there...
Stefan
Michal