[PATCH 0/8] virtiofsd: Announce submounts to the guest

[PATCH 0/8] virtiofsd: Announce submounts to the guest

[Max Reitz]

RFC: https://www.redhat.com/archives/virtio-fs/2020-May/msg00024.html

Branch: https://github.com/XanClic/qemu.git virtiofs-submounts-v2
Branch: https://git.xanclic.moe/XanClic/qemu.git virtiofs-submounts-v2


(Note that there is an accompanying Linux (kernel) series
“fuse: Mirror virtio-fs submounts”.)


Hi,

We want to (be able to) announce the host mount structure of the shared
directory to the guest so it can replicate that structure.  This ensures
that whenever the combination of st_dev and st_ino is unique on the
host, it will be unique in the guest as well.

This feature is optional and needs to be enabled explicitly, so that the
mount structure isn’t leaked to the guest if the user doesn’t want it to
be.

The last patch in this series adds a test script.  For it to pass, you
need to compile a kernel with the accompanying “fuse: Mirror virtio-fs
submounts” patch series, and provide it to the test (as described in the
test patch).


Known caveats:
- stat(2) doesn’t trigger auto-mounting.  Therefore, issuing a stat() on
  a sub-mountpoint before it’s been auto-mounted will show its parent’s
  st_dev together with the st_ino it has in the sub-mounted filesystem.

  For example, imagine you want to share a whole filesystem with the
  guest, which on the host first looks like this:

    root/           (st_dev=64, st_ino=128)
      sub_fs/       (st_dev=64, st_ino=234)

  And then you mount another filesystem under sub_fs, so it looks like
  this:

    root/           (st_dev=64, st_ino=128)
      sub_fs/       (st_dev=96, st_ino=128)
        ...

  As you can see, sub_fs becomes a mount point, so its st_dev and st_ino
  change from what they were on root’s filesystem to what they are in
  the sub-filesystem.  In fact, root and sub_fs now have the same
  st_ino, which is not unlikely given that both are root nodes in their
  respective filesystems.

  Now, this filesystem is shared with the guest through virtiofsd.
  There is no way for virtiofsd to uncover sub_fs’s original st_ino
  value of 234, so it will always provide st_ino=128 to the guest.
  However, virtiofsd does notice that sub_fs is a mount point and
  announces this fact to the guest.

  We want this to result in something like the following tree in the
  guest:

    root/           (st_dev=32, st_ino=128)
      sub_fs/       (st_dev=33, st_ino=128)
        ...

  That is, sub_fs should be a different filesystem that’s auto-mounted.
  However, as stated above, stat(2) doesn’t trigger auto-mounting, so
  before it happens, the following structure will be visible:

    root/           (st_dev=32, st_ino=128)
      sub_fs/       (st_dev=32, st_ino=128)

  That is, sub_fs and root will have the same st_dev/st_ino combination.

  This can easily be seen by executing find(1) on root in the guest,
  which will subsequently complain about an alleged filesystem loop.

  To properly fix this problem, we probably would have to be able to
  uncover sub_fs’s original st_ino value (i.e. 234) and let the guest
  use that until the auto-mount happens.  However, there is no way to
  get that value (from userspace at least).

  Note that NFS with crossmnt has the exact same issue.


- You can unmount auto-mounted submounts in the guest, but then you
  still cannot unmount them on the host.  The guest still holds a
  reference to the submount’s root directory, because that’s just a
  normal entry in its parent directory (on the submount’s parent
  filesystem).

  This is kind of related to the issue noted above: When the submount is
  unmounted, the guest shouldn’t have a reference to sub_fs as the
  submount’s root directory (host’s st_dev=96, st_ino=128), but to it as
  a normal entry in its parent filesystem (st_dev=64, st_ino=234).

  (When you have multiple nesting levels, you can unmount inner mounts
  when the outer ones have been unmounted in the guest.  For example,
  say you have a structure A/B/C/D, where each is a mount point, then
  unmounting D, C, and B in the guest will allow the host to unmount D
  and C.)


Max Reitz (8):
  linux/fuse.h: Pull in from Linux
  virtiofsd: Announce FUSE_ATTR_FLAGS
  virtiofsd: Add attr_flags to fuse_entry_param
  virtiofsd: Add fuse_reply_attr_with_flags()
  virtiofsd: Store every lo_inode's parent_dev
  virtiofsd: Announce sub-mount points
  tests/acceptance/boot_linux: Accept SSH pubkey
  tests/acceptance: Add virtiofs_submounts.py

 include/standard-headers/linux/fuse.h         |  11 +-
 tools/virtiofsd/fuse_common.h                 |   8 +
 tools/virtiofsd/fuse_lowlevel.h               |  20 ++
 tools/virtiofsd/fuse_lowlevel.c               |  34 ++-
 tools/virtiofsd/helper.c                      |   1 +
 tools/virtiofsd/passthrough_ll.c              |  84 ++++-
 tests/acceptance/boot_linux.py                |  13 +-
 tests/acceptance/virtiofs_submounts.py        | 289 ++++++++++++++++++
 .../virtiofs_submounts.py.data/cleanup.sh     |  46 +++
 .../guest-cleanup.sh                          |  30 ++
 .../virtiofs_submounts.py.data/guest.sh       | 138 +++++++++
 .../virtiofs_submounts.py.data/host.sh        | 127 ++++++++
 12 files changed, 780 insertions(+), 21 deletions(-)
 create mode 100644 tests/acceptance/virtiofs_submounts.py
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/cleanup.sh
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/guest-cleanup.sh
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/guest.sh
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/host.sh

-- 
2.26.2

[PATCH 1/8] linux/fuse.h: Pull in from Linux

[Max Reitz]

Update the linux/fuse.h standard header from the kernel development tree
that implements FUSE submounts.

This adds the fuse_attr.flags field, the FUSE_ATTR_FLAGS INIT flag, and
the FUSE_ATTR_SUBMOUNT flag for fuse_attr.flags.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/standard-headers/linux/fuse.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/standard-headers/linux/fuse.h b/include/standard-headers/linux/fuse.h
index f4df0a40f6..7dd7a3b992 100644
--- a/include/standard-headers/linux/fuse.h
+++ b/include/standard-headers/linux/fuse.h
@@ -227,7 +227,7 @@ struct fuse_attr {
 	uint32_t	gid;
 	uint32_t	rdev;
 	uint32_t	blksize;
-	uint32_t	padding;
+	uint32_t	flags;
 };
 
 struct fuse_kstatfs {
@@ -310,6 +310,7 @@ struct fuse_file_lock {
  * FUSE_NO_OPENDIR_SUPPORT: kernel supports zero-message opendir
  * FUSE_EXPLICIT_INVAL_DATA: only invalidate cached pages on explicit request
  * FUSE_MAP_ALIGNMENT: map_alignment field is valid
+ * FUSE_ATTR_FLAGS: fuse_attr.flags is present and valid
  */
 #define FUSE_ASYNC_READ		(1 << 0)
 #define FUSE_POSIX_LOCKS	(1 << 1)
@@ -338,6 +339,7 @@ struct fuse_file_lock {
 #define FUSE_NO_OPENDIR_SUPPORT (1 << 24)
 #define FUSE_EXPLICIT_INVAL_DATA (1 << 25)
 #define FUSE_MAP_ALIGNMENT	(1 << 26)
+#define FUSE_ATTR_FLAGS		(1 << 27)
 
 /**
  * CUSE INIT request/reply flags
@@ -413,6 +415,13 @@ struct fuse_file_lock {
  */
 #define FUSE_FSYNC_FDATASYNC	(1 << 0)
 
+/**
+ * fuse_attr flags
+ *
+ * FUSE_ATTR_SUBMOUNT: File/directory is a submount point
+ */
+#define FUSE_ATTR_SUBMOUNT      (1 << 0)
+
 enum fuse_opcode {
 	FUSE_LOOKUP		= 1,
 	FUSE_FORGET		= 2,  /* no reply */
-- 
2.26.2

[PATCH 2/8] virtiofsd: Announce FUSE_ATTR_FLAGS

[Max Reitz]

The fuse_attr.flags field is currently just initialized to 0, which is
valid.  Thus, there is no reason not to always announce FUSE_ATTR_FLAGS
(when the kernel supports it).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tools/virtiofsd/fuse_common.h   | 8 ++++++++
 tools/virtiofsd/fuse_lowlevel.c | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
index 686c42c0a5..870544fe13 100644
--- a/tools/virtiofsd/fuse_common.h
+++ b/tools/virtiofsd/fuse_common.h
@@ -352,6 +352,14 @@ struct fuse_file_info {
  */
 #define FUSE_CAP_NO_OPENDIR_SUPPORT (1 << 24)
 
+/**
+ * Indicates that the client will provide fuse_attr.flags, and the kernel will
+ * interpret it.
+ *
+ * This feature is enabled by default when supported by the kernel.
+ */
+#define FUSE_CAP_ATTR_FLAGS (1 << 27)
+
 /**
  * Ioctl flags
  *
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 2dd36ec03b..e9e2652d33 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -1988,6 +1988,9 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
             bufsize = max_bufsize;
         }
     }
+    if (arg->flags & FUSE_ATTR_FLAGS) {
+        se->conn.capable |= FUSE_CAP_ATTR_FLAGS;
+    }
 #ifdef HAVE_SPLICE
 #ifdef HAVE_VMSPLICE
     se->conn.capable |= FUSE_CAP_SPLICE_WRITE | FUSE_CAP_SPLICE_MOVE;
@@ -2014,6 +2017,7 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
     LL_SET_DEFAULT(1, FUSE_CAP_ASYNC_DIO);
     LL_SET_DEFAULT(1, FUSE_CAP_IOCTL_DIR);
     LL_SET_DEFAULT(1, FUSE_CAP_ATOMIC_O_TRUNC);
+    LL_SET_DEFAULT(1, FUSE_CAP_ATTR_FLAGS);
     LL_SET_DEFAULT(se->op.write_buf, FUSE_CAP_SPLICE_READ);
     LL_SET_DEFAULT(se->op.getlk && se->op.setlk, FUSE_CAP_POSIX_LOCKS);
     LL_SET_DEFAULT(se->op.flock, FUSE_CAP_FLOCK_LOCKS);
@@ -2103,6 +2107,9 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
     if (se->conn.want & FUSE_CAP_POSIX_ACL) {
         outarg.flags |= FUSE_POSIX_ACL;
     }
+    if (se->conn.want & FUSE_CAP_ATTR_FLAGS) {
+        outarg.flags |= FUSE_ATTR_FLAGS;
+    }
     outarg.max_readahead = se->conn.max_readahead;
     outarg.max_write = se->conn.max_write;
     if (se->conn.max_background >= (1 << 16)) {
-- 
2.26.2

[PATCH 3/8] virtiofsd: Add attr_flags to fuse_entry_param

[Max Reitz]

fuse_entry_param is converted to fuse_attr on the line (by
fill_entry()), so it should have a member that mirrors fuse_attr.flags.

fill_entry() should then copy this fuse_entry_param.attr_flags to
fuse_attr.flags.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tools/virtiofsd/fuse_lowlevel.h |  5 +++++
 tools/virtiofsd/fuse_lowlevel.c | 13 +++++++++----
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
index 562fd5241e..9c06240f9e 100644
--- a/tools/virtiofsd/fuse_lowlevel.h
+++ b/tools/virtiofsd/fuse_lowlevel.h
@@ -102,6 +102,11 @@ struct fuse_entry_param {
      *  large value.
      */
     double entry_timeout;
+
+    /**
+     * Flags for fuse_attr.flags that do not fit into attr.
+     */
+    uint32_t attr_flags;
 };
 
 /**
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index e9e2652d33..3ca49456c3 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -329,7 +329,8 @@ static unsigned int calc_timeout_nsec(double t)
     }
 }
 
-static void fill_entry(struct fuse_entry_out *arg,
+static void fill_entry(struct fuse_session *se,
+                       struct fuse_entry_out *arg,
                        const struct fuse_entry_param *e)
 {
     *arg = (struct fuse_entry_out){
@@ -341,6 +342,10 @@ static void fill_entry(struct fuse_entry_out *arg,
         .attr_valid_nsec = calc_timeout_nsec(e->attr_timeout),
     };
     convert_stat(&e->attr, &arg->attr);
+
+    if (se->conn.capable & FUSE_CAP_ATTR_FLAGS) {
+        arg->attr.flags = e->attr_flags;
+    }
 }
 
 /*
@@ -365,7 +370,7 @@ size_t fuse_add_direntry_plus(fuse_req_t req, char *buf, size_t bufsize,
 
     struct fuse_direntplus *dp = (struct fuse_direntplus *)buf;
     memset(&dp->entry_out, 0, sizeof(dp->entry_out));
-    fill_entry(&dp->entry_out, e);
+    fill_entry(req->se, &dp->entry_out, e);
 
     struct fuse_dirent *dirent = &dp->dirent;
     *dirent = (struct fuse_dirent){
@@ -403,7 +408,7 @@ int fuse_reply_entry(fuse_req_t req, const struct fuse_entry_param *e)
     size_t size = sizeof(arg);
 
     memset(&arg, 0, sizeof(arg));
-    fill_entry(&arg, e);
+    fill_entry(req->se, &arg, e);
     return send_reply_ok(req, &arg, size);
 }
 
@@ -416,7 +421,7 @@ int fuse_reply_create(fuse_req_t req, const struct fuse_entry_param *e,
     struct fuse_open_out *oarg = (struct fuse_open_out *)(buf + entrysize);
 
     memset(buf, 0, sizeof(buf));
-    fill_entry(earg, e);
+    fill_entry(req->se, earg, e);
     fill_open(oarg, f);
     return send_reply_ok(req, buf, entrysize + sizeof(struct fuse_open_out));
 }
-- 
2.26.2

[PATCH 4/8] virtiofsd: Add fuse_reply_attr_with_flags()

[Max Reitz]

The plain fuse_reply_attr() function does not allow setting
fuse_attr.flags, so add this new function that does.

Make fuse_reply_attr() a wrapper around it.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tools/virtiofsd/fuse_lowlevel.h | 15 +++++++++++++++
 tools/virtiofsd/fuse_lowlevel.c | 14 ++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
index 9c06240f9e..1ff6ba1e4f 100644
--- a/tools/virtiofsd/fuse_lowlevel.h
+++ b/tools/virtiofsd/fuse_lowlevel.h
@@ -1313,6 +1313,21 @@ int fuse_reply_create(fuse_req_t req, const struct fuse_entry_param *e,
 int fuse_reply_attr(fuse_req_t req, const struct stat *attr,
                     double attr_timeout);
 
+/**
+ * Reply with attributes and set fuse_attr.flags
+ *
+ * Possible requests:
+ *   getattr, setattr
+ *
+ * @param req request handle
+ * @param attr the attributes
+ * @param attr_timeout validity timeout (in seconds) for the attributes
+ * @param attr_flags flags to put into fuse_attr.flags
+ * @return zero for success, -errno for failure to send reply
+ */
+int fuse_reply_attr_with_flags(fuse_req_t req, const struct stat *attr,
+                               double attr_timeout, uint32_t attr_flags);
+
 /**
  * Reply with the contents of a symbolic link
  *
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 3ca49456c3..70efd3feaf 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -426,8 +426,8 @@ int fuse_reply_create(fuse_req_t req, const struct fuse_entry_param *e,
     return send_reply_ok(req, buf, entrysize + sizeof(struct fuse_open_out));
 }
 
-int fuse_reply_attr(fuse_req_t req, const struct stat *attr,
-                    double attr_timeout)
+int fuse_reply_attr_with_flags(fuse_req_t req, const struct stat *attr,
+                               double attr_timeout, uint32_t attr_flags)
 {
     struct fuse_attr_out arg;
     size_t size = sizeof(arg);
@@ -437,9 +437,19 @@ int fuse_reply_attr(fuse_req_t req, const struct stat *attr,
     arg.attr_valid_nsec = calc_timeout_nsec(attr_timeout);
     convert_stat(attr, &arg.attr);
 
+    if (req->se->conn.capable & FUSE_CAP_ATTR_FLAGS) {
+        arg.attr.flags = attr_flags;
+    }
+
     return send_reply_ok(req, &arg, size);
 }
 
+int fuse_reply_attr(fuse_req_t req, const struct stat *attr,
+                    double attr_timeout)
+{
+    return fuse_reply_attr_with_flags(req, attr, attr_timeout, 0);
+}
+
 int fuse_reply_readlink(fuse_req_t req, const char *linkname)
 {
     return send_reply_ok(req, linkname, strlen(linkname));
-- 
2.26.2

[PATCH 5/8] virtiofsd: Store every lo_inode's parent_dev

[Max Reitz]

We want to detect mount points in the shared tree.  We report them to
the guest by setting the FUSE_ATTR_SUBMOUNT flag in fuse_attr.flags, but
because the FUSE client will create a submount for every directory that
has this flag set, we must do this only for the actual mount points.

We can detect mount points by comparing a directory's st_dev with its
parent's st_dev.  To be able to do so, we need to store the parent's
st_dev in the lo_inode object.

Note that mount points need not necessarily be directories; a single
file can be a mount point as well.  However, for the sake of simplicity
let us ignore any non-directory mount points for now.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 784330e0e4..2c48c03b4c 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -124,6 +124,14 @@ struct lo_inode {
     GHashTable *posix_locks; /* protected by lo_inode->plock_mutex */
 
     mode_t filetype;
+
+    /*
+     * So we can detect crossmount roots
+     * (As such, this only needs to be valid for directories.  Note
+     * that files can have multiple parents due to hard links, and so
+     * their parent_dev may fluctuate.)
+     */
+    dev_t parent_dev;
 };
 
 struct lo_cred {
@@ -821,6 +829,7 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
         g_hash_table_insert(lo->inodes, &inode->key, inode);
         pthread_mutex_unlock(&lo->mutex);
     }
+    inode->parent_dev = dir->key.dev;
     e->ino = inode->fuse_ino;
     lo_inode_put(lo, &inode);
     lo_inode_put(lo, &dir);
@@ -1047,6 +1056,14 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent,
     fuse_log(FUSE_LOG_DEBUG, "  %lli/%s -> %lli\n", (unsigned long long)parent,
              name, (unsigned long long)e.ino);
 
+    /*
+     * No need to update inode->parent_dev, because
+     * (1) We cannot, the inode now has more than one parent,
+     * (2) Directories cannot have more than one parent, so link()
+     *     does not work for them; but parent_dev only needs to be
+     *     valid for directories.
+     */
+
     fuse_reply_entry(req, &e);
     lo_inode_put(lo, &parent_inode);
     lo_inode_put(lo, &inode);
-- 
2.26.2

[PATCH 6/8] virtiofsd: Announce sub-mount points

[Max Reitz]

Whenever we encounter a directory with an st_dev that differs from that
of its parent, we set the FUSE_ATTR_SUBMOUNT flag so the guest can
create a submount for it.

Make this behavior 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.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tools/virtiofsd/helper.c         |  1 +
 tools/virtiofsd/passthrough_ll.c | 67 ++++++++++++++++++++++++++++----
 2 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/tools/virtiofsd/helper.c b/tools/virtiofsd/helper.c
index 7bc5d7dc5a..8c5b923604 100644
--- a/tools/virtiofsd/helper.c
+++ b/tools/virtiofsd/helper.c
@@ -178,6 +178,7 @@ void fuse_cmdline_help(void)
            "                               (0 leaves rlimit unchanged)\n"
            "                               default: min(1000000, fs.file-max - 16384)\n"
            "                                        if the current rlimit is lower\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 2c48c03b4c..fe7b9fbbb6 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>
@@ -159,6 +160,7 @@ struct lo_data {
     int timeout_set;
     int readdirplus_set;
     int readdirplus_clear;
+    int announce_submounts;
     struct lo_inode root;
     GHashTable *inodes; /* protected by lo->mutex */
     struct lo_map ino_map; /* protected by lo->mutex */
@@ -187,6 +189,7 @@ static const struct fuse_opt lo_opts[] = {
     { "cache=always", offsetof(struct lo_data, cache), CACHE_ALWAYS },
     { "readdirplus", offsetof(struct lo_data, readdirplus_set), 1 },
     { "no_readdirplus", offsetof(struct lo_data, readdirplus_clear), 1 },
+    { "announce_submounts", offsetof(struct lo_data, announce_submounts), 1 },
     FUSE_OPT_END
 };
 static bool use_syslog = false;
@@ -582,22 +585,52 @@ static void lo_init(void *userdata, struct fuse_conn_info *conn)
     }
 }
 
+/**
+ * Call fstatat() and set st_rdev whenever a directory's st_dev
+ * differs from the rparent's st_dev (@parent_dev).  This will
+ * announce submounts to the FUSE client (unless @announce_submounts
+ * is false).
+ */
+static int do_fstatat(int dirfd, const char *pathname, struct stat *statbuf,
+                      int flags, dev_t parent_dev, uint32_t *fuse_attr_flags)
+{
+    int res = fstatat(dirfd, pathname, statbuf, flags);
+    if (res == -1) {
+        return res;
+    }
+
+    if (statbuf->st_dev != parent_dev && S_ISDIR(statbuf->st_mode) &&
+        fuse_attr_flags)
+    {
+        *fuse_attr_flags |= FUSE_ATTR_SUBMOUNT;
+    }
+
+    return 0;
+}
+
 static void lo_getattr(fuse_req_t req, fuse_ino_t ino,
                        struct fuse_file_info *fi)
 {
     int res;
     struct stat buf;
     struct lo_data *lo = lo_data(req);
+    struct lo_inode *inode = lo_inode(req, ino);
+    uint32_t fuse_attr_flags = 0;
 
     (void)fi;
 
-    res =
-        fstatat(lo_fd(req, ino), "", &buf, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
+    res = do_fstatat(inode->fd, "", &buf, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW,
+                     inode->parent_dev, &fuse_attr_flags);
+    lo_inode_put(lo, &inode);
     if (res == -1) {
         return (void)fuse_reply_err(req, errno);
     }
 
-    fuse_reply_attr(req, &buf, lo->timeout);
+    if (!lo->announce_submounts) {
+        fuse_attr_flags &= ~FUSE_ATTR_SUBMOUNT;
+    }
+
+    fuse_reply_attr_with_flags(req, &buf, lo->timeout, fuse_attr_flags);
 }
 
 static int lo_fi_fd(fuse_req_t req, struct fuse_file_info *fi)
@@ -793,11 +826,16 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
         goto out_err;
     }
 
-    res = fstatat(newfd, "", &e->attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
+    res = do_fstatat(newfd, "", &e->attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW,
+                     dir->key.dev, &e->attr_flags);
     if (res == -1) {
         goto out_err;
     }
 
+    if (!lo->announce_submounts) {
+        e->attr_flags &= ~FUSE_ATTR_SUBMOUNT;
+    }
+
     inode = lo_find(lo, &e->attr);
     if (inode) {
         close(newfd);
@@ -1043,11 +1081,17 @@ static void lo_link(fuse_req_t req, fuse_ino_t ino, fuse_ino_t parent,
         goto out_err;
     }
 
-    res = fstatat(inode->fd, "", &e.attr, AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
+    res = do_fstatat(inode->fd, "", &e.attr,
+                     AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW,
+                     parent_inode->key.dev, &e.attr_flags);
     if (res == -1) {
         goto out_err;
     }
 
+    if (!lo->announce_submounts) {
+        e.attr_flags &= ~FUSE_ATTR_SUBMOUNT;
+    }
+
     pthread_mutex_lock(&lo->mutex);
     inode->nlookup++;
     pthread_mutex_unlock(&lo->mutex);
@@ -1082,14 +1126,21 @@ static struct lo_inode *lookup_name(fuse_req_t req, fuse_ino_t parent,
 {
     int res;
     struct stat attr;
+    struct lo_data *lo = lo_data(req);
+    struct lo_inode *dir = lo_inode(req, parent);
+
+    if (!dir) {
+        return NULL;
+    }
 
-    res = fstatat(lo_fd(req, parent), name, &attr,
-                  AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW);
+    res = do_fstatat(dir->fd, name, &attr,
+                     AT_EMPTY_PATH | AT_SYMLINK_NOFOLLOW, dir->key.dev, NULL);
+    lo_inode_put(lo, &dir);
     if (res == -1) {
         return NULL;
     }
 
-    return lo_find(lo_data(req), &attr);
+    return lo_find(lo, &attr);
 }
 
 static void lo_rmdir(fuse_req_t req, fuse_ino_t parent, const char *name)
-- 
2.26.2

[PATCH 7/8] tests/acceptance/boot_linux: Accept SSH pubkey

[Max Reitz]

Let download_cloudinit() take an optional pubkey, which subclasses of
BootLinux can pass through setUp().

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/acceptance/boot_linux.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
index 0055dc7cee..ad997c3f2e 100644
--- a/tests/acceptance/boot_linux.py
+++ b/tests/acceptance/boot_linux.py
@@ -57,7 +57,7 @@ class BootLinuxBase(Test):
             self.cancel('Failed to download/prepare boot image')
         return boot.path
 
-    def download_cloudinit(self):
+    def download_cloudinit(self, ssh_pubkey=None):
         self.log.info('Preparing cloudinit image')
         try:
             cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
@@ -67,7 +67,8 @@ class BootLinuxBase(Test):
                           password='password',
                           # QEMU's hard coded usermode router address
                           phone_home_host='10.0.2.2',
-                          phone_home_port=self.phone_home_port)
+                          phone_home_port=self.phone_home_port,
+                          authorized_key=ssh_pubkey)
         except Exception:
             self.cancel('Failed to prepared cloudinit image')
         return cloudinit_iso
@@ -80,19 +81,19 @@ class BootLinux(BootLinuxBase):
     timeout = 900
     chksum = None
 
-    def setUp(self):
+    def setUp(self, ssh_pubkey=None):
         super(BootLinux, self).setUp()
         self.vm.add_args('-smp', '2')
         self.vm.add_args('-m', '1024')
         self.prepare_boot()
-        self.prepare_cloudinit()
+        self.prepare_cloudinit(ssh_pubkey)
 
     def prepare_boot(self):
         path = self.download_boot()
         self.vm.add_args('-drive', 'file=%s' % path)
 
-    def prepare_cloudinit(self):
-        cloudinit_iso = self.download_cloudinit()
+    def prepare_cloudinit(self, ssh_pubkey=None):
+        cloudinit_iso = self.download_cloudinit(ssh_pubkey)
         self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
 
     def launch_and_wait(self):
-- 
2.26.2

[PATCH 8/8] tests/acceptance: Add virtiofs_submounts.py

[Max Reitz]

This test invokes several shell scripts to create a random directory
tree full of submounts, and then check in the VM whether every submount
has its own ID and the structure looks as expected.

(Note that the test scripts must be non-executable, so Avocado will not
try to execute them as if they were tests on their own, too.)

Because at this commit's date it is unlikely that the Linux kernel on
the image provided by boot_linux.py supports submounts in virtio-fs, the
test will be cancelled if no custom Linux binary is provided through the
vmlinuz parameter.  (The on-image kernel can be used by providing an
empty string via vmlinuz=.)

So, invoking the test can be done as follows:
$ avocado run \
    tests/acceptance/virtiofs_submounts.py \
    -p vmlinuz=/path/to/linux/build/arch/x86/boot/bzImage

This test requires root privileges (through passwordless sudo -n),
because at this point, virtiofsd requires them.  (If you have a
timestamp_timeout period for sudoers (e.g. the default of 5 min), you
can provide this by executing something like "sudo true" before invoking
Avocado.)

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/acceptance/virtiofs_submounts.py        | 289 ++++++++++++++++++
 .../virtiofs_submounts.py.data/cleanup.sh     |  46 +++
 .../guest-cleanup.sh                          |  30 ++
 .../virtiofs_submounts.py.data/guest.sh       | 138 +++++++++
 .../virtiofs_submounts.py.data/host.sh        | 127 ++++++++
 5 files changed, 630 insertions(+)
 create mode 100644 tests/acceptance/virtiofs_submounts.py
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/cleanup.sh
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/guest-cleanup.sh
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/guest.sh
 create mode 100644 tests/acceptance/virtiofs_submounts.py.data/host.sh

diff --git a/tests/acceptance/virtiofs_submounts.py b/tests/acceptance/virtiofs_submounts.py
new file mode 100644
index 0000000000..8b207b3e57
--- /dev/null
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -0,0 +1,289 @@
+import logging
+import re
+import os
+import subprocess
+import time
+
+from avocado import skipUnless
+from avocado_qemu import Test, BUILD_DIR
+from avocado_qemu import wait_for_console_pattern
+from avocado.utils import ssh
+
+from qemu.accel import kvm_available
+
+from boot_linux import BootLinux
+
+
+def run_cmd(args):
+    subp = subprocess.Popen(args,
+                            stdout=subprocess.PIPE,
+                            stderr=subprocess.PIPE,
+                            universal_newlines=True)
+    stdout, stderr = subp.communicate()
+    ret = subp.returncode
+
+    return (stdout, stderr, ret)
+
+def has_passwordless_sudo():
+    """
+    This function is for use in a @avocado.skipUnless decorator, e.g.:
+
+        @skipUnless(*has_passwordless_sudo())
+        def test_something_that_needs_sudo(self):
+            ...
+    """
+
+    _, stderr, exitcode = run_cmd(('sudo', '-n', 'true'))
+    if exitcode != 0:
+        return (False, f'Failed to use sudo -n: {stderr.strip()}')
+    else:
+        return (True, '')
+
+
+class VirtiofsSubmountsTest(BootLinux):
+    """
+    :avocado: tags=arch:x86_64
+    """
+
+    def get_portfwd(self):
+        port = None
+
+        res = self.vm.command('human-monitor-command',
+                              command_line='info usernet')
+        for line in res.split('\r\n'):
+            match = \
+                re.search(r'TCP.HOST_FORWARD.*127\.0\.0\.1\s*(\d+)\s+10\.',
+                          line)
+            if match is not None:
+                port = match[1]
+                break
+
+        self.assertIsNotNone(port)
+        self.log.debug('sshd listening on port: ' + port)
+        return port
+
+    def ssh_connect(self, username, keyfile):
+        self.ssh_logger = logging.getLogger('ssh')
+        port = self.get_portfwd()
+        self.ssh_session = ssh.Session('127.0.0.1', port=int(port),
+                                       user=username, key=keyfile)
+        for i in range(10):
+            try:
+                self.ssh_session.connect()
+                return
+            except:
+                time.sleep(4)
+                pass
+        self.fail('sshd timeout')
+
+    def ssh_command(self, command):
+        self.ssh_logger.info(command)
+        result = self.ssh_session.cmd(command)
+        stdout_lines = [line.rstrip() for line
+                        in result.stdout_text.splitlines()]
+        for line in stdout_lines:
+            self.ssh_logger.info(line)
+        stderr_lines = [line.rstrip() for line
+                        in result.stderr_text.splitlines()]
+        for line in stderr_lines:
+            self.ssh_logger.warning(line)
+
+        self.assertEqual(result.exit_status, 0,
+                         f'Guest command failed: {command}')
+        return stdout_lines, stderr_lines
+
+    def run(self, args, ignore_error=False):
+        stdout, stderr, ret = run_cmd(args)
+
+        if ret != 0:
+            cmdline = ' '.join(args)
+            if not ignore_error:
+                self.fail(f'{cmdline}: Returned {ret}: {stderr}')
+            else:
+                self.log.warn(f'{cmdline}: Returned {ret}: {stderr}')
+
+        return (stdout, stderr, ret)
+
+    def set_up_shared_dir(self):
+        atwd = os.getenv('AVOCADO_TEST_WORKDIR')
+        self.shared_dir = os.path.join(atwd, 'virtiofs-shared')
+
+        os.mkdir(self.shared_dir)
+
+        self.run(('cp', self.get_data('guest.sh'),
+                 os.path.join(self.shared_dir, 'check.sh')))
+
+        self.run(('cp', self.get_data('guest-cleanup.sh'),
+                 os.path.join(self.shared_dir, 'cleanup.sh')))
+
+    def set_up_virtiofs(self):
+        attmp = os.getenv('AVOCADO_TESTS_COMMON_TMPDIR')
+        self.vfsdsock = os.path.join(attmp, 'vfsdsock')
+
+        self.run(('sudo', '-n', 'rm', '-f', self.vfsdsock), ignore_error=True)
+
+        self.virtiofsd = \
+            subprocess.Popen(('sudo', '-n',
+                              'tools/virtiofsd/virtiofsd',
+                              f'--socket-path={self.vfsdsock}',
+                              '-o', f'source={self.shared_dir}',
+                              '-o', 'cache=always',
+                              '-o', 'xattr',
+                              '-o', 'announce_submounts',
+                              '-f'),
+                             stdout=subprocess.DEVNULL,
+                             stderr=subprocess.PIPE,
+                             universal_newlines=True)
+
+        while not os.path.exists(self.vfsdsock):
+            if self.virtiofsd.poll() is not None:
+                self.fail('virtiofsd exited prematurely: ' +
+                          self.virtiofsd.communicate()[1])
+            time.sleep(0.1)
+
+        self.run(('sudo', '-n', 'chmod', 'go+rw', self.vfsdsock))
+
+        self.vm.add_args('-chardev',
+                         f'socket,id=vfsdsock,path={self.vfsdsock}',
+                         '-device',
+                         'vhost-user-fs-pci,queue-size=1024,chardev=vfsdsock' \
+                             ',tag=host',
+                         '-object',
+                         'memory-backend-file,id=mem,size=1G,' \
+                             'mem-path=/dev/shm,share=on',
+                         '-numa',
+                         'node,memdev=mem')
+
+    def launch_vm(self):
+        self.launch_and_wait()
+        self.ssh_connect('root', self.ssh_key)
+
+    def set_up_nested_mounts(self):
+        scratch_dir = os.path.join(self.shared_dir, 'scratch')
+        try:
+            os.mkdir(scratch_dir)
+        except FileExistsError:
+            pass
+
+        args = ['bash', self.get_data('host.sh'), scratch_dir]
+        if self.seed:
+            args += [self.seed]
+
+        out, _, _ = self.run(args)
+        seed = re.search(r'^Seed: \d+', out)
+        self.log.info(seed[0])
+
+    def mount_in_guest(self):
+        self.ssh_command('mkdir -p /mnt/host')
+        self.ssh_command('mount -t virtiofs host /mnt/host')
+
+    def check_in_guest(self):
+        self.ssh_command('bash /mnt/host/check.sh /mnt/host/scratch/share')
+
+    def live_cleanup(self):
+        self.ssh_command('bash /mnt/host/cleanup.sh /mnt/host/scratch')
+
+        # It would be nice if the above was sufficient to make virtiofsd clear
+        # all references to the mounted directories (so they can be unmounted
+        # on the host), but unfortunately it is not.  To do so, we have to
+        # resort to a remount.
+        self.ssh_command('mount -o remount /mnt/host')
+
+        scratch_dir = os.path.join(self.shared_dir, 'scratch')
+        self.run(('bash', self.get_data('cleanup.sh'), scratch_dir))
+
+    @skipUnless(*has_passwordless_sudo())
+    def setUp(self):
+        vmlinuz = self.params.get('vmlinuz')
+        if vmlinuz is None:
+            self.cancel('vmlinuz parameter not set; you must point it to a '
+                        'Linux kernel binary to test (to run this test with ' \
+                        'the on-image kernel, set it to an empty string)')
+
+        self.seed = self.params.get('seed')
+
+        atwd = os.getenv('AVOCADO_TEST_WORKDIR')
+        self.ssh_key = os.path.join(atwd, 'id_ed25519')
+
+        self.run(('ssh-keygen', '-t', 'ed25519', '-f', self.ssh_key))
+
+        pubkey = open(self.ssh_key + '.pub').read()
+
+        super(VirtiofsSubmountsTest, self).setUp(pubkey)
+
+        if len(vmlinuz) > 0:
+            self.vm.add_args('-kernel', vmlinuz,
+                             '-append', 'console=ttyS0 root=/dev/sda1')
+
+        # Allow us to connect to SSH
+        self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22',
+                         '-device', 'e1000,netdev=vnet')
+
+        if not kvm_available(self.arch, self.qemu_bin):
+            self.cancel(KVM_NOT_AVAILABLE)
+        self.vm.add_args('-accel', 'kvm')
+
+    def tearDown(self):
+        try:
+            self.vm.shutdown()
+        except:
+            pass
+
+        scratch_dir = os.path.join(self.shared_dir, 'scratch')
+        self.run(('bash', self.get_data('cleanup.sh'), scratch_dir),
+                 ignore_error=True)
+
+    def test_pre_virtiofsd_set_up(self):
+        self.set_up_shared_dir()
+
+        self.set_up_nested_mounts()
+
+        self.set_up_virtiofs()
+        self.launch_vm()
+        self.mount_in_guest()
+        self.check_in_guest()
+
+    def test_pre_launch_set_up(self):
+        self.set_up_shared_dir()
+        self.set_up_virtiofs()
+
+        self.set_up_nested_mounts()
+
+        self.launch_vm()
+        self.mount_in_guest()
+        self.check_in_guest()
+
+    def test_post_launch_set_up(self):
+        self.set_up_shared_dir()
+        self.set_up_virtiofs()
+        self.launch_vm()
+
+        self.set_up_nested_mounts()
+
+        self.mount_in_guest()
+        self.check_in_guest()
+
+    def test_post_mount_set_up(self):
+        self.set_up_shared_dir()
+        self.set_up_virtiofs()
+        self.launch_vm()
+        self.mount_in_guest()
+
+        self.set_up_nested_mounts()
+
+        self.check_in_guest()
+
+    def test_two_runs(self):
+        self.set_up_shared_dir()
+
+        self.set_up_nested_mounts()
+
+        self.set_up_virtiofs()
+        self.launch_vm()
+        self.mount_in_guest()
+        self.check_in_guest()
+
+        self.live_cleanup()
+        self.set_up_nested_mounts()
+
+        self.check_in_guest()
diff --git a/tests/acceptance/virtiofs_submounts.py.data/cleanup.sh b/tests/acceptance/virtiofs_submounts.py.data/cleanup.sh
new file mode 100644
index 0000000000..2a6579a0fe
--- /dev/null
+++ b/tests/acceptance/virtiofs_submounts.py.data/cleanup.sh
@@ -0,0 +1,46 @@
+#!/bin/bash
+
+function print_usage()
+{
+    if [ -n "$2" ]; then
+        echo "Error: $2"
+        echo
+    fi
+    echo "Usage: $1 <scratch dir>"
+}
+
+scratch_dir=$1
+if [ -z "$scratch_dir" ]; then
+    print_usage "$0" 'Scratch dir not given' >&2
+    exit 1
+fi
+
+cd "$scratch_dir/share" || exit 1
+mps=(mnt*)
+mp_i=0
+for mp in "${mps[@]}"; do
+    mp_i=$((mp_i + 1))
+    printf "Unmounting %i/%i...\r" "$mp_i" "${#mps[@]}"
+
+    sudo umount -R "$mp"
+    rm -rf "$mp"
+done
+echo
+
+rm some-file
+cd ..
+rmdir share
+
+imgs=(fs*.img)
+img_i=0
+for img in "${imgs[@]}"; do
+    img_i=$((img_i + 1))
+    printf "Detaching and deleting %i/%i...\r" "$img_i" "${#imgs[@]}"
+
+    dev=$(losetup -j "$img" | sed -e 's/:.*//')
+    sudo losetup -d "$dev"
+    rm -f "$img"
+done
+echo
+
+echo 'Done.'
diff --git a/tests/acceptance/virtiofs_submounts.py.data/guest-cleanup.sh b/tests/acceptance/virtiofs_submounts.py.data/guest-cleanup.sh
new file mode 100644
index 0000000000..729cb2d1a5
--- /dev/null
+++ b/tests/acceptance/virtiofs_submounts.py.data/guest-cleanup.sh
@@ -0,0 +1,30 @@
+#!/bin/bash
+
+function print_usage()
+{
+    if [ -n "$2" ]; then
+        echo "Error: $2"
+        echo
+    fi
+    echo "Usage: $1 <scratch dir>"
+}
+
+scratch_dir=$1
+if [ -z "$scratch_dir" ]; then
+    print_usage "$0" 'Scratch dir not given' >&2
+    exit 1
+fi
+
+cd "$scratch_dir/share" || exit 1
+
+mps=(mnt*)
+mp_i=0
+for mp in "${mps[@]}"; do
+    mp_i=$((mp_i + 1))
+    printf "Unmounting %i/%i...\r" "$mp_i" "${#mps[@]}"
+
+    sudo umount -R "$mp"
+done
+echo
+
+echo 'Done.'
diff --git a/tests/acceptance/virtiofs_submounts.py.data/guest.sh b/tests/acceptance/virtiofs_submounts.py.data/guest.sh
new file mode 100644
index 0000000000..59ba40fde1
--- /dev/null
+++ b/tests/acceptance/virtiofs_submounts.py.data/guest.sh
@@ -0,0 +1,138 @@
+#!/bin/bash
+
+function print_usage()
+{
+    if [ -n "$2" ]; then
+        echo "Error: $2"
+        echo
+    fi
+    echo "Usage: $1 <shared dir>"
+    echo '(The shared directory is the "share" directory in the scratch' \
+         'directory)'
+}
+
+shared_dir=$1
+if [ -z "$shared_dir" ]; then
+    print_usage "$0" 'Shared dir not given' >&2
+    exit 1
+fi
+
+cd "$shared_dir"
+
+# FIXME: This should not be necessary, but it is.  In order for all
+# submounts to be proper mount points, we need to visit them.
+# (Before we visit them, they will not be auto-mounted, and so just
+# appear as normal directories, with the catch that their st_ino will
+# be the st_ino of the filesystem they host, while the st_dev will
+# still be the st_dev of the parent.)
+# `find` does not work, because it will refuse to touch the mount
+# points as long as they are not mounted; their st_dev being shared
+# with the parent and st_ino just being the root node's inode ID
+# will practically ensure that this node exists elsewhere on the
+# filesystem, and `find` is required to recognize loops and not to
+# follow them.
+# Thus, we have to manually visit all nodes first.
+
+mnt_i=0
+
+function recursively_visit()
+{
+    pushd "$1" >/dev/null
+    for entry in *; do
+        if [[ "$entry" == mnt* ]]; then
+            mnt_i=$((mnt_i + 1))
+            printf "Triggering auto-mount $mnt_i...\r"
+        fi
+
+        if [ -d "$entry" ]; then
+            recursively_visit "$entry"
+        fi
+    done
+    popd >/dev/null
+}
+
+recursively_visit .
+echo
+
+
+if [ -n "$(find -name not-mounted)" ]; then
+    echo "Error: not-mounted files visible on mount points:" >&2
+    find -name not-mounted >&2
+    exit 1
+fi
+
+if [ ! -f some-file -o "$(cat some-file)" != 'root' ]; then
+    echo "Error: Bad file in the share root" >&2
+    exit 1
+fi
+
+shopt -s nullglob
+
+function check_submounts()
+{
+    local base_path=$1
+
+    for mp in mnt*; do
+        printf "Checking submount %i...\r" "$((${#devs[@]} + 1))"
+
+        mp_i=$(echo "$mp" | sed -e 's/mnt//')
+        dev=$(stat -c '%D' "$mp")
+
+        if [ -n "${devs[mp_i]}" ]; then
+            echo "Error: $mp encountered twice" >&2
+            exit 1
+        fi
+        devs[mp_i]=$dev
+
+        pushd "$mp" >/dev/null
+        path="$base_path$mp"
+        while true; do
+            expected_content="$(printf '%s\n%s\n' "$mp_i" "$path")"
+            if [ ! -f some-file ]; then
+                echo "Error: $PWD/some-file does not exist" >&2
+                exit 1
+            fi
+
+            if [ "$(cat some-file)" != "$expected_content" ]; then
+                echo "Error: Bad content in $PWD/some-file:" >&2
+                echo '--- found ---'
+                cat some-file
+                echo '--- expected ---'
+                echo "$expected_content"
+                exit 1
+            fi
+            if [ "$(stat -c '%D' some-file)" != "$dev" ]; then
+                echo "Error: $PWD/some-file has the wrong device ID" >&2
+                exit 1
+            fi
+
+            if [ -d sub ]; then
+                if [ "$(stat -c '%D' sub)" != "$dev" ]; then
+                    echo "Error: $PWD/some-file has the wrong device ID" >&2
+                    exit 1
+                fi
+                cd sub
+                path="$path/sub"
+            else
+                if [ -n "$(echo mnt*)" ]; then
+                    check_submounts "$path/"
+                fi
+                break
+            fi
+        done
+        popd >/dev/null
+    done
+}
+
+root_dev=$(stat -c '%D' some-file)
+devs=()
+check_submounts ''
+echo
+
+reused_devs=$(echo "$root_dev ${devs[@]}" | tr ' ' '\n' | sort | uniq -d)
+if [ -n "$reused_devs" ]; then
+    echo "Error: Reused device IDs: $reused_devs" >&2
+    exit 1
+fi
+
+echo "Test passed for ${#devs[@]} submounts."
diff --git a/tests/acceptance/virtiofs_submounts.py.data/host.sh b/tests/acceptance/virtiofs_submounts.py.data/host.sh
new file mode 100644
index 0000000000..d8a9afebdb
--- /dev/null
+++ b/tests/acceptance/virtiofs_submounts.py.data/host.sh
@@ -0,0 +1,127 @@
+#!/bin/bash
+
+mount_count=128
+
+function print_usage()
+{
+    if [ -n "$2" ]; then
+        echo "Error: $2"
+        echo
+    fi
+    echo "Usage: $1 <scratch dir> [seed]"
+    echo "(If no seed is given, it will be randomly generated.)"
+}
+
+scratch_dir=$1
+if [ -z "$scratch_dir" ]; then
+    print_usage "$0" 'No scratch dir given' >&2
+    exit 1
+fi
+
+if [ ! -d "$scratch_dir" ]; then
+    print_usage "$0" "$scratch_dir is not a directory" >&2
+    exit 1
+fi
+
+seed=$2
+if [ -z "$seed" ]; then
+    seed=$RANDOM
+fi
+RANDOM=$seed
+
+echo "Seed: $seed"
+
+set -e
+shopt -s nullglob
+
+cd "$scratch_dir"
+if [ -d share ]; then
+    echo 'Error: This directory seems to be in use already' >&2
+    exit 1
+fi
+
+for ((i = 0; i < $mount_count; i++)); do
+    printf "Setting up fs %i/%i...\r" "$((i + 1))" "$mount_count"
+
+    rm -f fs$i.img
+    truncate -s 512M fs$i.img
+    mkfs.xfs -q fs$i.img
+    devs[i]=$(sudo losetup -f --show fs$i.img)
+done
+echo
+
+top_level_mounts=$((RANDOM % mount_count + 1))
+
+mkdir -p share
+echo 'root' > share/some-file
+
+for ((i = 0; i < $top_level_mounts; i++)); do
+    printf "Mounting fs %i/%i...\r" "$((i + 1))" "$mount_count"
+
+    mkdir -p share/mnt$i
+    touch share/mnt$i/not-mounted
+    sudo mount "${devs[i]}" share/mnt$i
+    sudo chown "$(id -u):$(id -g)" share/mnt$i
+
+    pushd share/mnt$i >/dev/null
+    path=mnt$i
+    nesting=$((RANDOM % 4))
+    for ((j = 0; j < $nesting; j++)); do
+        cat > some-file <<EOF
+$i
+$path
+EOF
+        mkdir sub
+        cd sub
+        path="$path/sub"
+    done
+cat > some-file <<EOF
+$i
+$path
+EOF
+    popd >/dev/null
+done
+
+for ((; i < $mount_count; i++)); do
+    printf "Mounting fs %i/%i...\r" "$((i + 1))" "$mount_count"
+
+    mp_i=$((i % top_level_mounts))
+
+    pushd share/mnt$mp_i >/dev/null
+    path=mnt$mp_i
+    while true; do
+        sub_mp="$(echo mnt*)"
+        if cd sub 2>/dev/null; then
+            path="$path/sub"
+        elif [ -n "$sub_mp" ] && cd "$sub_mp" 2>/dev/null; then
+            path="$path/$sub_mp"
+        else
+            break
+        fi
+    done
+    mkdir mnt$i
+    touch mnt$i/not-mounted
+    sudo mount "${devs[i]}" mnt$i
+    sudo chown "$(id -u):$(id -g)" mnt$i
+
+    cd mnt$i
+    path="$path/mnt$i"
+    nesting=$((RANDOM % 4))
+    for ((j = 0; j < $nesting; j++)); do
+        cat > some-file <<EOF
+$i
+$path
+EOF
+        mkdir sub
+        cd sub
+        path="$path/sub"
+    done
+    cat > some-file <<EOF
+$i
+$path
+EOF
+    popd >/dev/null
+done
+echo
+
+echo 'Done.'
-- 
2.26.2

Re: [PATCH 7/8] tests/acceptance/boot_linux: Accept SSH pubkey

[Philippe Mathieu-Daudé]

On 9/9/20 8:40 PM, Max Reitz wrote:
> Let download_cloudinit() take an optional pubkey, which subclasses of
> BootLinux can pass through setUp().
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  tests/acceptance/boot_linux.py | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
> index 0055dc7cee..ad997c3f2e 100644
> --- a/tests/acceptance/boot_linux.py
> +++ b/tests/acceptance/boot_linux.py
> @@ -57,7 +57,7 @@ class BootLinuxBase(Test):
>              self.cancel('Failed to download/prepare boot image')
>          return boot.path
>  
> -    def download_cloudinit(self):
> +    def download_cloudinit(self, ssh_pubkey=None):
>          self.log.info('Preparing cloudinit image')
>          try:
>              cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
> @@ -67,7 +67,8 @@ class BootLinuxBase(Test):
>                            password='password',
>                            # QEMU's hard coded usermode router address
>                            phone_home_host='10.0.2.2',
> -                          phone_home_port=self.phone_home_port)
> +                          phone_home_port=self.phone_home_port,
> +                          authorized_key=ssh_pubkey)
>          except Exception:
>              self.cancel('Failed to prepared cloudinit image')
>          return cloudinit_iso
> @@ -80,19 +81,19 @@ class BootLinux(BootLinuxBase):
>      timeout = 900
>      chksum = None
>  
> -    def setUp(self):
> +    def setUp(self, ssh_pubkey=None):
>          super(BootLinux, self).setUp()
>          self.vm.add_args('-smp', '2')
>          self.vm.add_args('-m', '1024')
>          self.prepare_boot()
> -        self.prepare_cloudinit()
> +        self.prepare_cloudinit(ssh_pubkey)
>  
>      def prepare_boot(self):
>          path = self.download_boot()
>          self.vm.add_args('-drive', 'file=%s' % path)
>  
> -    def prepare_cloudinit(self):
> -        cloudinit_iso = self.download_cloudinit()
> +    def prepare_cloudinit(self, ssh_pubkey=None):
> +        cloudinit_iso = self.download_cloudinit(ssh_pubkey)
>          self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
>  
>      def launch_and_wait(self):
> 

Re: [PATCH 0/8] virtiofsd: Announce submounts to the guest

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 451 bytes --]

On Wed, Sep 09, 2020 at 08:40:20PM +0200, Max Reitz wrote:
> We want to (be able to) announce the host mount structure of the shared
> directory to the guest so it can replicate that structure.  This ensures
> that whenever the combination of st_dev and st_ino is unique on the
> host, it will be unique in the guest as well.

Great, thank you for solving the long-standing inode collision problem!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/8] tests/acceptance/boot_linux: Accept SSH pubkey

[Willian Rampazzo]

On Wed, Sep 9, 2020 at 3:46 PM Max Reitz <mreitz@redhat.com> wrote:
>
> Let download_cloudinit() take an optional pubkey, which subclasses of
> BootLinux can pass through setUp().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/acceptance/boot_linux.py | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
> index 0055dc7cee..ad997c3f2e 100644
> --- a/tests/acceptance/boot_linux.py
> +++ b/tests/acceptance/boot_linux.py
> @@ -57,7 +57,7 @@ class BootLinuxBase(Test):
>              self.cancel('Failed to download/prepare boot image')
>          return boot.path
>
> -    def download_cloudinit(self):
> +    def download_cloudinit(self, ssh_pubkey=None):
>          self.log.info('Preparing cloudinit image')
>          try:
>              cloudinit_iso = os.path.join(self.workdir, 'cloudinit.iso')
> @@ -67,7 +67,8 @@ class BootLinuxBase(Test):
>                            password='password',
>                            # QEMU's hard coded usermode router address
>                            phone_home_host='10.0.2.2',
> -                          phone_home_port=self.phone_home_port)
> +                          phone_home_port=self.phone_home_port,
> +                          authorized_key=ssh_pubkey)
>          except Exception:
>              self.cancel('Failed to prepared cloudinit image')
>          return cloudinit_iso
> @@ -80,19 +81,19 @@ class BootLinux(BootLinuxBase):
>      timeout = 900
>      chksum = None
>
> -    def setUp(self):
> +    def setUp(self, ssh_pubkey=None):
>          super(BootLinux, self).setUp()
>          self.vm.add_args('-smp', '2')
>          self.vm.add_args('-m', '1024')
>          self.prepare_boot()
> -        self.prepare_cloudinit()
> +        self.prepare_cloudinit(ssh_pubkey)
>
>      def prepare_boot(self):
>          path = self.download_boot()
>          self.vm.add_args('-drive', 'file=%s' % path)
>
> -    def prepare_cloudinit(self):
> -        cloudinit_iso = self.download_cloudinit()
> +    def prepare_cloudinit(self, ssh_pubkey=None):
> +        cloudinit_iso = self.download_cloudinit(ssh_pubkey)
>          self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
>
>      def launch_and_wait(self):
> --
> 2.26.2
>
>

Reviewed-by: WIllian Rampazzo <willianr@redhat.com>

Re: [PATCH 0/8] virtiofsd: Announce submounts to the guest

[Dr. David Alan Gilbert]

* Max Reitz (mreitz@redhat.com) wrote:
> RFC: https://www.redhat.com/archives/virtio-fs/2020-May/msg00024.html
> 
> Branch: https://github.com/XanClic/qemu.git virtiofs-submounts-v2
> Branch: https://git.xanclic.moe/XanClic/qemu.git virtiofs-submounts-v2

Queued

> 
> 
> (Note that there is an accompanying Linux (kernel) series
> “fuse: Mirror virtio-fs submounts”.)
> 
> 
> Hi,
> 
> We want to (be able to) announce the host mount structure of the shared
> directory to the guest so it can replicate that structure.  This ensures
> that whenever the combination of st_dev and st_ino is unique on the
> host, it will be unique in the guest as well.
> 
> This feature is optional and needs to be enabled explicitly, so that the
> mount structure isn’t leaked to the guest if the user doesn’t want it to
> be.
> 
> The last patch in this series adds a test script.  For it to pass, you
> need to compile a kernel with the accompanying “fuse: Mirror virtio-fs
> submounts” patch series, and provide it to the test (as described in the
> test patch).
> 
> 
> Known caveats:
> - stat(2) doesn’t trigger auto-mounting.  Therefore, issuing a stat() on
>   a sub-mountpoint before it’s been auto-mounted will show its parent’s
>   st_dev together with the st_ino it has in the sub-mounted filesystem.
> 
>   For example, imagine you want to share a whole filesystem with the
>   guest, which on the host first looks like this:
> 
>     root/           (st_dev=64, st_ino=128)
>       sub_fs/       (st_dev=64, st_ino=234)
> 
>   And then you mount another filesystem under sub_fs, so it looks like
>   this:
> 
>     root/           (st_dev=64, st_ino=128)
>       sub_fs/       (st_dev=96, st_ino=128)
>         ...
> 
>   As you can see, sub_fs becomes a mount point, so its st_dev and st_ino
>   change from what they were on root’s filesystem to what they are in
>   the sub-filesystem.  In fact, root and sub_fs now have the same
>   st_ino, which is not unlikely given that both are root nodes in their
>   respective filesystems.
> 
>   Now, this filesystem is shared with the guest through virtiofsd.
>   There is no way for virtiofsd to uncover sub_fs’s original st_ino
>   value of 234, so it will always provide st_ino=128 to the guest.
>   However, virtiofsd does notice that sub_fs is a mount point and
>   announces this fact to the guest.
> 
>   We want this to result in something like the following tree in the
>   guest:
> 
>     root/           (st_dev=32, st_ino=128)
>       sub_fs/       (st_dev=33, st_ino=128)
>         ...
> 
>   That is, sub_fs should be a different filesystem that’s auto-mounted.
>   However, as stated above, stat(2) doesn’t trigger auto-mounting, so
>   before it happens, the following structure will be visible:
> 
>     root/           (st_dev=32, st_ino=128)
>       sub_fs/       (st_dev=32, st_ino=128)
> 
>   That is, sub_fs and root will have the same st_dev/st_ino combination.
> 
>   This can easily be seen by executing find(1) on root in the guest,
>   which will subsequently complain about an alleged filesystem loop.
> 
>   To properly fix this problem, we probably would have to be able to
>   uncover sub_fs’s original st_ino value (i.e. 234) and let the guest
>   use that until the auto-mount happens.  However, there is no way to
>   get that value (from userspace at least).
> 
>   Note that NFS with crossmnt has the exact same issue.
> 
> 
> - You can unmount auto-mounted submounts in the guest, but then you
>   still cannot unmount them on the host.  The guest still holds a
>   reference to the submount’s root directory, because that’s just a
>   normal entry in its parent directory (on the submount’s parent
>   filesystem).
> 
>   This is kind of related to the issue noted above: When the submount is
>   unmounted, the guest shouldn’t have a reference to sub_fs as the
>   submount’s root directory (host’s st_dev=96, st_ino=128), but to it as
>   a normal entry in its parent filesystem (st_dev=64, st_ino=234).
> 
>   (When you have multiple nesting levels, you can unmount inner mounts
>   when the outer ones have been unmounted in the guest.  For example,
>   say you have a structure A/B/C/D, where each is a mount point, then
>   unmounting D, C, and B in the guest will allow the host to unmount D
>   and C.)
> 
> 
> Max Reitz (8):
>   linux/fuse.h: Pull in from Linux
>   virtiofsd: Announce FUSE_ATTR_FLAGS
>   virtiofsd: Add attr_flags to fuse_entry_param
>   virtiofsd: Add fuse_reply_attr_with_flags()
>   virtiofsd: Store every lo_inode's parent_dev
>   virtiofsd: Announce sub-mount points
>   tests/acceptance/boot_linux: Accept SSH pubkey
>   tests/acceptance: Add virtiofs_submounts.py
> 
>  include/standard-headers/linux/fuse.h         |  11 +-
>  tools/virtiofsd/fuse_common.h                 |   8 +
>  tools/virtiofsd/fuse_lowlevel.h               |  20 ++
>  tools/virtiofsd/fuse_lowlevel.c               |  34 ++-
>  tools/virtiofsd/helper.c                      |   1 +
>  tools/virtiofsd/passthrough_ll.c              |  84 ++++-
>  tests/acceptance/boot_linux.py                |  13 +-
>  tests/acceptance/virtiofs_submounts.py        | 289 ++++++++++++++++++
>  .../virtiofs_submounts.py.data/cleanup.sh     |  46 +++
>  .../guest-cleanup.sh                          |  30 ++
>  .../virtiofs_submounts.py.data/guest.sh       | 138 +++++++++
>  .../virtiofs_submounts.py.data/host.sh        | 127 ++++++++
>  12 files changed, 780 insertions(+), 21 deletions(-)
>  create mode 100644 tests/acceptance/virtiofs_submounts.py
>  create mode 100644 tests/acceptance/virtiofs_submounts.py.data/cleanup.sh
>  create mode 100644 tests/acceptance/virtiofs_submounts.py.data/guest-cleanup.sh
>  create mode 100644 tests/acceptance/virtiofs_submounts.py.data/guest.sh
>  create mode 100644 tests/acceptance/virtiofs_submounts.py.data/host.sh
> 
> -- 
> 2.26.2
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK