From: Vivek Goyal <vgoyal@redhat.com> To: Stefan Hajnoczi <stefanha@redhat.com> Cc: miklos@szeredi.hu, qemu-devel@nongnu.org, iangelak@redhat.com, dgilbert@redhat.com, virtio-fs@redhat.com, jaggel@bu.edu Subject: Re: [PATCH 12/13] virtiofsd: Implement blocking posix locks Date: Tue, 5 Oct 2021 09:26:14 -0400 [thread overview] Message-ID: <YVxSdmNsNXm4DU1z@redhat.com> (raw) In-Reply-To: <YVsYmHhbjPs/LEUc@stefanha-x1.localdomain> On Mon, Oct 04, 2021 at 04:07:04PM +0100, Stefan Hajnoczi wrote: > On Thu, Sep 30, 2021 at 11:30:36AM -0400, Vivek Goyal wrote: > > As of now we don't support fcntl(F_SETLKW) and if we see one, we return > > -EOPNOTSUPP. > > > > Change that by accepting these requests and returning a reply > > immediately asking caller to wait. Once lock is available, send a > > notification to the waiter indicating lock is available. > > > > In response to lock request, we are returning error value as "1", which > > signals to client to queue the lock request internally and later client > > will get a notification which will signal lock is taken (or error). And > > then fuse client should wake up the guest process. > > > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > Signed-off-by: Ioannis Angelakopoulos <iangelak@redhat.com> > > --- > > tools/virtiofsd/fuse_lowlevel.c | 37 ++++++++++++++++- > > tools/virtiofsd/fuse_lowlevel.h | 26 ++++++++++++ > > tools/virtiofsd/fuse_virtio.c | 50 ++++++++++++++++++++--- > > tools/virtiofsd/passthrough_ll.c | 70 ++++++++++++++++++++++++++++---- > > 4 files changed, 167 insertions(+), 16 deletions(-) > > > > diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c > > index e4679c73ab..2e7f4b786d 100644 > > --- a/tools/virtiofsd/fuse_lowlevel.c > > +++ b/tools/virtiofsd/fuse_lowlevel.c > > @@ -179,8 +179,8 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov, > > .unique = req->unique, > > .error = error, > > }; > > - > > - if (error <= -1000 || error > 0) { > > + /* error = 1 has been used to signal client to wait for notificaiton */ > > s/notificaiton/notification/ Will fix. I have made too many spelling mistakes. :-( > > > + if (error <= -1000 || error > 1) { > > fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n", error); > > out.error = -ERANGE; > > } > > @@ -290,6 +290,11 @@ int fuse_reply_err(fuse_req_t req, int err) > > return send_reply(req, -err, NULL, 0); > > } > > > > +int fuse_reply_wait(fuse_req_t req) > > +{ > > + return send_reply(req, 1, NULL, 0); > > +} > > + > > void fuse_reply_none(fuse_req_t req) > > { > > fuse_free_req(req); > > @@ -2165,6 +2170,34 @@ static void do_destroy(fuse_req_t req, fuse_ino_t nodeid, > > send_reply_ok(req, NULL, 0); > > } > > > > +static int send_notify_iov(struct fuse_session *se, int notify_code, > > + struct iovec *iov, int count) > > +{ > > + struct fuse_out_header out; > > + if (!se->got_init) { > > + return -ENOTCONN; > > + } > > + out.unique = 0; > > + out.error = notify_code; > > Please fully initialize all fuse_out_header fields so it's obvious that > there is no accidental information leak from virtiofsd to the guest: > > struct fuse_out_header out = { > .error = notify_code, > }; > > The host must not expose uninitialized memory to the guest (just like > the kernel vs userspace). fuse_send_msg() initializes out.len later, but > to be on the safe side I think we should be explicit here. Agreed. Its better to be explicit here and initialize fuse_out_header fully. Will do. Vivek
WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com> To: Stefan Hajnoczi <stefanha@redhat.com> Cc: miklos@szeredi.hu, qemu-devel@nongnu.org, virtio-fs@redhat.com Subject: Re: [Virtio-fs] [PATCH 12/13] virtiofsd: Implement blocking posix locks Date: Tue, 5 Oct 2021 09:26:14 -0400 [thread overview] Message-ID: <YVxSdmNsNXm4DU1z@redhat.com> (raw) In-Reply-To: <YVsYmHhbjPs/LEUc@stefanha-x1.localdomain> On Mon, Oct 04, 2021 at 04:07:04PM +0100, Stefan Hajnoczi wrote: > On Thu, Sep 30, 2021 at 11:30:36AM -0400, Vivek Goyal wrote: > > As of now we don't support fcntl(F_SETLKW) and if we see one, we return > > -EOPNOTSUPP. > > > > Change that by accepting these requests and returning a reply > > immediately asking caller to wait. Once lock is available, send a > > notification to the waiter indicating lock is available. > > > > In response to lock request, we are returning error value as "1", which > > signals to client to queue the lock request internally and later client > > will get a notification which will signal lock is taken (or error). And > > then fuse client should wake up the guest process. > > > > Signed-off-by: Vivek Goyal <vgoyal@redhat.com> > > Signed-off-by: Ioannis Angelakopoulos <iangelak@redhat.com> > > --- > > tools/virtiofsd/fuse_lowlevel.c | 37 ++++++++++++++++- > > tools/virtiofsd/fuse_lowlevel.h | 26 ++++++++++++ > > tools/virtiofsd/fuse_virtio.c | 50 ++++++++++++++++++++--- > > tools/virtiofsd/passthrough_ll.c | 70 ++++++++++++++++++++++++++++---- > > 4 files changed, 167 insertions(+), 16 deletions(-) > > > > diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c > > index e4679c73ab..2e7f4b786d 100644 > > --- a/tools/virtiofsd/fuse_lowlevel.c > > +++ b/tools/virtiofsd/fuse_lowlevel.c > > @@ -179,8 +179,8 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov, > > .unique = req->unique, > > .error = error, > > }; > > - > > - if (error <= -1000 || error > 0) { > > + /* error = 1 has been used to signal client to wait for notificaiton */ > > s/notificaiton/notification/ Will fix. I have made too many spelling mistakes. :-( > > > + if (error <= -1000 || error > 1) { > > fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n", error); > > out.error = -ERANGE; > > } > > @@ -290,6 +290,11 @@ int fuse_reply_err(fuse_req_t req, int err) > > return send_reply(req, -err, NULL, 0); > > } > > > > +int fuse_reply_wait(fuse_req_t req) > > +{ > > + return send_reply(req, 1, NULL, 0); > > +} > > + > > void fuse_reply_none(fuse_req_t req) > > { > > fuse_free_req(req); > > @@ -2165,6 +2170,34 @@ static void do_destroy(fuse_req_t req, fuse_ino_t nodeid, > > send_reply_ok(req, NULL, 0); > > } > > > > +static int send_notify_iov(struct fuse_session *se, int notify_code, > > + struct iovec *iov, int count) > > +{ > > + struct fuse_out_header out; > > + if (!se->got_init) { > > + return -ENOTCONN; > > + } > > + out.unique = 0; > > + out.error = notify_code; > > Please fully initialize all fuse_out_header fields so it's obvious that > there is no accidental information leak from virtiofsd to the guest: > > struct fuse_out_header out = { > .error = notify_code, > }; > > The host must not expose uninitialized memory to the guest (just like > the kernel vs userspace). fuse_send_msg() initializes out.len later, but > to be on the safe side I think we should be explicit here. Agreed. Its better to be explicit here and initialize fuse_out_header fully. Will do. Vivek
next prev parent reply other threads:[~2021-10-05 13:28 UTC|newest] Thread overview: 106+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-30 15:30 [PATCH 00/13] virtiofsd: Support notification queue and Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-09-30 15:30 ` [PATCH 01/13] virtio_fs.h: Add notification queue feature bit Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 13:12 ` Stefan Hajnoczi 2021-10-04 13:12 ` [Virtio-fs] " Stefan Hajnoczi 2021-09-30 15:30 ` [PATCH 02/13] virtiofsd: fuse.h header file changes for lock notification Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 13:16 ` Stefan Hajnoczi 2021-10-04 13:16 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-04 14:01 ` Vivek Goyal 2021-10-04 14:01 ` [Virtio-fs] " Vivek Goyal 2021-09-30 15:30 ` [PATCH 03/13] virtiofsd: Remove unused virtio_fs_config definition Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 13:17 ` Stefan Hajnoczi 2021-10-04 13:17 ` [Virtio-fs] " Stefan Hajnoczi 2021-09-30 15:30 ` [PATCH 04/13] virtiofsd: Add a helper to send element on virtqueue Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 13:19 ` Stefan Hajnoczi 2021-10-04 13:19 ` [Virtio-fs] " Stefan Hajnoczi 2021-09-30 15:30 ` [PATCH 05/13] virtiofsd: Add a helper to stop all queues Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 13:22 ` Stefan Hajnoczi 2021-10-04 13:22 ` [Virtio-fs] " Stefan Hajnoczi 2021-09-30 15:30 ` [PATCH 06/13] vhost-user-fs: Use helpers to create/cleanup virtqueue Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 13:54 ` Stefan Hajnoczi 2021-10-04 13:54 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-04 19:58 ` Vivek Goyal 2021-10-04 19:58 ` [Virtio-fs] " Vivek Goyal 2021-10-05 8:09 ` Stefan Hajnoczi 2021-10-05 8:09 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-06 13:35 ` Christophe de Dinechin 2021-10-06 13:35 ` Christophe de Dinechin 2021-10-06 17:40 ` Vivek Goyal 2021-10-06 17:40 ` Vivek Goyal 2021-09-30 15:30 ` [PATCH 07/13] virtiofsd: Release file locks using F_UNLCK Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-05 13:37 ` Christophe de Dinechin 2021-10-05 13:37 ` Christophe de Dinechin 2021-10-05 15:38 ` Vivek Goyal 2021-10-05 15:38 ` Vivek Goyal 2021-09-30 15:30 ` [PATCH 08/13] virtiofsd: Create a notification queue Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 14:30 ` Stefan Hajnoczi 2021-10-04 14:30 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-04 21:01 ` Vivek Goyal 2021-10-04 21:01 ` [Virtio-fs] " Vivek Goyal 2021-10-05 8:14 ` Stefan Hajnoczi 2021-10-05 8:14 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-05 12:31 ` Vivek Goyal 2021-10-05 12:31 ` [Virtio-fs] " Vivek Goyal 2021-09-30 15:30 ` [PATCH 09/13] virtiofsd: Specify size of notification buffer using config space Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 14:33 ` Stefan Hajnoczi 2021-10-04 14:33 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-04 21:10 ` Vivek Goyal 2021-10-04 21:10 ` [Virtio-fs] " Vivek Goyal 2021-10-06 10:05 ` Christophe de Dinechin 2021-10-06 10:05 ` Christophe de Dinechin 2021-09-30 15:30 ` [PATCH 10/13] virtiofsd: Custom threadpool for remote blocking posix locks requests Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 14:54 ` Stefan Hajnoczi 2021-10-04 14:54 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-05 13:06 ` Vivek Goyal 2021-10-05 13:06 ` [Virtio-fs] " Vivek Goyal 2021-10-05 20:09 ` Vivek Goyal 2021-10-05 20:09 ` [Virtio-fs] " Vivek Goyal 2021-10-06 10:26 ` Stefan Hajnoczi 2021-10-06 10:26 ` [Virtio-fs] " Stefan Hajnoczi 2021-09-30 15:30 ` [PATCH 11/13] virtiofsd: Shutdown notification queue in the end Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 15:01 ` Stefan Hajnoczi 2021-10-04 15:01 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-05 13:19 ` Vivek Goyal 2021-10-05 13:19 ` [Virtio-fs] " Vivek Goyal 2021-10-06 15:15 ` Christophe de Dinechin 2021-10-06 15:15 ` Christophe de Dinechin 2021-10-06 17:58 ` Vivek Goyal 2021-10-06 17:58 ` Vivek Goyal 2021-09-30 15:30 ` [PATCH 12/13] virtiofsd: Implement blocking posix locks Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-04 15:07 ` Stefan Hajnoczi 2021-10-04 15:07 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-05 13:26 ` Vivek Goyal [this message] 2021-10-05 13:26 ` Vivek Goyal 2021-10-05 12:22 ` Stefan Hajnoczi 2021-10-05 12:22 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-05 15:14 ` Vivek Goyal 2021-10-05 15:14 ` [Virtio-fs] " Vivek Goyal 2021-10-05 15:49 ` Stefan Hajnoczi 2021-10-05 15:49 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-06 15:34 ` Christophe de Dinechin 2021-10-06 15:34 ` Christophe de Dinechin 2021-10-06 18:17 ` Vivek Goyal 2021-10-06 18:17 ` Vivek Goyal 2021-09-30 15:30 ` [PATCH 13/13] virtiofsd, seccomp: Add clock_nanosleep() to allow list Vivek Goyal 2021-09-30 15:30 ` [Virtio-fs] " Vivek Goyal 2021-10-05 12:22 ` Stefan Hajnoczi 2021-10-05 12:22 ` [Virtio-fs] " Stefan Hajnoczi 2021-10-05 15:16 ` Vivek Goyal 2021-10-05 15:50 ` Stefan Hajnoczi 2021-10-05 17:28 ` Vivek Goyal 2021-10-06 10:27 ` Stefan Hajnoczi 2021-10-25 18:00 ` [PATCH 00/13] virtiofsd: Support notification queue and Dr. David Alan Gilbert 2021-10-25 18:00 ` [Virtio-fs] " 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=YVxSdmNsNXm4DU1z@redhat.com \ --to=vgoyal@redhat.com \ --cc=dgilbert@redhat.com \ --cc=iangelak@redhat.com \ --cc=jaggel@bu.edu \ --cc=miklos@szeredi.hu \ --cc=qemu-devel@nongnu.org \ --cc=stefanha@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: linkBe 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.