On Mon, Nov 2, 2020 at 5:19 PM Max Reitz <mre...@redhat.com> wrote:
>
> Whenever we encounter a directory with an st_dev or mount ID that
> differs from that of its parent, we set the FUSE_ATTR_SUBMOUNT flag so
> the guest can create a submount for it.
>
> We only need to do so in lo_do_lookup(). The following functions return
> a fuse_attr object:
> - lo_create(), though fuse_reply_create(): Calls lo_do_lookup().
> - lo_lookup(), though fuse_reply_entry(): Calls lo_do_lookup().
> - lo_mknod_symlink(), through fuse_reply_entry(): Calls lo_do_lookup().
> - lo_link(), through fuse_reply_entry(): Creating a link cannot create a
> submount, so there is no need to check for it.
> - lo_getattr(), through fuse_reply_attr(): Announcing submounts when the
> node is first detected (at lookup) is sufficient. We do not need to
> return the submount attribute later.
> - lo_do_readdir(), through fuse_add_direntry_plus(): Calls
> lo_do_lookup().
>
> Make announcing submounts optional, so submounts are only announced to
> the guest with the announce_submounts option. Some users may prefer the
> current behavior, so that the guest learns nothing about the host mount
> structure.
>
> (announce_submounts is force-disabled when the guest does not present
> the FUSE_SUBMOUNTS capability, or when there is no statx().)
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
> ---
> tools/virtiofsd/helper.c | 1 +
> tools/virtiofsd/passthrough_ll.c | 22 ++++++++++++++++++++++
> 2 files changed, 23 insertions(+)
>
> diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
> index 2e181a49b5..4433724d3a 100644
> --- a/tools/virtiofsd/helper.c
> +++ b/tools/virtiofsd/helper.c
> @@ -190,6 +190,7 @@ void fuse_cmdline_help(void)
> " retain/discard O_DIRECT flags
> passed down\n"
> " to virtiofsd from guest
> applications.\n"
> " default: no_allow_direct_io\n"
> + " -o announce_submounts Announce sub-mount points to the
> guest\n"
> );
> }
>
> diff --git a/tools/virtiofsd/passthrough_ll.c
> b/tools/virtiofsd/passthrough_ll.c
> index 34d107975f..ec1008bceb 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -40,6 +40,7 @@
> #include "fuse_virtio.h"
> #include "fuse_log.h"
> #include "fuse_lowlevel.h"
> +#include "standard-headers/linux/fuse.h"
> #include <assert.h>
> #include <cap-ng.h>
> #include <dirent.h>
> @@ -167,6 +168,7 @@ struct lo_data {
> int readdirplus_set;
> int readdirplus_clear;
> int allow_direct_io;
> + int announce_submounts;
> bool use_statx;
> struct lo_inode root;
> GHashTable *inodes; /* protected by lo->mutex */
> @@ -207,6 +209,7 @@ static const struct fuse_opt lo_opts[] = {
> { "no_readdirplus", offsetof(struct lo_data, readdirplus_clear), 1 },
> { "allow_direct_io", offsetof(struct lo_data, allow_direct_io), 1 },
> { "no_allow_direct_io", offsetof(struct lo_data, allow_direct_io), 0 },
> + { "announce_submounts", offsetof(struct lo_data, announce_submounts), 1
> },
> FUSE_OPT_END
> };
> static bool use_syslog = false;
> @@ -601,6 +604,20 @@ static void lo_init(void *userdata, struct
> fuse_conn_info *conn)
> fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling readdirplus\n");
> conn->want &= ~FUSE_CAP_READDIRPLUS;
> }
> +
> + if (!(conn->capable & FUSE_CAP_SUBMOUNTS) && lo->announce_submounts) {
> + fuse_log(FUSE_LOG_WARNING, "lo_init: Cannot announce submounts,
> client "
> + "does not support it\n");
> + lo->announce_submounts = false;
> + }
> +
> +#ifndef CONFIG_STATX
> + if (lo->announce_submounts) {
> + fuse_log(FUSE_LOG_WARNING, "lo_init: Cannot announce submounts,
> there "
> + "is no statx()\n");
> + lo->announce_submounts = false;
Minor issue: this warns only when libc doesn't have statx, and not
when kernel doesn't have it.
> + }
> +#endif
> }
>
> static void lo_getattr(fuse_req_t req, fuse_ino_t ino,
> @@ -877,6 +894,11 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t
> parent, const char *name,
> goto out_err;
> }
>
> + if (S_ISDIR(e->attr.st_mode) && lo->announce_submounts &&
> + (e->attr.st_dev != dir->key.dev || mnt_id != dir->key.mnt_id)) {
> + e->attr_flags |= FUSE_ATTR_SUBMOUNT;
> + }
... and since announce_submounts isn't turned off in case the kernel
doesn't have STATX_MNT_ID, this will trigger a submount when crossing
into a different filesystem.
Possible solutions:
a) test and warn at startup in case kernel doesn't have statx
b) do not test st_dev, only mnt_id (which will always be zero if not supported)
c) merge announce_submounts and use_statx, which are basically doing
the same thing
> +
> inode = lo_find(lo, &e->attr, mnt_id);
> if (inode) {
> close(newfd);
> --
> 2.26.2
>
