* [PATCH v6 0/1] virtiofsd: Add support for FUSE_SYNCFS request
@ 2022-02-15 18:15 ` Greg Kurz
0 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2022-02-15 18:15 UTC (permalink / raw)
To: qemu-devel
Cc: Sebastian Hasler, Greg Kurz, Dr. David Alan Gilbert, virtio-fs,
Stefan Hajnoczi, Vivek Goyal
FUSE_SYNCFS allows the client to flush the host page cache.
v6: - drop ! announce_submounts part for now, only the root inode is
synced in this case
v5: - split announce_submounts and ! announce_submounts to separate patches
for easier review (but could be squashed together)
- track submounts directly in lo_inode structure following the same
logic as FUSE_ATTR_SUBMOUNT (i.e. directory with either st_dev or
stx_mnt_id different from parent)
v4: - based on upstream linux FUSE_SYNCFS
- added support for the '-o announce_submounts' case, i.e. client sends
a FUSE_SYNCFS request for each submount (identified by its root inode)
- adapted the case without '-o announce_submounts' so that syncfs() is
no longer called with lo->mutex held
v3: - track submounts and do per-submount syncfs() (Vivek)
- based on new version of FUSE_SYNCFS (still not upstream)
https://listman.redhat.com/archives/virtio-fs/2021-May/msg00025.html
v2: - based on new version of FUSE_SYNCFS
https://listman.redhat.com/archives/virtio-fs/2021-April/msg00166.html
- propagate syncfs() errors to client (Vivek)
Greg Kurz (1):
virtiofsd: Add basic support for FUSE_SYNCFS request
tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
tools/virtiofsd/passthrough_seccomp.c | 1 +
4 files changed, 69 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Virtio-fs] [PATCH v6 0/1] virtiofsd: Add support for FUSE_SYNCFS request
@ 2022-02-15 18:15 ` Greg Kurz
0 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2022-02-15 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: virtio-fs, Vivek Goyal
FUSE_SYNCFS allows the client to flush the host page cache.
v6: - drop ! announce_submounts part for now, only the root inode is
synced in this case
v5: - split announce_submounts and ! announce_submounts to separate patches
for easier review (but could be squashed together)
- track submounts directly in lo_inode structure following the same
logic as FUSE_ATTR_SUBMOUNT (i.e. directory with either st_dev or
stx_mnt_id different from parent)
v4: - based on upstream linux FUSE_SYNCFS
- added support for the '-o announce_submounts' case, i.e. client sends
a FUSE_SYNCFS request for each submount (identified by its root inode)
- adapted the case without '-o announce_submounts' so that syncfs() is
no longer called with lo->mutex held
v3: - track submounts and do per-submount syncfs() (Vivek)
- based on new version of FUSE_SYNCFS (still not upstream)
https://listman.redhat.com/archives/virtio-fs/2021-May/msg00025.html
v2: - based on new version of FUSE_SYNCFS
https://listman.redhat.com/archives/virtio-fs/2021-April/msg00166.html
- propagate syncfs() errors to client (Vivek)
Greg Kurz (1):
virtiofsd: Add basic support for FUSE_SYNCFS request
tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
tools/virtiofsd/passthrough_seccomp.c | 1 +
4 files changed, 69 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v6 1/1] virtiofsd: Add basic support for FUSE_SYNCFS request
2022-02-15 18:15 ` [Virtio-fs] " Greg Kurz
@ 2022-02-15 18:15 ` Greg Kurz
-1 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2022-02-15 18:15 UTC (permalink / raw)
To: qemu-devel
Cc: Sebastian Hasler, Greg Kurz, Dr. David Alan Gilbert, virtio-fs,
Stefan Hajnoczi, Vivek Goyal
Honor the expected behavior of syncfs() to synchronously flush all data
and metadata to disk on linux systems.
If virtiofsd is started with '-o announce_submounts', the client is
expected to send a FUSE_SYNCFS request for each individual submount.
In this case, we just create a new file descriptor on the submount
inode with lo_inode_open(), call syncfs() on it and close it. The
intermediary file is needed because O_PATH descriptors aren't
backed by an actual file and syncfs() would fail with EBADF.
If virtiofsd is started without '-o announce_submounts' or if the
client doesn't have the FUSE_CAP_SUBMOUNTS capability, the client
only sends a single FUSE_SYNCFS request for the root inode. The
server would thus need to track submounts internally and call
syncfs() on each of them. This will be implemented later.
Note that syncfs() might suffer from a time penalty if the submounts
are being hammered by some unrelated workload on the host. The only
solution to prevent that is to avoid shared mounts.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
tools/virtiofsd/passthrough_seccomp.c | 1 +
4 files changed, 69 insertions(+)
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index e4679c73abc2..e02d8b25a5f6 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -1876,6 +1876,16 @@ static void do_lseek(fuse_req_t req, fuse_ino_t nodeid,
}
}
+static void do_syncfs(fuse_req_t req, fuse_ino_t nodeid,
+ struct fuse_mbuf_iter *iter)
+{
+ if (req->se->op.syncfs) {
+ req->se->op.syncfs(req, nodeid);
+ } else {
+ fuse_reply_err(req, ENOSYS);
+ }
+}
+
static void do_init(fuse_req_t req, fuse_ino_t nodeid,
struct fuse_mbuf_iter *iter)
{
@@ -2280,6 +2290,7 @@ static struct {
[FUSE_RENAME2] = { do_rename2, "RENAME2" },
[FUSE_COPY_FILE_RANGE] = { do_copy_file_range, "COPY_FILE_RANGE" },
[FUSE_LSEEK] = { do_lseek, "LSEEK" },
+ [FUSE_SYNCFS] = { do_syncfs, "SYNCFS" },
};
#define FUSE_MAXOP (sizeof(fuse_ll_ops) / sizeof(fuse_ll_ops[0]))
diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
index c55c0ca2fc1c..b889dae4de0e 100644
--- a/tools/virtiofsd/fuse_lowlevel.h
+++ b/tools/virtiofsd/fuse_lowlevel.h
@@ -1226,6 +1226,19 @@ struct fuse_lowlevel_ops {
*/
void (*lseek)(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
struct fuse_file_info *fi);
+
+ /**
+ * Synchronize file system content
+ *
+ * If this request is answered with an error code of ENOSYS,
+ * this is treated as success and future calls to syncfs() will
+ * succeed automatically without being sent to the filesystem
+ * process.
+ *
+ * @param req request handle
+ * @param ino the inode number
+ */
+ void (*syncfs)(fuse_req_t req, fuse_ino_t ino);
};
/**
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index b3d0674f6d2f..0f65e6423cf5 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3357,6 +3357,49 @@ static void lo_lseek(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
}
}
+static int lo_do_syncfs(struct lo_data *lo, struct lo_inode *inode)
+{
+ int fd, ret = 0;
+
+ fuse_log(FUSE_LOG_DEBUG, "lo_do_syncfs(ino=%" PRIu64 ")\n",
+ inode->fuse_ino);
+
+ fd = lo_inode_open(lo, inode, O_RDONLY);
+ if (fd < 0) {
+ return -fd;
+ }
+
+ if (syncfs(fd) < 0) {
+ ret = errno;
+ }
+
+ close(fd);
+ return ret;
+}
+
+static void lo_syncfs(fuse_req_t req, fuse_ino_t ino)
+{
+ struct lo_data *lo = lo_data(req);
+ struct lo_inode *inode = lo_inode(req, ino);
+ int err;
+
+ if (!inode) {
+ fuse_reply_err(req, EBADF);
+ return;
+ }
+
+ err = lo_do_syncfs(lo, inode);
+ lo_inode_put(lo, &inode);
+
+ /*
+ * If submounts aren't announced, the client only sends a request to
+ * sync the root inode. TODO: Track submounts internally and iterate
+ * over them as well.
+ */
+
+ fuse_reply_err(req, err);
+}
+
static void lo_destroy(void *userdata)
{
struct lo_data *lo = (struct lo_data *)userdata;
@@ -3417,6 +3460,7 @@ static struct fuse_lowlevel_ops lo_oper = {
.copy_file_range = lo_copy_file_range,
#endif
.lseek = lo_lseek,
+ .syncfs = lo_syncfs,
.destroy = lo_destroy,
};
diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c
index a3ce9f898d2d..3e9d6181dc69 100644
--- a/tools/virtiofsd/passthrough_seccomp.c
+++ b/tools/virtiofsd/passthrough_seccomp.c
@@ -108,6 +108,7 @@ static const int syscall_allowlist[] = {
SCMP_SYS(set_robust_list),
SCMP_SYS(setxattr),
SCMP_SYS(symlinkat),
+ SCMP_SYS(syncfs),
SCMP_SYS(time), /* Rarely needed, except on static builds */
SCMP_SYS(tgkill),
SCMP_SYS(unlinkat),
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [Virtio-fs] [PATCH v6 1/1] virtiofsd: Add basic support for FUSE_SYNCFS request
@ 2022-02-15 18:15 ` Greg Kurz
0 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2022-02-15 18:15 UTC (permalink / raw)
To: qemu-devel; +Cc: virtio-fs, Vivek Goyal
Honor the expected behavior of syncfs() to synchronously flush all data
and metadata to disk on linux systems.
If virtiofsd is started with '-o announce_submounts', the client is
expected to send a FUSE_SYNCFS request for each individual submount.
In this case, we just create a new file descriptor on the submount
inode with lo_inode_open(), call syncfs() on it and close it. The
intermediary file is needed because O_PATH descriptors aren't
backed by an actual file and syncfs() would fail with EBADF.
If virtiofsd is started without '-o announce_submounts' or if the
client doesn't have the FUSE_CAP_SUBMOUNTS capability, the client
only sends a single FUSE_SYNCFS request for the root inode. The
server would thus need to track submounts internally and call
syncfs() on each of them. This will be implemented later.
Note that syncfs() might suffer from a time penalty if the submounts
are being hammered by some unrelated workload on the host. The only
solution to prevent that is to avoid shared mounts.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
tools/virtiofsd/passthrough_seccomp.c | 1 +
4 files changed, 69 insertions(+)
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index e4679c73abc2..e02d8b25a5f6 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -1876,6 +1876,16 @@ static void do_lseek(fuse_req_t req, fuse_ino_t nodeid,
}
}
+static void do_syncfs(fuse_req_t req, fuse_ino_t nodeid,
+ struct fuse_mbuf_iter *iter)
+{
+ if (req->se->op.syncfs) {
+ req->se->op.syncfs(req, nodeid);
+ } else {
+ fuse_reply_err(req, ENOSYS);
+ }
+}
+
static void do_init(fuse_req_t req, fuse_ino_t nodeid,
struct fuse_mbuf_iter *iter)
{
@@ -2280,6 +2290,7 @@ static struct {
[FUSE_RENAME2] = { do_rename2, "RENAME2" },
[FUSE_COPY_FILE_RANGE] = { do_copy_file_range, "COPY_FILE_RANGE" },
[FUSE_LSEEK] = { do_lseek, "LSEEK" },
+ [FUSE_SYNCFS] = { do_syncfs, "SYNCFS" },
};
#define FUSE_MAXOP (sizeof(fuse_ll_ops) / sizeof(fuse_ll_ops[0]))
diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
index c55c0ca2fc1c..b889dae4de0e 100644
--- a/tools/virtiofsd/fuse_lowlevel.h
+++ b/tools/virtiofsd/fuse_lowlevel.h
@@ -1226,6 +1226,19 @@ struct fuse_lowlevel_ops {
*/
void (*lseek)(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
struct fuse_file_info *fi);
+
+ /**
+ * Synchronize file system content
+ *
+ * If this request is answered with an error code of ENOSYS,
+ * this is treated as success and future calls to syncfs() will
+ * succeed automatically without being sent to the filesystem
+ * process.
+ *
+ * @param req request handle
+ * @param ino the inode number
+ */
+ void (*syncfs)(fuse_req_t req, fuse_ino_t ino);
};
/**
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index b3d0674f6d2f..0f65e6423cf5 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3357,6 +3357,49 @@ static void lo_lseek(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
}
}
+static int lo_do_syncfs(struct lo_data *lo, struct lo_inode *inode)
+{
+ int fd, ret = 0;
+
+ fuse_log(FUSE_LOG_DEBUG, "lo_do_syncfs(ino=%" PRIu64 ")\n",
+ inode->fuse_ino);
+
+ fd = lo_inode_open(lo, inode, O_RDONLY);
+ if (fd < 0) {
+ return -fd;
+ }
+
+ if (syncfs(fd) < 0) {
+ ret = errno;
+ }
+
+ close(fd);
+ return ret;
+}
+
+static void lo_syncfs(fuse_req_t req, fuse_ino_t ino)
+{
+ struct lo_data *lo = lo_data(req);
+ struct lo_inode *inode = lo_inode(req, ino);
+ int err;
+
+ if (!inode) {
+ fuse_reply_err(req, EBADF);
+ return;
+ }
+
+ err = lo_do_syncfs(lo, inode);
+ lo_inode_put(lo, &inode);
+
+ /*
+ * If submounts aren't announced, the client only sends a request to
+ * sync the root inode. TODO: Track submounts internally and iterate
+ * over them as well.
+ */
+
+ fuse_reply_err(req, err);
+}
+
static void lo_destroy(void *userdata)
{
struct lo_data *lo = (struct lo_data *)userdata;
@@ -3417,6 +3460,7 @@ static struct fuse_lowlevel_ops lo_oper = {
.copy_file_range = lo_copy_file_range,
#endif
.lseek = lo_lseek,
+ .syncfs = lo_syncfs,
.destroy = lo_destroy,
};
diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c
index a3ce9f898d2d..3e9d6181dc69 100644
--- a/tools/virtiofsd/passthrough_seccomp.c
+++ b/tools/virtiofsd/passthrough_seccomp.c
@@ -108,6 +108,7 @@ static const int syscall_allowlist[] = {
SCMP_SYS(set_robust_list),
SCMP_SYS(setxattr),
SCMP_SYS(symlinkat),
+ SCMP_SYS(syncfs),
SCMP_SYS(time), /* Rarely needed, except on static builds */
SCMP_SYS(tgkill),
SCMP_SYS(unlinkat),
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v6 1/1] virtiofsd: Add basic support for FUSE_SYNCFS request
2022-02-15 18:15 ` [Virtio-fs] " Greg Kurz
@ 2022-02-15 19:06 ` Vivek Goyal
-1 siblings, 0 replies; 8+ messages in thread
From: Vivek Goyal @ 2022-02-15 19:06 UTC (permalink / raw)
To: Greg Kurz
Cc: virtio-fs, Sebastian Hasler, qemu-devel, Stefan Hajnoczi,
Dr. David Alan Gilbert
On Tue, Feb 15, 2022 at 07:15:29PM +0100, Greg Kurz wrote:
> Honor the expected behavior of syncfs() to synchronously flush all data
> and metadata to disk on linux systems.
>
> If virtiofsd is started with '-o announce_submounts', the client is
> expected to send a FUSE_SYNCFS request for each individual submount.
> In this case, we just create a new file descriptor on the submount
> inode with lo_inode_open(), call syncfs() on it and close it. The
> intermediary file is needed because O_PATH descriptors aren't
> backed by an actual file and syncfs() would fail with EBADF.
>
> If virtiofsd is started without '-o announce_submounts' or if the
> client doesn't have the FUSE_CAP_SUBMOUNTS capability, the client
> only sends a single FUSE_SYNCFS request for the root inode. The
> server would thus need to track submounts internally and call
> syncfs() on each of them. This will be implemented later.
>
> Note that syncfs() might suffer from a time penalty if the submounts
> are being hammered by some unrelated workload on the host. The only
> solution to prevent that is to avoid shared mounts.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Looks good to me. Thanks Greg.
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Vivek
> ---
> tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
> tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
> tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
> tools/virtiofsd/passthrough_seccomp.c | 1 +
> 4 files changed, 69 insertions(+)
>
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index e4679c73abc2..e02d8b25a5f6 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -1876,6 +1876,16 @@ static void do_lseek(fuse_req_t req, fuse_ino_t nodeid,
> }
> }
>
> +static void do_syncfs(fuse_req_t req, fuse_ino_t nodeid,
> + struct fuse_mbuf_iter *iter)
> +{
> + if (req->se->op.syncfs) {
> + req->se->op.syncfs(req, nodeid);
> + } else {
> + fuse_reply_err(req, ENOSYS);
> + }
> +}
> +
> static void do_init(fuse_req_t req, fuse_ino_t nodeid,
> struct fuse_mbuf_iter *iter)
> {
> @@ -2280,6 +2290,7 @@ static struct {
> [FUSE_RENAME2] = { do_rename2, "RENAME2" },
> [FUSE_COPY_FILE_RANGE] = { do_copy_file_range, "COPY_FILE_RANGE" },
> [FUSE_LSEEK] = { do_lseek, "LSEEK" },
> + [FUSE_SYNCFS] = { do_syncfs, "SYNCFS" },
> };
>
> #define FUSE_MAXOP (sizeof(fuse_ll_ops) / sizeof(fuse_ll_ops[0]))
> diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
> index c55c0ca2fc1c..b889dae4de0e 100644
> --- a/tools/virtiofsd/fuse_lowlevel.h
> +++ b/tools/virtiofsd/fuse_lowlevel.h
> @@ -1226,6 +1226,19 @@ struct fuse_lowlevel_ops {
> */
> void (*lseek)(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> struct fuse_file_info *fi);
> +
> + /**
> + * Synchronize file system content
> + *
> + * If this request is answered with an error code of ENOSYS,
> + * this is treated as success and future calls to syncfs() will
> + * succeed automatically without being sent to the filesystem
> + * process.
> + *
> + * @param req request handle
> + * @param ino the inode number
> + */
> + void (*syncfs)(fuse_req_t req, fuse_ino_t ino);
> };
>
> /**
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index b3d0674f6d2f..0f65e6423cf5 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -3357,6 +3357,49 @@ static void lo_lseek(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> }
> }
>
> +static int lo_do_syncfs(struct lo_data *lo, struct lo_inode *inode)
> +{
> + int fd, ret = 0;
> +
> + fuse_log(FUSE_LOG_DEBUG, "lo_do_syncfs(ino=%" PRIu64 ")\n",
> + inode->fuse_ino);
> +
> + fd = lo_inode_open(lo, inode, O_RDONLY);
> + if (fd < 0) {
> + return -fd;
> + }
> +
> + if (syncfs(fd) < 0) {
> + ret = errno;
> + }
> +
> + close(fd);
> + return ret;
> +}
> +
> +static void lo_syncfs(fuse_req_t req, fuse_ino_t ino)
> +{
> + struct lo_data *lo = lo_data(req);
> + struct lo_inode *inode = lo_inode(req, ino);
> + int err;
> +
> + if (!inode) {
> + fuse_reply_err(req, EBADF);
> + return;
> + }
> +
> + err = lo_do_syncfs(lo, inode);
> + lo_inode_put(lo, &inode);
> +
> + /*
> + * If submounts aren't announced, the client only sends a request to
> + * sync the root inode. TODO: Track submounts internally and iterate
> + * over them as well.
> + */
> +
> + fuse_reply_err(req, err);
> +}
> +
> static void lo_destroy(void *userdata)
> {
> struct lo_data *lo = (struct lo_data *)userdata;
> @@ -3417,6 +3460,7 @@ static struct fuse_lowlevel_ops lo_oper = {
> .copy_file_range = lo_copy_file_range,
> #endif
> .lseek = lo_lseek,
> + .syncfs = lo_syncfs,
> .destroy = lo_destroy,
> };
>
> diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c
> index a3ce9f898d2d..3e9d6181dc69 100644
> --- a/tools/virtiofsd/passthrough_seccomp.c
> +++ b/tools/virtiofsd/passthrough_seccomp.c
> @@ -108,6 +108,7 @@ static const int syscall_allowlist[] = {
> SCMP_SYS(set_robust_list),
> SCMP_SYS(setxattr),
> SCMP_SYS(symlinkat),
> + SCMP_SYS(syncfs),
> SCMP_SYS(time), /* Rarely needed, except on static builds */
> SCMP_SYS(tgkill),
> SCMP_SYS(unlinkat),
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Virtio-fs] [PATCH v6 1/1] virtiofsd: Add basic support for FUSE_SYNCFS request
@ 2022-02-15 19:06 ` Vivek Goyal
0 siblings, 0 replies; 8+ messages in thread
From: Vivek Goyal @ 2022-02-15 19:06 UTC (permalink / raw)
To: Greg Kurz; +Cc: virtio-fs, qemu-devel
On Tue, Feb 15, 2022 at 07:15:29PM +0100, Greg Kurz wrote:
> Honor the expected behavior of syncfs() to synchronously flush all data
> and metadata to disk on linux systems.
>
> If virtiofsd is started with '-o announce_submounts', the client is
> expected to send a FUSE_SYNCFS request for each individual submount.
> In this case, we just create a new file descriptor on the submount
> inode with lo_inode_open(), call syncfs() on it and close it. The
> intermediary file is needed because O_PATH descriptors aren't
> backed by an actual file and syncfs() would fail with EBADF.
>
> If virtiofsd is started without '-o announce_submounts' or if the
> client doesn't have the FUSE_CAP_SUBMOUNTS capability, the client
> only sends a single FUSE_SYNCFS request for the root inode. The
> server would thus need to track submounts internally and call
> syncfs() on each of them. This will be implemented later.
>
> Note that syncfs() might suffer from a time penalty if the submounts
> are being hammered by some unrelated workload on the host. The only
> solution to prevent that is to avoid shared mounts.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Looks good to me. Thanks Greg.
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Vivek
> ---
> tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
> tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
> tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
> tools/virtiofsd/passthrough_seccomp.c | 1 +
> 4 files changed, 69 insertions(+)
>
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index e4679c73abc2..e02d8b25a5f6 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -1876,6 +1876,16 @@ static void do_lseek(fuse_req_t req, fuse_ino_t nodeid,
> }
> }
>
> +static void do_syncfs(fuse_req_t req, fuse_ino_t nodeid,
> + struct fuse_mbuf_iter *iter)
> +{
> + if (req->se->op.syncfs) {
> + req->se->op.syncfs(req, nodeid);
> + } else {
> + fuse_reply_err(req, ENOSYS);
> + }
> +}
> +
> static void do_init(fuse_req_t req, fuse_ino_t nodeid,
> struct fuse_mbuf_iter *iter)
> {
> @@ -2280,6 +2290,7 @@ static struct {
> [FUSE_RENAME2] = { do_rename2, "RENAME2" },
> [FUSE_COPY_FILE_RANGE] = { do_copy_file_range, "COPY_FILE_RANGE" },
> [FUSE_LSEEK] = { do_lseek, "LSEEK" },
> + [FUSE_SYNCFS] = { do_syncfs, "SYNCFS" },
> };
>
> #define FUSE_MAXOP (sizeof(fuse_ll_ops) / sizeof(fuse_ll_ops[0]))
> diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
> index c55c0ca2fc1c..b889dae4de0e 100644
> --- a/tools/virtiofsd/fuse_lowlevel.h
> +++ b/tools/virtiofsd/fuse_lowlevel.h
> @@ -1226,6 +1226,19 @@ struct fuse_lowlevel_ops {
> */
> void (*lseek)(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> struct fuse_file_info *fi);
> +
> + /**
> + * Synchronize file system content
> + *
> + * If this request is answered with an error code of ENOSYS,
> + * this is treated as success and future calls to syncfs() will
> + * succeed automatically without being sent to the filesystem
> + * process.
> + *
> + * @param req request handle
> + * @param ino the inode number
> + */
> + void (*syncfs)(fuse_req_t req, fuse_ino_t ino);
> };
>
> /**
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index b3d0674f6d2f..0f65e6423cf5 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -3357,6 +3357,49 @@ static void lo_lseek(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> }
> }
>
> +static int lo_do_syncfs(struct lo_data *lo, struct lo_inode *inode)
> +{
> + int fd, ret = 0;
> +
> + fuse_log(FUSE_LOG_DEBUG, "lo_do_syncfs(ino=%" PRIu64 ")\n",
> + inode->fuse_ino);
> +
> + fd = lo_inode_open(lo, inode, O_RDONLY);
> + if (fd < 0) {
> + return -fd;
> + }
> +
> + if (syncfs(fd) < 0) {
> + ret = errno;
> + }
> +
> + close(fd);
> + return ret;
> +}
> +
> +static void lo_syncfs(fuse_req_t req, fuse_ino_t ino)
> +{
> + struct lo_data *lo = lo_data(req);
> + struct lo_inode *inode = lo_inode(req, ino);
> + int err;
> +
> + if (!inode) {
> + fuse_reply_err(req, EBADF);
> + return;
> + }
> +
> + err = lo_do_syncfs(lo, inode);
> + lo_inode_put(lo, &inode);
> +
> + /*
> + * If submounts aren't announced, the client only sends a request to
> + * sync the root inode. TODO: Track submounts internally and iterate
> + * over them as well.
> + */
> +
> + fuse_reply_err(req, err);
> +}
> +
> static void lo_destroy(void *userdata)
> {
> struct lo_data *lo = (struct lo_data *)userdata;
> @@ -3417,6 +3460,7 @@ static struct fuse_lowlevel_ops lo_oper = {
> .copy_file_range = lo_copy_file_range,
> #endif
> .lseek = lo_lseek,
> + .syncfs = lo_syncfs,
> .destroy = lo_destroy,
> };
>
> diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c
> index a3ce9f898d2d..3e9d6181dc69 100644
> --- a/tools/virtiofsd/passthrough_seccomp.c
> +++ b/tools/virtiofsd/passthrough_seccomp.c
> @@ -108,6 +108,7 @@ static const int syscall_allowlist[] = {
> SCMP_SYS(set_robust_list),
> SCMP_SYS(setxattr),
> SCMP_SYS(symlinkat),
> + SCMP_SYS(syncfs),
> SCMP_SYS(time), /* Rarely needed, except on static builds */
> SCMP_SYS(tgkill),
> SCMP_SYS(unlinkat),
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v6 1/1] virtiofsd: Add basic support for FUSE_SYNCFS request
2022-02-15 19:06 ` [Virtio-fs] " Vivek Goyal
@ 2022-02-16 12:18 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2022-02-16 12:18 UTC (permalink / raw)
To: Vivek Goyal
Cc: virtio-fs, Sebastian Hasler, Greg Kurz, Stefan Hajnoczi, qemu-devel
* Vivek Goyal (vgoyal@redhat.com) wrote:
> On Tue, Feb 15, 2022 at 07:15:29PM +0100, Greg Kurz wrote:
> > Honor the expected behavior of syncfs() to synchronously flush all data
> > and metadata to disk on linux systems.
> >
> > If virtiofsd is started with '-o announce_submounts', the client is
> > expected to send a FUSE_SYNCFS request for each individual submount.
> > In this case, we just create a new file descriptor on the submount
> > inode with lo_inode_open(), call syncfs() on it and close it. The
> > intermediary file is needed because O_PATH descriptors aren't
> > backed by an actual file and syncfs() would fail with EBADF.
> >
> > If virtiofsd is started without '-o announce_submounts' or if the
> > client doesn't have the FUSE_CAP_SUBMOUNTS capability, the client
> > only sends a single FUSE_SYNCFS request for the root inode. The
> > server would thus need to track submounts internally and call
> > syncfs() on each of them. This will be implemented later.
> >
> > Note that syncfs() might suffer from a time penalty if the submounts
> > are being hammered by some unrelated workload on the host. The only
> > solution to prevent that is to avoid shared mounts.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> Looks good to me. Thanks Greg.
>
> Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Queued
> Vivek
>
> > ---
> > tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
> > tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
> > tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
> > tools/virtiofsd/passthrough_seccomp.c | 1 +
> > 4 files changed, 69 insertions(+)
> >
> > diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> > index e4679c73abc2..e02d8b25a5f6 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.c
> > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > @@ -1876,6 +1876,16 @@ static void do_lseek(fuse_req_t req, fuse_ino_t nodeid,
> > }
> > }
> >
> > +static void do_syncfs(fuse_req_t req, fuse_ino_t nodeid,
> > + struct fuse_mbuf_iter *iter)
> > +{
> > + if (req->se->op.syncfs) {
> > + req->se->op.syncfs(req, nodeid);
> > + } else {
> > + fuse_reply_err(req, ENOSYS);
> > + }
> > +}
> > +
> > static void do_init(fuse_req_t req, fuse_ino_t nodeid,
> > struct fuse_mbuf_iter *iter)
> > {
> > @@ -2280,6 +2290,7 @@ static struct {
> > [FUSE_RENAME2] = { do_rename2, "RENAME2" },
> > [FUSE_COPY_FILE_RANGE] = { do_copy_file_range, "COPY_FILE_RANGE" },
> > [FUSE_LSEEK] = { do_lseek, "LSEEK" },
> > + [FUSE_SYNCFS] = { do_syncfs, "SYNCFS" },
> > };
> >
> > #define FUSE_MAXOP (sizeof(fuse_ll_ops) / sizeof(fuse_ll_ops[0]))
> > diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
> > index c55c0ca2fc1c..b889dae4de0e 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.h
> > +++ b/tools/virtiofsd/fuse_lowlevel.h
> > @@ -1226,6 +1226,19 @@ struct fuse_lowlevel_ops {
> > */
> > void (*lseek)(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> > struct fuse_file_info *fi);
> > +
> > + /**
> > + * Synchronize file system content
> > + *
> > + * If this request is answered with an error code of ENOSYS,
> > + * this is treated as success and future calls to syncfs() will
> > + * succeed automatically without being sent to the filesystem
> > + * process.
> > + *
> > + * @param req request handle
> > + * @param ino the inode number
> > + */
> > + void (*syncfs)(fuse_req_t req, fuse_ino_t ino);
> > };
> >
> > /**
> > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > index b3d0674f6d2f..0f65e6423cf5 100644
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -3357,6 +3357,49 @@ static void lo_lseek(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> > }
> > }
> >
> > +static int lo_do_syncfs(struct lo_data *lo, struct lo_inode *inode)
> > +{
> > + int fd, ret = 0;
> > +
> > + fuse_log(FUSE_LOG_DEBUG, "lo_do_syncfs(ino=%" PRIu64 ")\n",
> > + inode->fuse_ino);
> > +
> > + fd = lo_inode_open(lo, inode, O_RDONLY);
> > + if (fd < 0) {
> > + return -fd;
> > + }
> > +
> > + if (syncfs(fd) < 0) {
> > + ret = errno;
> > + }
> > +
> > + close(fd);
> > + return ret;
> > +}
> > +
> > +static void lo_syncfs(fuse_req_t req, fuse_ino_t ino)
> > +{
> > + struct lo_data *lo = lo_data(req);
> > + struct lo_inode *inode = lo_inode(req, ino);
> > + int err;
> > +
> > + if (!inode) {
> > + fuse_reply_err(req, EBADF);
> > + return;
> > + }
> > +
> > + err = lo_do_syncfs(lo, inode);
> > + lo_inode_put(lo, &inode);
> > +
> > + /*
> > + * If submounts aren't announced, the client only sends a request to
> > + * sync the root inode. TODO: Track submounts internally and iterate
> > + * over them as well.
> > + */
> > +
> > + fuse_reply_err(req, err);
> > +}
> > +
> > static void lo_destroy(void *userdata)
> > {
> > struct lo_data *lo = (struct lo_data *)userdata;
> > @@ -3417,6 +3460,7 @@ static struct fuse_lowlevel_ops lo_oper = {
> > .copy_file_range = lo_copy_file_range,
> > #endif
> > .lseek = lo_lseek,
> > + .syncfs = lo_syncfs,
> > .destroy = lo_destroy,
> > };
> >
> > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c
> > index a3ce9f898d2d..3e9d6181dc69 100644
> > --- a/tools/virtiofsd/passthrough_seccomp.c
> > +++ b/tools/virtiofsd/passthrough_seccomp.c
> > @@ -108,6 +108,7 @@ static const int syscall_allowlist[] = {
> > SCMP_SYS(set_robust_list),
> > SCMP_SYS(setxattr),
> > SCMP_SYS(symlinkat),
> > + SCMP_SYS(syncfs),
> > SCMP_SYS(time), /* Rarely needed, except on static builds */
> > SCMP_SYS(tgkill),
> > SCMP_SYS(unlinkat),
> > --
> > 2.34.1
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Virtio-fs] [PATCH v6 1/1] virtiofsd: Add basic support for FUSE_SYNCFS request
@ 2022-02-16 12:18 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 8+ messages in thread
From: Dr. David Alan Gilbert @ 2022-02-16 12:18 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs, qemu-devel
* Vivek Goyal (vgoyal@redhat.com) wrote:
> On Tue, Feb 15, 2022 at 07:15:29PM +0100, Greg Kurz wrote:
> > Honor the expected behavior of syncfs() to synchronously flush all data
> > and metadata to disk on linux systems.
> >
> > If virtiofsd is started with '-o announce_submounts', the client is
> > expected to send a FUSE_SYNCFS request for each individual submount.
> > In this case, we just create a new file descriptor on the submount
> > inode with lo_inode_open(), call syncfs() on it and close it. The
> > intermediary file is needed because O_PATH descriptors aren't
> > backed by an actual file and syncfs() would fail with EBADF.
> >
> > If virtiofsd is started without '-o announce_submounts' or if the
> > client doesn't have the FUSE_CAP_SUBMOUNTS capability, the client
> > only sends a single FUSE_SYNCFS request for the root inode. The
> > server would thus need to track submounts internally and call
> > syncfs() on each of them. This will be implemented later.
> >
> > Note that syncfs() might suffer from a time penalty if the submounts
> > are being hammered by some unrelated workload on the host. The only
> > solution to prevent that is to avoid shared mounts.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> Looks good to me. Thanks Greg.
>
> Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Queued
> Vivek
>
> > ---
> > tools/virtiofsd/fuse_lowlevel.c | 11 +++++++
> > tools/virtiofsd/fuse_lowlevel.h | 13 ++++++++
> > tools/virtiofsd/passthrough_ll.c | 44 +++++++++++++++++++++++++++
> > tools/virtiofsd/passthrough_seccomp.c | 1 +
> > 4 files changed, 69 insertions(+)
> >
> > diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> > index e4679c73abc2..e02d8b25a5f6 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.c
> > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > @@ -1876,6 +1876,16 @@ static void do_lseek(fuse_req_t req, fuse_ino_t nodeid,
> > }
> > }
> >
> > +static void do_syncfs(fuse_req_t req, fuse_ino_t nodeid,
> > + struct fuse_mbuf_iter *iter)
> > +{
> > + if (req->se->op.syncfs) {
> > + req->se->op.syncfs(req, nodeid);
> > + } else {
> > + fuse_reply_err(req, ENOSYS);
> > + }
> > +}
> > +
> > static void do_init(fuse_req_t req, fuse_ino_t nodeid,
> > struct fuse_mbuf_iter *iter)
> > {
> > @@ -2280,6 +2290,7 @@ static struct {
> > [FUSE_RENAME2] = { do_rename2, "RENAME2" },
> > [FUSE_COPY_FILE_RANGE] = { do_copy_file_range, "COPY_FILE_RANGE" },
> > [FUSE_LSEEK] = { do_lseek, "LSEEK" },
> > + [FUSE_SYNCFS] = { do_syncfs, "SYNCFS" },
> > };
> >
> > #define FUSE_MAXOP (sizeof(fuse_ll_ops) / sizeof(fuse_ll_ops[0]))
> > diff --git a/tools/virtiofsd/fuse_lowlevel.h b/tools/virtiofsd/fuse_lowlevel.h
> > index c55c0ca2fc1c..b889dae4de0e 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.h
> > +++ b/tools/virtiofsd/fuse_lowlevel.h
> > @@ -1226,6 +1226,19 @@ struct fuse_lowlevel_ops {
> > */
> > void (*lseek)(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> > struct fuse_file_info *fi);
> > +
> > + /**
> > + * Synchronize file system content
> > + *
> > + * If this request is answered with an error code of ENOSYS,
> > + * this is treated as success and future calls to syncfs() will
> > + * succeed automatically without being sent to the filesystem
> > + * process.
> > + *
> > + * @param req request handle
> > + * @param ino the inode number
> > + */
> > + void (*syncfs)(fuse_req_t req, fuse_ino_t ino);
> > };
> >
> > /**
> > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > index b3d0674f6d2f..0f65e6423cf5 100644
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -3357,6 +3357,49 @@ static void lo_lseek(fuse_req_t req, fuse_ino_t ino, off_t off, int whence,
> > }
> > }
> >
> > +static int lo_do_syncfs(struct lo_data *lo, struct lo_inode *inode)
> > +{
> > + int fd, ret = 0;
> > +
> > + fuse_log(FUSE_LOG_DEBUG, "lo_do_syncfs(ino=%" PRIu64 ")\n",
> > + inode->fuse_ino);
> > +
> > + fd = lo_inode_open(lo, inode, O_RDONLY);
> > + if (fd < 0) {
> > + return -fd;
> > + }
> > +
> > + if (syncfs(fd) < 0) {
> > + ret = errno;
> > + }
> > +
> > + close(fd);
> > + return ret;
> > +}
> > +
> > +static void lo_syncfs(fuse_req_t req, fuse_ino_t ino)
> > +{
> > + struct lo_data *lo = lo_data(req);
> > + struct lo_inode *inode = lo_inode(req, ino);
> > + int err;
> > +
> > + if (!inode) {
> > + fuse_reply_err(req, EBADF);
> > + return;
> > + }
> > +
> > + err = lo_do_syncfs(lo, inode);
> > + lo_inode_put(lo, &inode);
> > +
> > + /*
> > + * If submounts aren't announced, the client only sends a request to
> > + * sync the root inode. TODO: Track submounts internally and iterate
> > + * over them as well.
> > + */
> > +
> > + fuse_reply_err(req, err);
> > +}
> > +
> > static void lo_destroy(void *userdata)
> > {
> > struct lo_data *lo = (struct lo_data *)userdata;
> > @@ -3417,6 +3460,7 @@ static struct fuse_lowlevel_ops lo_oper = {
> > .copy_file_range = lo_copy_file_range,
> > #endif
> > .lseek = lo_lseek,
> > + .syncfs = lo_syncfs,
> > .destroy = lo_destroy,
> > };
> >
> > diff --git a/tools/virtiofsd/passthrough_seccomp.c b/tools/virtiofsd/passthrough_seccomp.c
> > index a3ce9f898d2d..3e9d6181dc69 100644
> > --- a/tools/virtiofsd/passthrough_seccomp.c
> > +++ b/tools/virtiofsd/passthrough_seccomp.c
> > @@ -108,6 +108,7 @@ static const int syscall_allowlist[] = {
> > SCMP_SYS(set_robust_list),
> > SCMP_SYS(setxattr),
> > SCMP_SYS(symlinkat),
> > + SCMP_SYS(syncfs),
> > SCMP_SYS(time), /* Rarely needed, except on static builds */
> > SCMP_SYS(tgkill),
> > SCMP_SYS(unlinkat),
> > --
> > 2.34.1
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-02-16 12:21 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-15 18:15 [PATCH v6 0/1] virtiofsd: Add support for FUSE_SYNCFS request Greg Kurz
2022-02-15 18:15 ` [Virtio-fs] " Greg Kurz
2022-02-15 18:15 ` [PATCH v6 1/1] virtiofsd: Add basic " Greg Kurz
2022-02-15 18:15 ` [Virtio-fs] " Greg Kurz
2022-02-15 19:06 ` Vivek Goyal
2022-02-15 19:06 ` [Virtio-fs] " Vivek Goyal
2022-02-16 12:18 ` Dr. David Alan Gilbert
2022-02-16 12:18 ` [Virtio-fs] " Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.