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

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