All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: virtio-fs-list <virtio-fs@redhat.com>
Subject: Re: [Virtio-fs] [PATCH] virtiofsd: During setup mapping, open file O_RDWR only if needed
Date: Thu, 25 Jul 2019 11:21:19 -0400	[thread overview]
Message-ID: <20190725152119.GA15543@redhat.com> (raw)
In-Reply-To: <20190725132743.GH2656@work-vm>

On Thu, Jul 25, 2019 at 02:27:43PM +0100, Dr. David Alan Gilbert wrote:
> * Liu Bo (bo.liu@linux.alibaba.com) wrote:
> > On Wed, Jul 24, 2019 at 05:10:24PM -0400, Vivek Goyal wrote:
> > > 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/passthrough_ll.c |    9 ++++++---
> > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > 
> > > Index: qemu/contrib/virtiofsd/passthrough_ll.c
> > > ===================================================================
> > > --- qemu.orig/contrib/virtiofsd/passthrough_ll.c	2019-07-24 16:31:07.014871768 -0400
> > > +++ qemu/contrib/virtiofsd/passthrough_ll.c	2019-07-24 16:32:10.064412722 -0400
> > > @@ -2197,15 +2197,15 @@ static void lo_setupmapping(fuse_req_t r
> > >          VhostUserFSSlaveMsg msg = { 0 };
> > >          uint64_t vhu_flags;
> > >  	char *buf;
> > > +	bool writable = flags & FUSE_SETUPMAPPING_FLAG_WRITE;
> > 
> > typo? flags is set to O_WRONLY or 0 in do_lookup, although this may work as same.
> 
> Right; that should be O_WRONLY;  
> do_setupmapping does:
> 
>         genflags = 0;
>         genflags |= (arg->flags & FUSE_SETUPMAPPING_FLAG_WRITE) ? O_WRONLY : 0;
> 
> to convert from the fuse definition on the wire to the O_ notation
> before calling the filesstem code.

So I have two questions.

- O_WRONLY does not sound like correct mapping. FUSE_SETUPMAPPING_FLAG_WRITE
  only says that write behavior is required. It does not say *write only*.

- Is it must that we need to translate fuse message specific flag
  to O_* notation. How about let filesystem code parse it?

Thanks
Vivek


  reply	other threads:[~2019-07-25 15:21 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-24 21:10 [Virtio-fs] [PATCH] virtiofsd: During setup mapping, open file O_RDWR only if needed Vivek Goyal
2019-07-24 23:29 ` Liu Bo
2019-07-25 13:27   ` Dr. David Alan Gilbert
2019-07-25 15:21     ` Vivek Goyal [this message]
2019-07-25 15:48       ` Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190725152119.GA15543@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=virtio-fs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.