diff mbox series

[v4,4/9] virtiofsd: Extend size of fuse_conn_info->capable and ->want fields

Message ID 20220124212455.83968-5-vgoyal@redhat.com (mailing list archive)
State New, archived
Headers show
Series virtiofsd: Add support for file security context at creation | expand

Commit Message

Vivek Goyal Jan. 24, 2022, 9:24 p.m. UTC
->capable keeps track of what capabilities kernel supports and ->wants keep
track of what capabilities filesytem wants.

Right now these fields are 32bit in size. But now fuse has run out of
bits and capabilities can now have bit number which are higher than 31.

That means 32 bit fields are not suffcient anymore. Increase size to 64
bit so that we can add newer capabilities and still be able to use existing
code to check and set the capabilities.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
 tools/virtiofsd/fuse_common.h   | 4 ++--
 tools/virtiofsd/fuse_lowlevel.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

Comments

Dr. David Alan Gilbert Jan. 27, 2022, 5:53 p.m. UTC | #1
* Vivek Goyal (vgoyal@redhat.com) wrote:
> ->capable keeps track of what capabilities kernel supports and ->wants keep
> track of what capabilities filesytem wants.
> 
> Right now these fields are 32bit in size. But now fuse has run out of
> bits and capabilities can now have bit number which are higher than 31.
> 
> That means 32 bit fields are not suffcient anymore. Increase size to 64
> bit so that we can add newer capabilities and still be able to use existing
> code to check and set the capabilities.
> 
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> ---
>  tools/virtiofsd/fuse_common.h   | 4 ++--
>  tools/virtiofsd/fuse_lowlevel.c | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
> index 0c2665b977..6f8a988202 100644
> --- a/tools/virtiofsd/fuse_common.h
> +++ b/tools/virtiofsd/fuse_common.h
> @@ -439,7 +439,7 @@ struct fuse_conn_info {
>      /**
>       * Capability flags that the kernel supports (read-only)
>       */
> -    unsigned capable;
> +    uint64_t capable;
>  
>      /**
>       * Capability flags that the filesystem wants to enable.
> @@ -447,7 +447,7 @@ struct fuse_conn_info {
>       * libfuse attempts to initialize this field with
>       * reasonable default values before calling the init() handler.
>       */
> -    unsigned want;
> +    uint64_t want;
>  
>      /**
>       * Maximum number of pending "background" requests. A
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index c3af5ede08..f3f5e70be6 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -2063,7 +2063,7 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
>      if (se->conn.want & (~se->conn.capable)) {
>          fuse_log(FUSE_LOG_ERR,
>                   "fuse: error: filesystem requested capabilities "
> -                 "0x%x that are not supported by kernel, aborting.\n",
> +                 "0x%lx that are not supported by kernel, aborting.\n",

I think this will be OK in practice (need to check 32 bit); but weren't
you using llx in the last patch?



Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Dave

>                   se->conn.want & (~se->conn.capable));
>          fuse_reply_err(req, EPROTO);
>          se->error = -EPROTO;
> -- 
> 2.31.1
>
Vivek Goyal Jan. 27, 2022, 6:31 p.m. UTC | #2
On Thu, Jan 27, 2022 at 05:53:20PM +0000, Dr. David Alan Gilbert wrote:
> * Vivek Goyal (vgoyal@redhat.com) wrote:
> > ->capable keeps track of what capabilities kernel supports and ->wants keep
> > track of what capabilities filesytem wants.
> > 
> > Right now these fields are 32bit in size. But now fuse has run out of
> > bits and capabilities can now have bit number which are higher than 31.
> > 
> > That means 32 bit fields are not suffcient anymore. Increase size to 64
> > bit so that we can add newer capabilities and still be able to use existing
> > code to check and set the capabilities.
> > 
> > Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
> > ---
> >  tools/virtiofsd/fuse_common.h   | 4 ++--
> >  tools/virtiofsd/fuse_lowlevel.c | 2 +-
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
> > index 0c2665b977..6f8a988202 100644
> > --- a/tools/virtiofsd/fuse_common.h
> > +++ b/tools/virtiofsd/fuse_common.h
> > @@ -439,7 +439,7 @@ struct fuse_conn_info {
> >      /**
> >       * Capability flags that the kernel supports (read-only)
> >       */
> > -    unsigned capable;
> > +    uint64_t capable;
> >  
> >      /**
> >       * Capability flags that the filesystem wants to enable.
> > @@ -447,7 +447,7 @@ struct fuse_conn_info {
> >       * libfuse attempts to initialize this field with
> >       * reasonable default values before calling the init() handler.
> >       */
> > -    unsigned want;
> > +    uint64_t want;
> >  
> >      /**
> >       * Maximum number of pending "background" requests. A
> > diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> > index c3af5ede08..f3f5e70be6 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.c
> > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > @@ -2063,7 +2063,7 @@ static void do_init(fuse_req_t req, fuse_ino_t nodeid,
> >      if (se->conn.want & (~se->conn.capable)) {
> >          fuse_log(FUSE_LOG_ERR,
> >                   "fuse: error: filesystem requested capabilities "
> > -                 "0x%x that are not supported by kernel, aborting.\n",
> > +                 "0x%lx that are not supported by kernel, aborting.\n",
> 
> I think this will be OK in practice (need to check 32 bit); but weren't
> you using llx in the last patch?

Probably I should use %llx so that it works fine on 32 bit. Will change
it.

Vivek

> 
> 
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Dave
> 
> >                   se->conn.want & (~se->conn.capable));
> >          fuse_reply_err(req, EPROTO);
> >          se->error = -EPROTO;
> > -- 
> > 2.31.1
> > 
> -- 
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
diff mbox series

Patch

diff --git a/tools/virtiofsd/fuse_common.h b/tools/virtiofsd/fuse_common.h
index 0c2665b977..6f8a988202 100644
--- a/tools/virtiofsd/fuse_common.h
+++ b/tools/virtiofsd/fuse_common.h
@@ -439,7 +439,7 @@  struct fuse_conn_info {
     /**
      * Capability flags that the kernel supports (read-only)
      */
-    unsigned capable;
+    uint64_t capable;
 
     /**
      * Capability flags that the filesystem wants to enable.
@@ -447,7 +447,7 @@  struct fuse_conn_info {
      * libfuse attempts to initialize this field with
      * reasonable default values before calling the init() handler.
      */
-    unsigned want;
+    uint64_t want;
 
     /**
      * Maximum number of pending "background" requests. A
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index c3af5ede08..f3f5e70be6 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -2063,7 +2063,7 @@  static void do_init(fuse_req_t req, fuse_ino_t nodeid,
     if (se->conn.want & (~se->conn.capable)) {
         fuse_log(FUSE_LOG_ERR,
                  "fuse: error: filesystem requested capabilities "
-                 "0x%x that are not supported by kernel, aborting.\n",
+                 "0x%lx that are not supported by kernel, aborting.\n",
                  se->conn.want & (~se->conn.capable));
         fuse_reply_err(req, EPROTO);
         se->error = -EPROTO;