On Thu, May 02, 2024 at 19:39:39 +0200, Andrea Bolognani wrote:
> As explained in the comment, this can help in scenarios where
> a shared filesystem can't be detected as such by libvirt, by
> giving the admin the opportunity to provide this information
> manually.
>
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> ---
> src/qemu/libvirtd_qemu.aug | 3 +++
> src/qemu/qemu.conf.in | 23 ++++++++++++++++++++++
> src/qemu/qemu_conf.c | 31 ++++++++++++++++++++++++++++++
> src/qemu/qemu_conf.h | 2 ++
> src/qemu/test_libvirtd_qemu.aug.in | 5 +++++
> 5 files changed, 64 insertions(+)
[...]
> diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
> index f406df8749..c543a0a55b 100644
> --- a/src/qemu/qemu.conf.in
> +++ b/src/qemu/qemu.conf.in
> @@ -986,3 +986,26 @@
> # note that the default might change in future releases.
> #
> #storage_use_nbdkit = @USE_NBDKIT_DEFAULT@
> +
> +# libvirt will normally prevent migration if the storage backing the VM is not
> +# on a shared filesystems. Sometimes, however, the storage *is* shared despite
> +# not being detected as such: for example, this is the case when one of the
> +# hosts involved in the migration is exporting its local storage to the other
> +# one via NFS.
I wanted to suggest using VIR_MIGRATE_UNSAFE flag to bypass this check,
but doing so without disabling dynamic ownership on the image itself
would still make the security driver cut off access to this file.
This means that users would have to use VIR_MIGRATE_UNSAFE and disabling
relabelling to all images they wish to use for such migration.
> +# Any directory listed here will be assumed to live on a shared filesystem,
> +# making migration possible in scenarios such as the one described above.
> +#
> +# Due to how label remembering is implemented, it will generally be necessary
> +# to set remember_owner=0 *on both sides of the migration* as well.
So IIUC the problem here is that on the box where the storage is local
this will create the XATTR entries used for remembering the lablels
(which also includes a refcount), which given that older NFS doesn't
support security labelling would then be leaked?
Looking at the code we seem to be calling 'qemuSecurityRestoreAllLabel'
with the 'migrated' flag set when migrating at all times. This should
make it theoretically possible to simply clear out the XATTRs for any
file which resides on the paths in question if we're migrating away
rather than ignoring them, which would solve the issue.
This would go together with the 'syntax-sugar'-ish flavor to this
feature. Additionally it would help in case, when the user configures
these paths after the VM was started and thus the XATTRs are already
applied. Those would not then be cleared on migration.
I also wanted to suggest to modify this paragraph to suggest a lesser
hammer to disable ownersip remembering, as that will disable it also for
the non-shared paths, but IIRC we don't have flags to disable it
individually (in contrast with the 'dynamic_ownership' flag which can be
disabled per-image via the 'relabel' attribute).
> +# NOTE: this option is intended to help in very specific scenarios that are
> +# rarely encountered. If you find yourself reaching for this option, consider
> +# reworking your environment so that it follows a more common architecture
> +# rather than using it.
> +#
> +#shared_filesystems = [
> +# "/path/to/images",
> +# "/path/to/nvram",
> +# "/path/to/swtpm"
> +#]
[...]
> +static int
> +virQEMUDriverConfigLoadFilesystemEntry(virQEMUDriverConfig *cfg,
> + virConf *conf)
> +{
> + char **iter;
> +
> + if (virConfGetValueStringList(conf, "shared_filesystems", false,
> + &cfg->sharedFilesystems) < 0)
> + return -1;
> +
> + if (!cfg->sharedFilesystems)
> + return 0;
> +
> + /* The paths provided by the user might contain trailing slashes
> + * and other fun diversions, which would break the naive string
> + * comparisons that we're later going to use them for */
> + for (iter = cfg->sharedFilesystems; *iter; iter++) {
> + char *canon = virFileCanonicalizePath(*iter);
'virFileCanonicalizePath' is using 'realpath()' inside which will return
'NULL' in case the path does not exist.
As the output is cached/remembered this would cause that:
- any path which doesn't exist at time the daemon started would not
work if created after startup of the daemon until the daemon is
restarted
- any valid config following this entry will not work either as this is
a string list and is accessed exclusively via NULL-termination.
If you want to canonicalize paths you must do so at the time this
information will be used.
> + g_free(*iter);
> + *iter = canon;
> + }
> +
> + return 0;
> +}
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
