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

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.