* [Virtio-fs] [PATCH 1/2] virtiofsd: During setup mapping, open file O_RDWR only if needed
2019-07-26 15:58 [Virtio-fs] [PATCH 0/2][V2] virtiofsd: Do not setup mapping read-write always Vivek Goyal
@ 2019-07-26 15:58 ` Vivek Goyal
2019-07-26 15:58 ` [Virtio-fs] [PATCH 2/2] virtiofsd: No need to promote O_WRONLY to O_RDWR Vivek Goyal
1 sibling, 0 replies; 4+ messages in thread
From: Vivek Goyal @ 2019-07-26 15:58 UTC (permalink / raw)
To: virtio-fs
As of now we always open file O_RDWR (even if writable mappings are not
required). This leads to copy up of file if file is backed by overlayfs
and hence nullying advantages of overlayfs.
So open file O_RDONLY if writable mappings are not required. Open O_RDWR
if writable mappings are needed.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
contrib/virtiofsd/fuse_lowlevel.c | 5 +++--
contrib/virtiofsd/passthrough_ll.c | 17 +++++++----------
2 files changed, 10 insertions(+), 12 deletions(-)
diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
index 8adc4b1ab8..2ed894563b 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -1891,8 +1891,9 @@ static void do_setupmapping(fuse_req_t req, fuse_ino_t nodeid,
// for now just use O_ flags
uint64_t genflags;
- genflags = 0;
- genflags |= (arg->flags & FUSE_SETUPMAPPING_FLAG_WRITE) ? O_WRONLY : 0;
+ genflags = O_RDONLY;
+ if (arg->flags & FUSE_SETUPMAPPING_FLAG_WRITE)
+ genflags = O_RDWR;
if (req->se->op.setupmapping) {
/*
diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
index 9ae1381618..c09c46a7e5 100644
--- a/contrib/virtiofsd/passthrough_ll.c
+++ b/contrib/virtiofsd/passthrough_ll.c
@@ -2197,15 +2197,17 @@ static void lo_setupmapping(fuse_req_t req, fuse_ino_t ino, uint64_t foffset,
VhostUserFSSlaveMsg msg = { 0 };
uint64_t vhu_flags;
char *buf;
+ bool writable = flags & O_RDWR;
if (lo_debug(req))
- fuse_debug("lo_setupmapping(ino=%" PRIu64 ", fi=0x%p)\n", ino,
- (void *)fi);
+ fuse_debug("lo_setupmapping(ino=%" PRIu64 ", fi=0x%p,"
+ " foffset=%" PRIu64 ", len=%" PRIu64
+ ", moffset=%" PRIu64 ", flags=%" PRIu64 ")\n", ino,
+ (void *)fi, foffset, len, moffset, flags);
vhu_flags = VHOST_USER_FS_FLAG_MAP_R;
- if (flags & O_WRONLY) {
+ if (writable)
vhu_flags |= VHOST_USER_FS_FLAG_MAP_W;
- }
msg.fd_offset[0] = foffset;
msg.len[0] = len;
@@ -2219,15 +2221,10 @@ static void lo_setupmapping(fuse_req_t req, fuse_ino_t ino, uint64_t foffset,
if (res == -1)
return (void) fuse_reply_err(req, errno);
- /*
- * TODO: O_RDWR might not be allowed if file is read only or
- * write only. Fix it.
- */
- fd = openat(lo->proc_self_fd, buf, O_RDWR);
+ fd = openat(lo->proc_self_fd, buf, flags);
free(buf);
if (fd == -1)
return (void) fuse_reply_err(req, errno);
-
}
if (fuse_virtio_map(req, &msg, fd)) {
--
2.17.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [Virtio-fs] [PATCH 2/2] virtiofsd: No need to promote O_WRONLY to O_RDWR
2019-07-26 15:58 [Virtio-fs] [PATCH 0/2][V2] virtiofsd: Do not setup mapping read-write always Vivek Goyal
2019-07-26 15:58 ` [Virtio-fs] [PATCH 1/2] virtiofsd: During setup mapping, open file O_RDWR only if needed Vivek Goyal
@ 2019-07-26 15:58 ` Vivek Goyal
2019-07-29 11:15 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 4+ messages in thread
From: Vivek Goyal @ 2019-07-26 15:58 UTC (permalink / raw)
To: virtio-fs
I am not sure why do we still need this logic. mmap() needs file to be
opened for O_RDONLY atleast. And we open file either as O_RDONLY or
O_RDWR in lo_setupmapping() depending on if read-only or read-write
mapping is required.
So it should not matter how fd is opended during lo_create() or lo_open()
because we are not going to use that fd anyway for mmap().
So for now drop this logic. I ran pjdfstests and blogbench and both
ran successfully.
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
contrib/virtiofsd/passthrough_ll.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
index c09c46a7e5..502ce50178 100644
--- a/contrib/virtiofsd/passthrough_ll.c
+++ b/contrib/virtiofsd/passthrough_ll.c
@@ -1513,12 +1513,6 @@ static void lo_create(fuse_req_t req, fuse_ino_t parent, const char *name,
if (err)
goto out;
- /* Promote O_WRONLY to O_RDWR. Otherwise later mmap(PROT_WRITE) fails */
- if ((fi->flags & O_ACCMODE) == O_WRONLY) {
- fi->flags &= ~O_ACCMODE;
- fi->flags |= O_RDWR;
- }
-
fd = openat(lo_fd(req, parent), name,
(fi->flags | O_CREAT) & ~O_NOFOLLOW, mode);
err = fd == -1 ? errno : 0;
@@ -1710,12 +1704,6 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
fuse_debug("lo_open(ino=%" PRIu64 ", flags=%d)\n",
ino, fi->flags);
- /* Promote O_WRONLY to O_RDWR. Otherwise later mmap(PROT_WRITE) fails */
- if ((fi->flags & O_ACCMODE) == O_WRONLY) {
- fi->flags &= ~O_ACCMODE;
- fi->flags |= O_RDWR;
- }
-
/* With writeback cache, kernel may send read requests even
when userspace opened write-only */
if (lo->writeback && (fi->flags & O_ACCMODE) == O_WRONLY) {
--
2.17.2
^ permalink raw reply related [flat|nested] 4+ messages in thread