[PATCH RFC v2 2/2] fuse: add an implementation of open+getattr

Horst Birthelmer posted 2 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH RFC v2 2/2] fuse: add an implementation of open+getattr
Posted by Horst Birthelmer 1 month, 2 weeks ago
The discussion about compound commands in fuse was
started over an argument to add a new operation that
will open a file and return its attributes in the same operation.

Here is a demonstration of that use case with compound commands.

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
 fs/fuse/file.c   | 125 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 fs/fuse/fuse_i.h |   6 ++-
 fs/fuse/inode.c  |   6 +++
 fs/fuse/ioctl.c  |   2 +-
 4 files changed, 121 insertions(+), 18 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 01bc894e9c2b..507b4c4ba257 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -126,8 +126,84 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
 	}
 }
 
+static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
+				int flags, int opcode,
+				struct fuse_file *ff,
+				struct fuse_attr_out *outattrp,
+				struct fuse_open_out *outopenp)
+{
+	struct fuse_compound_req *compound;
+	struct fuse_args open_args = {}, getattr_args = {};
+	struct fuse_open_in open_in = {};
+	struct fuse_getattr_in getattr_in = {};
+	int err;
+
+	/* Build compound request with flag to execute in the given order */
+	compound = fuse_compound_alloc(fm, 0);
+	if (IS_ERR(compound))
+		return PTR_ERR(compound);
+
+	/* Add OPEN */
+	open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
+	if (!fm->fc->atomic_o_trunc)
+		open_in.flags &= ~O_TRUNC;
+
+	if (fm->fc->handle_killpriv_v2 &&
+	    (open_in.flags & O_TRUNC) && !capable(CAP_FSETID)) {
+		open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
+	}
+	open_args.opcode = opcode;
+	open_args.nodeid = nodeid;
+	open_args.in_numargs = 1;
+	open_args.in_args[0].size = sizeof(open_in);
+	open_args.in_args[0].value = &open_in;
+	open_args.out_numargs = 1;
+	open_args.out_args[0].size = sizeof(struct fuse_open_out);
+	open_args.out_args[0].value = outopenp;
+
+	err = fuse_compound_add(compound, &open_args);
+	if (err)
+		goto out;
+
+	/* Add GETATTR */
+	getattr_args.opcode = FUSE_GETATTR;
+	getattr_args.nodeid = nodeid;
+	getattr_args.in_numargs = 1;
+	getattr_args.in_args[0].size = sizeof(getattr_in);
+	getattr_args.in_args[0].value = &getattr_in;
+	getattr_args.out_numargs = 1;
+	getattr_args.out_args[0].size = sizeof(struct fuse_attr_out);
+	getattr_args.out_args[0].value = outattrp;
+
+	err = fuse_compound_add(compound, &getattr_args);
+	if (err)
+		goto out;
+
+	err = fuse_compound_send(compound);
+	if (err)
+		goto out;
+
+	/* Check if the OPEN operation succeeded */
+	err = fuse_compound_get_error(compound, 0);
+	if (err)
+		goto out;
+
+	/* Check if the GETATTR operation succeeded */
+	err = fuse_compound_get_error(compound, 1);
+	if (err)
+		goto out;
+
+	ff->fh = outopenp->fh;
+	ff->open_flags = outopenp->open_flags;
+
+out:
+	fuse_compound_free(compound);
+	return err;
+}
+
 struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
-				 unsigned int open_flags, bool isdir)
+				struct inode *inode,
+				unsigned int open_flags, bool isdir)
 {
 	struct fuse_conn *fc = fm->fc;
 	struct fuse_file *ff;
@@ -153,23 +229,41 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 	if (open) {
 		/* Store outarg for fuse_finish_open() */
 		struct fuse_open_out *outargp = &ff->args->open_outarg;
-		int err;
+		int err = -ENOSYS;
+
+		if (inode && fc->compound_open_getattr) {
+			struct fuse_attr_out attr_outarg;
+			err = fuse_compound_open_getattr(fm, nodeid, open_flags,
+							opcode, ff, &attr_outarg, outargp);
+			if (!err)
+				fuse_change_attributes(inode, &attr_outarg.attr, NULL,
+						       ATTR_TIMEOUT(&attr_outarg),
+						       fuse_get_attr_version(fc));
+		}
+		if (err == -ENOSYS) {
+			err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
 
-		err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
-		if (!err) {
-			ff->fh = outargp->fh;
-			ff->open_flags = outargp->open_flags;
-		} else if (err != -ENOSYS) {
-			fuse_file_free(ff);
-			return ERR_PTR(err);
-		} else {
-			if (isdir) {
+			if (!err) {
+				ff->fh = outargp->fh;
+				ff->open_flags = outargp->open_flags;
+			}
+		}
+
+		if (err) {
+			if (err != -ENOSYS) {
+				/* err is not ENOSYS */
+				fuse_file_free(ff);
+				return ERR_PTR(err);
+			} else {
 				/* No release needed */
 				kfree(ff->args);
 				ff->args = NULL;
-				fc->no_opendir = 1;
-			} else {
-				fc->no_open = 1;
+
+				/* we don't have open */
+				if (isdir)
+					fc->no_opendir = 1;
+				else
+					fc->no_open = 1;
 			}
 		}
 	}
@@ -185,11 +279,10 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
 int fuse_do_open(struct fuse_mount *fm, u64 nodeid, struct file *file,
 		 bool isdir)
 {
-	struct fuse_file *ff = fuse_file_open(fm, nodeid, file->f_flags, isdir);
+	struct fuse_file *ff = fuse_file_open(fm, nodeid, file_inode(file), file->f_flags, isdir);
 
 	if (!IS_ERR(ff))
 		file->private_data = ff;
-
 	return PTR_ERR_OR_ZERO(ff);
 }
 EXPORT_SYMBOL_GPL(fuse_do_open);
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 86253517f59b..98af019037c3 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -924,6 +924,9 @@ struct fuse_conn {
 	/* Use io_uring for communication */
 	unsigned int io_uring;
 
+	/* Does the filesystem support compound operations? */
+	unsigned int compound_open_getattr:1;
+
 	/** Maximum stack depth for passthrough backing files */
 	int max_stack_depth;
 
@@ -1557,7 +1560,8 @@ void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
 
 /* file.c */
 struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
-				 unsigned int open_flags, bool isdir);
+								struct inode *inode,
+								unsigned int open_flags, bool isdir);
 void fuse_file_release(struct inode *inode, struct fuse_file *ff,
 		       unsigned int open_flags, fl_owner_t id, bool isdir);
 
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 819e50d66622..a5fd721be96d 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -991,6 +991,12 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	fc->blocked = 0;
 	fc->initialized = 0;
 	fc->connected = 1;
+
+	/* pretend fuse server supports compound operations
+	 * until it tells us otherwise.
+	 */
+	fc->compound_open_getattr = 1;
+
 	atomic64_set(&fc->attr_version, 1);
 	atomic64_set(&fc->evict_ctr, 1);
 	get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index fdc175e93f74..07a02e47b2c3 100644
--- a/fs/fuse/ioctl.c
+++ b/fs/fuse/ioctl.c
@@ -494,7 +494,7 @@ static struct fuse_file *fuse_priv_ioctl_prepare(struct inode *inode)
 	if (!S_ISREG(inode->i_mode) && !isdir)
 		return ERR_PTR(-ENOTTY);
 
-	return fuse_file_open(fm, get_node_id(inode), O_RDONLY, isdir);
+	return fuse_file_open(fm, get_node_id(inode), NULL, O_RDONLY, isdir);
 }
 
 static void fuse_priv_ioctl_cleanup(struct inode *inode, struct fuse_file *ff)

-- 
2.51.0
Re: [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr
Posted by Joanne Koong 1 month ago
On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer
<hbirthelmer@googlemail.com> wrote:
>
> The discussion about compound commands in fuse was
> started over an argument to add a new operation that
> will open a file and return its attributes in the same operation.
>
> Here is a demonstration of that use case with compound commands.
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
>  fs/fuse/file.c   | 125 ++++++++++++++++++++++++++++++++++++++++++++++++-------
>  fs/fuse/fuse_i.h |   6 ++-
>  fs/fuse/inode.c  |   6 +++
>  fs/fuse/ioctl.c  |   2 +-
>  4 files changed, 121 insertions(+), 18 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 01bc894e9c2b..507b4c4ba257 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -126,8 +126,84 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
>         }
>  }
>
> +static int fuse_compound_open_getattr(struct fuse_mount *fm, u64 nodeid,
> +                               int flags, int opcode,
> +                               struct fuse_file *ff,
> +                               struct fuse_attr_out *outattrp,
> +                               struct fuse_open_out *outopenp)
> +{
> +       struct fuse_compound_req *compound;
> +       struct fuse_args open_args = {}, getattr_args = {};
> +       struct fuse_open_in open_in = {};
> +       struct fuse_getattr_in getattr_in = {};
> +       int err;
> +
> +       /* Build compound request with flag to execute in the given order */
> +       compound = fuse_compound_alloc(fm, 0);
> +       if (IS_ERR(compound))
> +               return PTR_ERR(compound);
> +
> +       /* Add OPEN */
> +       open_in.flags = flags & ~(O_CREAT | O_EXCL | O_NOCTTY);
> +       if (!fm->fc->atomic_o_trunc)
> +               open_in.flags &= ~O_TRUNC;
> +
> +       if (fm->fc->handle_killpriv_v2 &&
> +           (open_in.flags & O_TRUNC) && !capable(CAP_FSETID)) {
> +               open_in.open_flags |= FUSE_OPEN_KILL_SUIDGID;
> +       }
> +       open_args.opcode = opcode;
> +       open_args.nodeid = nodeid;
> +       open_args.in_numargs = 1;
> +       open_args.in_args[0].size = sizeof(open_in);
> +       open_args.in_args[0].value = &open_in;
> +       open_args.out_numargs = 1;
> +       open_args.out_args[0].size = sizeof(struct fuse_open_out);
> +       open_args.out_args[0].value = outopenp;
> +
> +       err = fuse_compound_add(compound, &open_args);
> +       if (err)
> +               goto out;
> +
> +       /* Add GETATTR */
> +       getattr_args.opcode = FUSE_GETATTR;
> +       getattr_args.nodeid = nodeid;
> +       getattr_args.in_numargs = 1;
> +       getattr_args.in_args[0].size = sizeof(getattr_in);
> +       getattr_args.in_args[0].value = &getattr_in;
> +       getattr_args.out_numargs = 1;
> +       getattr_args.out_args[0].size = sizeof(struct fuse_attr_out);
> +       getattr_args.out_args[0].value = outattrp;

I think things end up looking cleaner here (and above for the open
args) if the arg initialization logic gets abstracted into helper
functions, as fuse_do_getattr() and fuse_send_open() have pretty much
the exact same logic.

Thanks,
Joanne

> +
> +       err = fuse_compound_add(compound, &getattr_args);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_send(compound);
> +       if (err)
> +               goto out;
> +
> +       /* Check if the OPEN operation succeeded */
> +       err = fuse_compound_get_error(compound, 0);
> +       if (err)
> +               goto out;
> +
> +       /* Check if the GETATTR operation succeeded */
> +       err = fuse_compound_get_error(compound, 1);
> +       if (err)
> +               goto out;
> +
> +       ff->fh = outopenp->fh;
> +       ff->open_flags = outopenp->open_flags;
> +
> +out:
> +       fuse_compound_free(compound);
> +       return err;
> +}
> +
>  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> -                                unsigned int open_flags, bool isdir)
> +                               struct inode *inode,
> +                               unsigned int open_flags, bool isdir)
>  {
>         struct fuse_conn *fc = fm->fc;
>         struct fuse_file *ff;
> @@ -153,23 +229,41 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
>         if (open) {
>                 /* Store outarg for fuse_finish_open() */
>                 struct fuse_open_out *outargp = &ff->args->open_outarg;
> -               int err;
> +               int err = -ENOSYS;
> +
> +               if (inode && fc->compound_open_getattr) {
> +                       struct fuse_attr_out attr_outarg;
> +                       err = fuse_compound_open_getattr(fm, nodeid, open_flags,
> +                                                       opcode, ff, &attr_outarg, outargp);
> +                       if (!err)
> +                               fuse_change_attributes(inode, &attr_outarg.attr, NULL,
> +                                                      ATTR_TIMEOUT(&attr_outarg),
> +                                                      fuse_get_attr_version(fc));
> +               }
> +               if (err == -ENOSYS) {
> +                       err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
>
> -               err = fuse_send_open(fm, nodeid, open_flags, opcode, outargp);
> -               if (!err) {
> -                       ff->fh = outargp->fh;
> -                       ff->open_flags = outargp->open_flags;
> -               } else if (err != -ENOSYS) {
> -                       fuse_file_free(ff);
> -                       return ERR_PTR(err);
> -               } else {
> -                       if (isdir) {
> +                       if (!err) {
> +                               ff->fh = outargp->fh;
> +                               ff->open_flags = outargp->open_flags;
> +                       }
> +               }
> +
> +               if (err) {
> +                       if (err != -ENOSYS) {
> +                               /* err is not ENOSYS */
> +                               fuse_file_free(ff);
> +                               return ERR_PTR(err);
> +                       } else {
>                                 /* No release needed */
>                                 kfree(ff->args);
>                                 ff->args = NULL;
> -                               fc->no_opendir = 1;
> -                       } else {
> -                               fc->no_open = 1;
> +
> +                               /* we don't have open */
> +                               if (isdir)
> +                                       fc->no_opendir = 1;
> +                               else
> +                                       fc->no_open = 1;
>                         }
>                 }
>         }
> @@ -185,11 +279,10 @@ struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
>  int fuse_do_open(struct fuse_mount *fm, u64 nodeid, struct file *file,
>                  bool isdir)
>  {
> -       struct fuse_file *ff = fuse_file_open(fm, nodeid, file->f_flags, isdir);
> +       struct fuse_file *ff = fuse_file_open(fm, nodeid, file_inode(file), file->f_flags, isdir);
>
>         if (!IS_ERR(ff))
>                 file->private_data = ff;
> -
>         return PTR_ERR_OR_ZERO(ff);
>  }
>  EXPORT_SYMBOL_GPL(fuse_do_open);
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 86253517f59b..98af019037c3 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -924,6 +924,9 @@ struct fuse_conn {
>         /* Use io_uring for communication */
>         unsigned int io_uring;
>
> +       /* Does the filesystem support compound operations? */
> +       unsigned int compound_open_getattr:1;
> +
>         /** Maximum stack depth for passthrough backing files */
>         int max_stack_depth;
>
> @@ -1557,7 +1560,8 @@ void fuse_file_io_release(struct fuse_file *ff, struct inode *inode);
>
>  /* file.c */
>  struct fuse_file *fuse_file_open(struct fuse_mount *fm, u64 nodeid,
> -                                unsigned int open_flags, bool isdir);
> +                                                               struct inode *inode,
> +                                                               unsigned int open_flags, bool isdir);
>  void fuse_file_release(struct inode *inode, struct fuse_file *ff,
>                        unsigned int open_flags, fl_owner_t id, bool isdir);
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 819e50d66622..a5fd721be96d 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -991,6 +991,12 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
>         fc->blocked = 0;
>         fc->initialized = 0;
>         fc->connected = 1;
> +
> +       /* pretend fuse server supports compound operations
> +        * until it tells us otherwise.
> +        */
> +       fc->compound_open_getattr = 1;
> +
>         atomic64_set(&fc->attr_version, 1);
>         atomic64_set(&fc->evict_ctr, 1);
>         get_random_bytes(&fc->scramble_key, sizeof(fc->scramble_key));
> diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> index fdc175e93f74..07a02e47b2c3 100644
> --- a/fs/fuse/ioctl.c
> +++ b/fs/fuse/ioctl.c
> @@ -494,7 +494,7 @@ static struct fuse_file *fuse_priv_ioctl_prepare(struct inode *inode)
>         if (!S_ISREG(inode->i_mode) && !isdir)
>                 return ERR_PTR(-ENOTTY);
>
> -       return fuse_file_open(fm, get_node_id(inode), O_RDONLY, isdir);
> +       return fuse_file_open(fm, get_node_id(inode), NULL, O_RDONLY, isdir);
>  }
>
>  static void fuse_priv_ioctl_cleanup(struct inode *inode, struct fuse_file *ff)
>
> --
> 2.51.0
>
>
Re: Re: [PATCH RFC v2 2/2] fuse: add an implementation of open+getattr
Posted by Horst Birthelmer 1 month ago
On Tue, Jan 06, 2026 at 05:46:09PM -0800, Joanne Koong wrote:
> On Tue, Dec 23, 2025 at 2:13 PM Horst Birthelmer
> <hbirthelmer@googlemail.com> wrote:
> >
> > +       open_args.opcode = opcode;
> > +       open_args.nodeid = nodeid;
> > +       open_args.in_numargs = 1;
> > +       open_args.in_args[0].size = sizeof(open_in);
> > +       open_args.in_args[0].value = &open_in;
> > +       open_args.out_numargs = 1;
> > +       open_args.out_args[0].size = sizeof(struct fuse_open_out);
> > +       open_args.out_args[0].value = outopenp;
> > +
> > +       err = fuse_compound_add(compound, &open_args);
> > +       if (err)
> > +               goto out;
> > +
> > +       /* Add GETATTR */
> > +       getattr_args.opcode = FUSE_GETATTR;
> > +       getattr_args.nodeid = nodeid;
> > +       getattr_args.in_numargs = 1;
> > +       getattr_args.in_args[0].size = sizeof(getattr_in);
> > +       getattr_args.in_args[0].value = &getattr_in;
> > +       getattr_args.out_numargs = 1;
> > +       getattr_args.out_args[0].size = sizeof(struct fuse_attr_out);
> > +       getattr_args.out_args[0].value = outattrp;
> 
> I think things end up looking cleaner here (and above for the open
> args) if the arg initialization logic gets abstracted into helper
> functions, as fuse_do_getattr() and fuse_send_open() have pretty much
> the exact same logic.
> 
> Thanks,
> Joanne
> 
You are completely right.
Will change that in the next version,

Thanks,
Horst