All of lore.kernel.org
 help / color / mirror / Atom feed
* [Virtio-fs] [PATCH] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries
@ 2019-05-13 14:41 Peng Tao
  2019-05-22 11:26 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 5+ messages in thread
From: Peng Tao @ 2019-05-13 14:41 UTC (permalink / raw)
  To: virtio-fs; +Cc: Peng Tao

The fuse wire protocol is changed so that we can unmap multiple
mappings in a single call.

Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
---
 contrib/virtiofsd/fuse_kernel.h    |  9 +++++++--
 contrib/virtiofsd/fuse_lowlevel.c  | 21 ++++++++++++++------
 contrib/virtiofsd/fuse_lowlevel.h  |  5 +++--
 contrib/virtiofsd/passthrough_ll.c | 31 ++++++++++++++++++++----------
 4 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
index ce46046a4f..093cacff02 100644
--- a/contrib/virtiofsd/fuse_kernel.h
+++ b/contrib/virtiofsd/fuse_kernel.h
@@ -830,9 +830,14 @@ struct fuse_setupmapping_out {
         uint64_t        len[FUSE_SETUPMAPPING_ENTRIES];
 };
 
-struct fuse_removemapping_in {
+struct fuse_removemapping_in_header {
         /* An already open handle */
-	uint64_t	fh;
+        uint64_t        fh;
+        /* number of fuse_removemapping_in follows */
+        unsigned        num;
+};
+
+struct fuse_removemapping_in {
         /* Offset into the dax to start the unmapping */
         uint64_t        moffset;
         /* Length of mapping required */
diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
index 111c6e1636..4619865c2c 100644
--- a/contrib/virtiofsd/fuse_lowlevel.c
+++ b/contrib/virtiofsd/fuse_lowlevel.c
@@ -1907,21 +1907,30 @@ static void do_setupmapping(fuse_req_t req, fuse_ino_t nodeid,
 static void do_removemapping(fuse_req_t req, fuse_ino_t nodeid,
 			     struct fuse_mbuf_iter *iter)
 {
-	struct fuse_removemapping_in *arg;
+	struct fuse_removemapping_in_header *header;
+	struct fuse_removemapping_in *argp;
         struct fuse_file_info fi;
 
-	arg = fuse_mbuf_iter_advance(iter, sizeof(*arg));
-	if (!arg) {
+	header = fuse_mbuf_iter_advance(iter, sizeof(*header));
+	if (!header || header->num <= 0) {
+		fprintf(stderr, "do_removemapping: invalid header %p\n", header);
+		fuse_reply_err(req, EINVAL);
+		return;
+	}
+
+	argp = fuse_mbuf_iter_advance(iter, header->num * sizeof(*argp));
+	if (!argp) {
+		fprintf(stderr, "do_removemapping: invalid in, expected %d * %ld, has %ld - %ld\n",
+				header->num, sizeof(*argp), iter->size, iter->pos);
 		fuse_reply_err(req, EINVAL);
 		return;
 	}
 
         memset(&fi, 0, sizeof(fi));
-	fi.fh = arg->fh;
+	fi.fh = header->fh;
 
 	if (req->se->op.removemapping)
-		req->se->op.removemapping(req, req->se, nodeid, arg->moffset,
-                                          arg->len, &fi);
+		req->se->op.removemapping(req, req->se, nodeid, header->num, argp, &fi);
 	else
 		fuse_reply_err(req, ENOSYS);
 }
diff --git a/contrib/virtiofsd/fuse_lowlevel.h b/contrib/virtiofsd/fuse_lowlevel.h
index 6a001c4605..b2b8747074 100644
--- a/contrib/virtiofsd/fuse_lowlevel.h
+++ b/contrib/virtiofsd/fuse_lowlevel.h
@@ -23,6 +23,7 @@
 #endif
 
 #include "fuse_common.h"
+#include "fuse_kernel.h"
 
 #include <utime.h>
 #include <fcntl.h>
@@ -1200,8 +1201,8 @@ struct fuse_lowlevel_ops {
          * TODO
          */
         void (*removemapping) (fuse_req_t req, struct fuse_session *se,
-                               fuse_ino_t ino, uint64_t moffset,
-                               uint64_t len, struct fuse_file_info *fi);
+                               fuse_ino_t ino, unsigned num,
+                               struct fuse_removemapping_in *args, struct fuse_file_info *fi);
 };
 
 /**
diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
index 1ddb68f1db..96d5468ead 100644
--- a/contrib/virtiofsd/passthrough_ll.c
+++ b/contrib/virtiofsd/passthrough_ll.c
@@ -1925,20 +1925,31 @@ err:
 }
 
 static void lo_removemapping(fuse_req_t req, struct fuse_session *se,
-                             fuse_ino_t ino, uint64_t moffset,
-                             uint64_t len, struct fuse_file_info *fi)
+                             fuse_ino_t ino, unsigned num,
+			     struct fuse_removemapping_in *argsp,
+			     struct fuse_file_info *fi)
 {
         VhostUserFSSlaveMsg msg = { 0 };
 	int ret = 0;
 
-	msg.len[0] = len;
-	msg.c_offset[0] = moffset;
-        if (fuse_virtio_unmap(se, &msg)) {
-                fprintf(stderr,
-                        "%s: unmap over virtio failed "
-                        "(offset=0x%lx, len=0x%lx)\n", __func__, moffset, len);
-                ret = EINVAL;
-        }
+	for (int i = 0; num > 0; i++, argsp++) {
+		msg.len[i] = argsp->len;
+		msg.c_offset[i] = argsp->moffset;
+
+		if (--num == 0 || i == VHOST_USER_FS_SLAVE_ENTRIES - 1) {
+			if (fuse_virtio_unmap(se, &msg)) {
+				fprintf(stderr,
+					"%s: unmap over virtio failed "
+					"(offset=0x%lx, len=0x%lx)\n", __func__, argsp->moffset, argsp->len);
+				ret = EINVAL;
+				break;
+			}
+			if (num > 0) {
+				i = 0;
+				memset(&msg, 0, sizeof(msg));
+			}
+		}
+	}
 
 	fuse_reply_err(req, ret);
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries
  2019-05-13 14:41 [Virtio-fs] [PATCH] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries Peng Tao
@ 2019-05-22 11:26 ` Dr. David Alan Gilbert
  2019-05-22 11:31   ` Miklos Szeredi
  2019-05-24  4:21   ` Tao Peng
  0 siblings, 2 replies; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2019-05-22 11:26 UTC (permalink / raw)
  To: Peng Tao, mszeredi, vgoyal; +Cc: virtio-fs

* Peng Tao (tao.peng@linux.alibaba.com) wrote:
> The fuse wire protocol is changed so that we can unmap multiple
> mappings in a single call.
> 
> Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>

Hi,
  Thanks for the patch and apologies for not responding sooner

> ---
>  contrib/virtiofsd/fuse_kernel.h    |  9 +++++++--
>  contrib/virtiofsd/fuse_lowlevel.c  | 21 ++++++++++++++------
>  contrib/virtiofsd/fuse_lowlevel.h  |  5 +++--
>  contrib/virtiofsd/passthrough_ll.c | 31 ++++++++++++++++++++----------
>  4 files changed, 46 insertions(+), 20 deletions(-)
> 
> diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
> index ce46046a4f..093cacff02 100644
> --- a/contrib/virtiofsd/fuse_kernel.h
> +++ b/contrib/virtiofsd/fuse_kernel.h
> @@ -830,9 +830,14 @@ struct fuse_setupmapping_out {
>          uint64_t        len[FUSE_SETUPMAPPING_ENTRIES];
>  };
>  
> -struct fuse_removemapping_in {
> +struct fuse_removemapping_in_header {
>          /* An already open handle */
> -	uint64_t	fh;
> +        uint64_t        fh;
> +        /* number of fuse_removemapping_in follows */
> +        unsigned        num;

I think all fields in fuse structures are fixed length - e.g. uint32_t
or uint64_t.

> +};
> +
> +struct fuse_removemapping_in {
>          /* Offset into the dax to start the unmapping */
>          uint64_t        moffset;
>          /* Length of mapping required */

Miklos: Does this make sense for a fuse structure? It's that header
followed by 'num' of fuse_removemapping_in.

   
This is an interface change so we do have to coordinate with your kernel
change.

> diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
> index 111c6e1636..4619865c2c 100644
> --- a/contrib/virtiofsd/fuse_lowlevel.c
> +++ b/contrib/virtiofsd/fuse_lowlevel.c
> @@ -1907,21 +1907,30 @@ static void do_setupmapping(fuse_req_t req, fuse_ino_t nodeid,
>  static void do_removemapping(fuse_req_t req, fuse_ino_t nodeid,
>  			     struct fuse_mbuf_iter *iter)
>  {
> -	struct fuse_removemapping_in *arg;
> +	struct fuse_removemapping_in_header *header;
> +	struct fuse_removemapping_in *argp;
>          struct fuse_file_info fi;
>  
> -	arg = fuse_mbuf_iter_advance(iter, sizeof(*arg));
> -	if (!arg) {
> +	header = fuse_mbuf_iter_advance(iter, sizeof(*header));
> +	if (!header || header->num <= 0) {
> +		fprintf(stderr, "do_removemapping: invalid header %p\n", header);
> +		fuse_reply_err(req, EINVAL);
> +		return;
> +	}
> +
> +	argp = fuse_mbuf_iter_advance(iter, header->num * sizeof(*argp));
> +	if (!argp) {
> +		fprintf(stderr, "do_removemapping: invalid in, expected %d * %ld, has %ld - %ld\n",
> +				header->num, sizeof(*argp), iter->size, iter->pos);
>  		fuse_reply_err(req, EINVAL);
>  		return;
>  	}
>          memset(&fi, 0, sizeof(fi));
> -	fi.fh = arg->fh;
> +	fi.fh = header->fh;
>  
>  	if (req->se->op.removemapping)
> -		req->se->op.removemapping(req, req->se, nodeid, arg->moffset,
> -                                          arg->len, &fi);
> +		req->se->op.removemapping(req, req->se, nodeid, header->num, argp, &fi);
>  	else
>  		fuse_reply_err(req, ENOSYS);
>  }
> diff --git a/contrib/virtiofsd/fuse_lowlevel.h b/contrib/virtiofsd/fuse_lowlevel.h
> index 6a001c4605..b2b8747074 100644
> --- a/contrib/virtiofsd/fuse_lowlevel.h
> +++ b/contrib/virtiofsd/fuse_lowlevel.h
> @@ -23,6 +23,7 @@
>  #endif
>  
>  #include "fuse_common.h"
> +#include "fuse_kernel.h"
>  
>  #include <utime.h>
>  #include <fcntl.h>
> @@ -1200,8 +1201,8 @@ struct fuse_lowlevel_ops {
>           * TODO
>           */
>          void (*removemapping) (fuse_req_t req, struct fuse_session *se,
> -                               fuse_ino_t ino, uint64_t moffset,
> -                               uint64_t len, struct fuse_file_info *fi);
> +                               fuse_ino_t ino, unsigned num,
> +                               struct fuse_removemapping_in *args, struct fuse_file_info *fi);
>  };
>  
>  /**
> diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
> index 1ddb68f1db..96d5468ead 100644
> --- a/contrib/virtiofsd/passthrough_ll.c
> +++ b/contrib/virtiofsd/passthrough_ll.c
> @@ -1925,20 +1925,31 @@ err:
>  }
>  
>  static void lo_removemapping(fuse_req_t req, struct fuse_session *se,
> -                             fuse_ino_t ino, uint64_t moffset,
> -                             uint64_t len, struct fuse_file_info *fi)
> +                             fuse_ino_t ino, unsigned num,
> +			     struct fuse_removemapping_in *argsp,
> +			     struct fuse_file_info *fi)
>  {
>          VhostUserFSSlaveMsg msg = { 0 };
>  	int ret = 0;
>  
> -	msg.len[0] = len;
> -	msg.c_offset[0] = moffset;
> -        if (fuse_virtio_unmap(se, &msg)) {
> -                fprintf(stderr,
> -                        "%s: unmap over virtio failed "
> -                        "(offset=0x%lx, len=0x%lx)\n", __func__, moffset, len);
> -                ret = EINVAL;
> -        }
> +	for (int i = 0; num > 0; i++, argsp++) {

Probably best to make 'i' unsigned as well.

Dave

> +		msg.len[i] = argsp->len;
> +		msg.c_offset[i] = argsp->moffset;
> +
> +		if (--num == 0 || i == VHOST_USER_FS_SLAVE_ENTRIES - 1) {
> +			if (fuse_virtio_unmap(se, &msg)) {
> +				fprintf(stderr,
> +					"%s: unmap over virtio failed "
> +					"(offset=0x%lx, len=0x%lx)\n", __func__, argsp->moffset, argsp->len);
> +				ret = EINVAL;
> +				break;
> +			}
> +			if (num > 0) {
> +				i = 0;
> +				memset(&msg, 0, sizeof(msg));
> +			}
> +		}
> +	}
>  
>  	fuse_reply_err(req, ret);
>  }
> -- 
> 2.17.1
> 
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries
  2019-05-22 11:26 ` Dr. David Alan Gilbert
@ 2019-05-22 11:31   ` Miklos Szeredi
  2019-05-22 11:38     ` Dr. David Alan Gilbert
  2019-05-24  4:21   ` Tao Peng
  1 sibling, 1 reply; 5+ messages in thread
From: Miklos Szeredi @ 2019-05-22 11:31 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-fs, Peng Tao, Vivek Goyal

On Wed, May 22, 2019 at 1:27 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Peng Tao (tao.peng@linux.alibaba.com) wrote:
> > The fuse wire protocol is changed so that we can unmap multiple
> > mappings in a single call.
> >
> > Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
>
> Hi,
>   Thanks for the patch and apologies for not responding sooner
>
> > ---
> >  contrib/virtiofsd/fuse_kernel.h    |  9 +++++++--
> >  contrib/virtiofsd/fuse_lowlevel.c  | 21 ++++++++++++++------
> >  contrib/virtiofsd/fuse_lowlevel.h  |  5 +++--
> >  contrib/virtiofsd/passthrough_ll.c | 31 ++++++++++++++++++++----------
> >  4 files changed, 46 insertions(+), 20 deletions(-)
> >
> > diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
> > index ce46046a4f..093cacff02 100644
> > --- a/contrib/virtiofsd/fuse_kernel.h
> > +++ b/contrib/virtiofsd/fuse_kernel.h
> > @@ -830,9 +830,14 @@ struct fuse_setupmapping_out {
> >          uint64_t        len[FUSE_SETUPMAPPING_ENTRIES];
> >  };
> >
> > -struct fuse_removemapping_in {
> > +struct fuse_removemapping_in_header {
> >          /* An already open handle */
> > -     uint64_t        fh;
> > +        uint64_t        fh;
> > +        /* number of fuse_removemapping_in follows */
> > +        unsigned        num;
>
> I think all fields in fuse structures are fixed length - e.g. uint32_t
> or uint64_t.
>
> > +};
> > +
> > +struct fuse_removemapping_in {
> >          /* Offset into the dax to start the unmapping */
> >          uint64_t        moffset;
> >          /* Length of mapping required */
>
> Miklos: Does this make sense for a fuse structure? It's that header
> followed by 'num' of fuse_removemapping_in.

There's one example of that already: fuse_batch_forget_in followed by
'count' number of fuse_forget_one.

Thanks,
Miklos


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries
  2019-05-22 11:31   ` Miklos Szeredi
@ 2019-05-22 11:38     ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 5+ messages in thread
From: Dr. David Alan Gilbert @ 2019-05-22 11:38 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: virtio-fs, Peng Tao, Vivek Goyal

* Miklos Szeredi (mszeredi@redhat.com) wrote:
> On Wed, May 22, 2019 at 1:27 PM Dr. David Alan Gilbert
> <dgilbert@redhat.com> wrote:
> >
> > * Peng Tao (tao.peng@linux.alibaba.com) wrote:
> > > The fuse wire protocol is changed so that we can unmap multiple
> > > mappings in a single call.
> > >
> > > Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
> >
> > Hi,
> >   Thanks for the patch and apologies for not responding sooner
> >
> > > ---
> > >  contrib/virtiofsd/fuse_kernel.h    |  9 +++++++--
> > >  contrib/virtiofsd/fuse_lowlevel.c  | 21 ++++++++++++++------
> > >  contrib/virtiofsd/fuse_lowlevel.h  |  5 +++--
> > >  contrib/virtiofsd/passthrough_ll.c | 31 ++++++++++++++++++++----------
> > >  4 files changed, 46 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
> > > index ce46046a4f..093cacff02 100644
> > > --- a/contrib/virtiofsd/fuse_kernel.h
> > > +++ b/contrib/virtiofsd/fuse_kernel.h
> > > @@ -830,9 +830,14 @@ struct fuse_setupmapping_out {
> > >          uint64_t        len[FUSE_SETUPMAPPING_ENTRIES];
> > >  };
> > >
> > > -struct fuse_removemapping_in {
> > > +struct fuse_removemapping_in_header {
> > >          /* An already open handle */
> > > -     uint64_t        fh;
> > > +        uint64_t        fh;
> > > +        /* number of fuse_removemapping_in follows */
> > > +        unsigned        num;
> >
> > I think all fields in fuse structures are fixed length - e.g. uint32_t
> > or uint64_t.
> >
> > > +};
> > > +
> > > +struct fuse_removemapping_in {
> > >          /* Offset into the dax to start the unmapping */
> > >          uint64_t        moffset;
> > >          /* Length of mapping required */
> >
> > Miklos: Does this make sense for a fuse structure? It's that header
> > followed by 'num' of fuse_removemapping_in.
> 
> There's one example of that already: fuse_batch_forget_in followed by
> 'count' number of fuse_forget_one.

OK, it would be great to follow the same naming pattern for this.

Dave

> Thanks,
> Miklos
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Virtio-fs] [PATCH] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries
  2019-05-22 11:26 ` Dr. David Alan Gilbert
  2019-05-22 11:31   ` Miklos Szeredi
@ 2019-05-24  4:21   ` Tao Peng
  1 sibling, 0 replies; 5+ messages in thread
From: Tao Peng @ 2019-05-24  4:21 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: virtio-fs, mszeredi, Peng Tao, Vivek Goyal

On Wed, May 22, 2019 at 7:27 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Peng Tao (tao.peng@linux.alibaba.com) wrote:
> > The fuse wire protocol is changed so that we can unmap multiple
> > mappings in a single call.
> >
> > Signed-off-by: Peng Tao <tao.peng@linux.alibaba.com>
>
> Hi,
>   Thanks for the patch and apologies for not responding sooner
>
> > ---
> >  contrib/virtiofsd/fuse_kernel.h    |  9 +++++++--
> >  contrib/virtiofsd/fuse_lowlevel.c  | 21 ++++++++++++++------
> >  contrib/virtiofsd/fuse_lowlevel.h  |  5 +++--
> >  contrib/virtiofsd/passthrough_ll.c | 31 ++++++++++++++++++++----------
> >  4 files changed, 46 insertions(+), 20 deletions(-)
> >
> > diff --git a/contrib/virtiofsd/fuse_kernel.h b/contrib/virtiofsd/fuse_kernel.h
> > index ce46046a4f..093cacff02 100644
> > --- a/contrib/virtiofsd/fuse_kernel.h
> > +++ b/contrib/virtiofsd/fuse_kernel.h
> > @@ -830,9 +830,14 @@ struct fuse_setupmapping_out {
> >          uint64_t        len[FUSE_SETUPMAPPING_ENTRIES];
> >  };
> >
> > -struct fuse_removemapping_in {
> > +struct fuse_removemapping_in_header {
> >          /* An already open handle */
> > -     uint64_t        fh;
> > +        uint64_t        fh;
> > +        /* number of fuse_removemapping_in follows */
> > +        unsigned        num;
>
> I think all fields in fuse structures are fixed length - e.g. uint32_t
> or uint64_t.
>
> > +};
> > +
> > +struct fuse_removemapping_in {
> >          /* Offset into the dax to start the unmapping */
> >          uint64_t        moffset;
> >          /* Length of mapping required */
>
> Miklos: Does this make sense for a fuse structure? It's that header
> followed by 'num' of fuse_removemapping_in.
>
>
> This is an interface change so we do have to coordinate with your kernel
> change.
>
Yes, please see "[Virtio-fs] [PATCH-v2 2/2] virtiofs:
FUSE_REMOVEMAPPING support multiple removing multiple entries" for the
kernel part of the change.

> > diff --git a/contrib/virtiofsd/fuse_lowlevel.c b/contrib/virtiofsd/fuse_lowlevel.c
> > index 111c6e1636..4619865c2c 100644
> > --- a/contrib/virtiofsd/fuse_lowlevel.c
> > +++ b/contrib/virtiofsd/fuse_lowlevel.c
> > @@ -1907,21 +1907,30 @@ static void do_setupmapping(fuse_req_t req, fuse_ino_t nodeid,
> >  static void do_removemapping(fuse_req_t req, fuse_ino_t nodeid,
> >                            struct fuse_mbuf_iter *iter)
> >  {
> > -     struct fuse_removemapping_in *arg;
> > +     struct fuse_removemapping_in_header *header;
> > +     struct fuse_removemapping_in *argp;
> >          struct fuse_file_info fi;
> >
> > -     arg = fuse_mbuf_iter_advance(iter, sizeof(*arg));
> > -     if (!arg) {
> > +     header = fuse_mbuf_iter_advance(iter, sizeof(*header));
> > +     if (!header || header->num <= 0) {
> > +             fprintf(stderr, "do_removemapping: invalid header %p\n", header);
> > +             fuse_reply_err(req, EINVAL);
> > +             return;
> > +     }
> > +
> > +     argp = fuse_mbuf_iter_advance(iter, header->num * sizeof(*argp));
> > +     if (!argp) {
> > +             fprintf(stderr, "do_removemapping: invalid in, expected %d * %ld, has %ld - %ld\n",
> > +                             header->num, sizeof(*argp), iter->size, iter->pos);
> >               fuse_reply_err(req, EINVAL);
> >               return;
> >       }
> >          memset(&fi, 0, sizeof(fi));
> > -     fi.fh = arg->fh;
> > +     fi.fh = header->fh;
> >
> >       if (req->se->op.removemapping)
> > -             req->se->op.removemapping(req, req->se, nodeid, arg->moffset,
> > -                                          arg->len, &fi);
> > +             req->se->op.removemapping(req, req->se, nodeid, header->num, argp, &fi);
> >       else
> >               fuse_reply_err(req, ENOSYS);
> >  }
> > diff --git a/contrib/virtiofsd/fuse_lowlevel.h b/contrib/virtiofsd/fuse_lowlevel.h
> > index 6a001c4605..b2b8747074 100644
> > --- a/contrib/virtiofsd/fuse_lowlevel.h
> > +++ b/contrib/virtiofsd/fuse_lowlevel.h
> > @@ -23,6 +23,7 @@
> >  #endif
> >
> >  #include "fuse_common.h"
> > +#include "fuse_kernel.h"
> >
> >  #include <utime.h>
> >  #include <fcntl.h>
> > @@ -1200,8 +1201,8 @@ struct fuse_lowlevel_ops {
> >           * TODO
> >           */
> >          void (*removemapping) (fuse_req_t req, struct fuse_session *se,
> > -                               fuse_ino_t ino, uint64_t moffset,
> > -                               uint64_t len, struct fuse_file_info *fi);
> > +                               fuse_ino_t ino, unsigned num,
> > +                               struct fuse_removemapping_in *args, struct fuse_file_info *fi);
> >  };
> >
> >  /**
> > diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
> > index 1ddb68f1db..96d5468ead 100644
> > --- a/contrib/virtiofsd/passthrough_ll.c
> > +++ b/contrib/virtiofsd/passthrough_ll.c
> > @@ -1925,20 +1925,31 @@ err:
> >  }
> >
> >  static void lo_removemapping(fuse_req_t req, struct fuse_session *se,
> > -                             fuse_ino_t ino, uint64_t moffset,
> > -                             uint64_t len, struct fuse_file_info *fi)
> > +                             fuse_ino_t ino, unsigned num,
> > +                          struct fuse_removemapping_in *argsp,
> > +                          struct fuse_file_info *fi)
> >  {
> >          VhostUserFSSlaveMsg msg = { 0 };
> >       int ret = 0;
> >
> > -     msg.len[0] = len;
> > -     msg.c_offset[0] = moffset;
> > -        if (fuse_virtio_unmap(se, &msg)) {
> > -                fprintf(stderr,
> > -                        "%s: unmap over virtio failed "
> > -                        "(offset=0x%lx, len=0x%lx)\n", __func__, moffset, len);
> > -                ret = EINVAL;
> > -        }
> > +     for (int i = 0; num > 0; i++, argsp++) {
>
> Probably best to make 'i' unsigned as well.
>
Thanks for reviewing. I'll change the patch as suggested.

Cheers,
Tao

-- 
bergwolf@hyper.sh


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-05-24  4:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-13 14:41 [Virtio-fs] [PATCH] virtiofsd: make FUSE_REMOVEMAPPING support multiple entries Peng Tao
2019-05-22 11:26 ` Dr. David Alan Gilbert
2019-05-22 11:31   ` Miklos Szeredi
2019-05-22 11:38     ` Dr. David Alan Gilbert
2019-05-24  4:21   ` Tao Peng

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.