On Tue, Oct 31, 2023 at 06:12:59PM +0100, Andrea Bolognani wrote:
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> ---
> src/qemu/qemu_migration.c | 5 +++++
> src/qemu/qemu_migration.h | 1 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index ac58aa1a8c..bf6f1de310 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1600,6 +1600,11 @@ qemuMigrationSrcIsSafe(virDomainDef *def,
> if ((rc = virFileIsSharedFS(src)) < 0) {
> return false;
> } else if (rc == 0) {
> + /* Ignore the outcome of this check if we've been
> + * asked to assume that storage is shared */
> + if (flags & VIR_MIGRATE_ASSUME_SHARED_STORAGE)
> + break;
> +
> unsafe = true;
> }
> if ((rc = virFileIsClusterFS(src)) < 0)
So this is essentially telling this particular bit of code to ignore
what virFileIsSharedFS reported.
The problem this is not the only piece of code relevant to live
migration that cares about the virFileIsSharedFS answer. The TPM
code also checks virFileIsSharedFS in the context of migration.
Also in the security manager, we check virFileIsSharedFS and skip
restoring ownership on the src host, as that would kill access on
the dst host.
So while VIR_MIGRATE_ASSUME_SHARED_STORAGE may work in the precise
scenario you've tested, it looks like an incomplete solution to
the described problem, and liable to cause new problems.
Of course FORCE is no better, because that'll share all the same
problems with code not behaving correctly due to not detecting
a shared fs. The new flag though is promoting itself as a safer
variant of FORCE, and that is giving apps a misleading impression
of what its actually able todo.
I feel like the root problem here needs addressing in the
virFileIsSharedFS method in some manner. Whether or not a given
location is shared between two hosts cannot be reliably determined
from stat() alone, we need some other inputs.
Maybe this is as simple as having a qemu.conf parameter
shared_storage_paths = [ "/some/dir", "/other/dir"....]
and then we register those paths via virFile during startup.
Extending virFileIsSharedFS to work correctly in special deployment
scenarios can now be done by the admin when configuring the host.
Alternatively, we could make it potentially "just work" for NFS
at least by reading the "/etc/exports" file and identfiying which
local paths have been exported to other hosts.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
