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 <gr...@kaod.org>
Looks good to me. Thanks Greg. Reviewed-by: Vivek Goyal <vgo...@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 > _______________________________________________ Virtio-fs mailing list Virtio-fs@redhat.com https://listman.redhat.com/mailman/listinfo/virtio-fs