[PATCH v4 3/3] fuse: add an implementation of open+getattr

Horst Birthelmer posted 3 patches 1 month ago
[PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Horst Birthelmer 1 month ago
From: Horst Birthelmer <hbirthelmer@ddn.com>

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   | 110 +++++++++++++++++++++++++++++++++++++++++++++++--------
 fs/fuse/fuse_i.h |   7 +++-
 fs/fuse/inode.c  |   6 +++
 fs/fuse/ioctl.c  |   2 +-
 4 files changed, 107 insertions(+), 18 deletions(-)

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 53744559455d..c0375b32967d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -152,8 +152,66 @@ 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 = {};
+	struct fuse_args getattr_args = {};
+	struct fuse_open_in open_in = {};
+	struct fuse_getattr_in getattr_in = {};
+	int err;
+
+	compound = fuse_compound_alloc(fm, 0);
+	if (IS_ERR(compound))
+		return PTR_ERR(compound);
+
+	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;
+
+	fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
+
+	err = fuse_compound_add(compound, &open_args);
+	if (err)
+		goto out;
+
+	fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
+
+	err = fuse_compound_add(compound, &getattr_args);
+	if (err)
+		goto out;
+
+	err = fuse_compound_send(compound);
+	if (err)
+		goto out;
+
+	err = fuse_compound_get_error(compound, 0);
+	if (err)
+		goto out;
+
+	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;
@@ -179,23 +237,44 @@ 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);
+			if (!err) {
+				ff->fh = outargp->fh;
+				ff->open_flags = outargp->open_flags;
+			}
+		}
 
-		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) {
+			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;
 			}
 		}
 	}
@@ -211,11 +290,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 98ea41f76623..e7828405e262 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;
 
@@ -1563,7 +1566,9 @@ 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 v4 3/3] fuse: add an implementation of open+getattr
Posted by Joanne Koong 3 weeks, 4 days ago
On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> 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   | 110 +++++++++++++++++++++++++++++++++++++++++++++++--------
>  fs/fuse/fuse_i.h |   7 +++-
>  fs/fuse/inode.c  |   6 +++
>  fs/fuse/ioctl.c  |   2 +-
>  4 files changed, 107 insertions(+), 18 deletions(-)
>
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 53744559455d..c0375b32967d 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -152,8 +152,66 @@ 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 = {};
> +       struct fuse_args getattr_args = {};
> +       struct fuse_open_in open_in = {};
> +       struct fuse_getattr_in getattr_in = {};
> +       int err;
> +
> +       compound = fuse_compound_alloc(fm, 0);
> +       if (IS_ERR(compound))
> +               return PTR_ERR(compound);
> +
> +       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;
> +
> +       fuse_open_args_fill(&open_args, nodeid, opcode, &open_in, outopenp);
> +
> +       err = fuse_compound_add(compound, &open_args);
> +       if (err)
> +               goto out;
> +
> +       fuse_getattr_args_fill(&getattr_args, nodeid, &getattr_in, outattrp);
> +
> +       err = fuse_compound_add(compound, &getattr_args);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_send(compound);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_get_error(compound, 0);
> +       if (err)
> +               goto out;
> +
> +       err = fuse_compound_get_error(compound, 1);
> +       if (err)
> +               goto out;

Hmm, if the open succeeds but the getattr fails, why not process it
kernel-side as a success for the open? Especially since on the server
side, libfuse will disassemble the compound request into separate
ones, so the server has no idea the open is even part of a compound.

I haven't looked at the rest of the patch yet but this caught my
attention when i was looking at how fuse_compound_get_error() gets
used.

Thanks,
Joanne

> +
> +       ff->fh = outopenp->fh;
> +       ff->open_flags = outopenp->open_flags;
> +
> +out:
> +       fuse_compound_free(compound);
> +       return err;
> +}
> +
Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Horst Birthelmer 3 weeks, 4 days ago
On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > +
> > +       err = fuse_compound_send(compound);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_get_error(compound, 0);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_get_error(compound, 1);
> > +       if (err)
> > +               goto out;
> 
> Hmm, if the open succeeds but the getattr fails, why not process it
> kernel-side as a success for the open? Especially since on the server
> side, libfuse will disassemble the compound request into separate
> ones, so the server has no idea the open is even part of a compound.
> 
> I haven't looked at the rest of the patch yet but this caught my
> attention when i was looking at how fuse_compound_get_error() gets
> used.
>
After looking at this again ...
Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
 
> Thanks,
> Joanne
> 

Thanks,
Horst
Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Bernd Schubert 3 weeks, 4 days ago

On 1/15/26 14:38, Horst Birthelmer wrote:
> On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
>> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>>>
>>> +
>>> +       err = fuse_compound_send(compound);
>>> +       if (err)
>>> +               goto out;
>>> +
>>> +       err = fuse_compound_get_error(compound, 0);
>>> +       if (err)
>>> +               goto out;
>>> +
>>> +       err = fuse_compound_get_error(compound, 1);
>>> +       if (err)
>>> +               goto out;
>>
>> Hmm, if the open succeeds but the getattr fails, why not process it
>> kernel-side as a success for the open? Especially since on the server
>> side, libfuse will disassemble the compound request into separate
>> ones, so the server has no idea the open is even part of a compound.
>>
>> I haven't looked at the rest of the patch yet but this caught my
>> attention when i was looking at how fuse_compound_get_error() gets
>> used.
>>
> After looking at this again ...
> Do you think it would make sense to add an example of lookup+create, or would that just convolute things?


I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
the way Miklos proposes. To keep things simple, maybe not right now?


Thanks,
Bernd
Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Horst Birthelmer 3 weeks, 4 days ago
On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
> 
> 
> On 1/15/26 14:38, Horst Birthelmer wrote:
> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >>>
> >>> +
> >>> +       err = fuse_compound_send(compound);
> >>> +       if (err)
> >>> +               goto out;
> >>> +
> >>> +       err = fuse_compound_get_error(compound, 0);
> >>> +       if (err)
> >>> +               goto out;
> >>> +
> >>> +       err = fuse_compound_get_error(compound, 1);
> >>> +       if (err)
> >>> +               goto out;
> >>
> >> Hmm, if the open succeeds but the getattr fails, why not process it
> >> kernel-side as a success for the open? Especially since on the server
> >> side, libfuse will disassemble the compound request into separate
> >> ones, so the server has no idea the open is even part of a compound.
> >>
> >> I haven't looked at the rest of the patch yet but this caught my
> >> attention when i was looking at how fuse_compound_get_error() gets
> >> used.
> >>
> > After looking at this again ...
> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
> 
> 
> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
> the way Miklos proposes. To keep things simple, maybe not right now?

I was thinking more along the lines of ... we would have more than one example especially for the error handling. Otherwise it is easy to miss something because the given example just doesn't need that special case.
Like the case above. There we would be perfectly fine with a function returning the first error, which in the case of lookup+create is the opposite of success and you would need to access every single error to check what actually happened.

> 
> 
> Thanks,
> Bernd
Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Luis Henriques 3 weeks, 4 days ago
On Thu, Jan 15 2026, Horst Birthelmer wrote:

> On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
>> 
>> 
>> On 1/15/26 14:38, Horst Birthelmer wrote:
>> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
>> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>> >>>
>> >>> +
>> >>> +       err = fuse_compound_send(compound);
>> >>> +       if (err)
>> >>> +               goto out;
>> >>> +
>> >>> +       err = fuse_compound_get_error(compound, 0);
>> >>> +       if (err)
>> >>> +               goto out;
>> >>> +
>> >>> +       err = fuse_compound_get_error(compound, 1);
>> >>> +       if (err)
>> >>> +               goto out;
>> >>
>> >> Hmm, if the open succeeds but the getattr fails, why not process it
>> >> kernel-side as a success for the open? Especially since on the server
>> >> side, libfuse will disassemble the compound request into separate
>> >> ones, so the server has no idea the open is even part of a compound.
>> >>
>> >> I haven't looked at the rest of the patch yet but this caught my
>> >> attention when i was looking at how fuse_compound_get_error() gets
>> >> used.
>> >>
>> > After looking at this again ...
>> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
>> 
>> 
>> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
>> the way Miklos proposes. To keep things simple, maybe not right now?
>
> I was thinking more along the lines of ... we would have more than one example
> especially for the error handling. Otherwise it is easy to miss something
> because the given example just doesn't need that special case.
> Like the case above. There we would be perfectly fine with a function returning
> the first error, which in the case of lookup+create is the opposite of success
> and you would need to access every single error to check what actually happened.

Not sure if I can add a lot to this discussion, but I've been playing a
bit with your patchset.

I was trying to understand how to implement the LOOKUP_HANDLE+STATX, and
it doesn't look too difficult at the moment.  But I guess it'll take me some
more time to figure out all the other unknowns (e.g. other operations such
as readdirplus).

Anyway, the interface for compound operations seem to be quite usable in
general.  I'll try to do a proper review soon, but regarding the specific
comment of error handling, I find the interface a bit clumsy.  Have you
thought about using something like an iterator?  Or maybe some sort of
macro such as foreach_compound_error()?

And regarding the error handling in general: it sounds like things can
become really complex when some operations within a compound operation may
succeed and others fail.  Because these examples are using two operations
only, but there's nothing preventing us from having 3 or more in the
future, right?  Wouldn't it be easier to have the compound operation
itself fail or succeed, instead of each op?  (Although that would probably
simply move the complexity into user-space, that would be required to do
more clean-up work when there are failures.)

Cheers,
-- 
Luís
Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Horst Birthelmer 3 weeks, 4 days ago
Hi Luis,

thanks for looking at this.

On Thu, Jan 15, 2026 at 03:11:25PM +0000, Luis Henriques wrote:
> On Thu, Jan 15 2026, Horst Birthelmer wrote:
> 
> > On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
> >> 
> >> 
> >> On 1/15/26 14:38, Horst Birthelmer wrote:
> >> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> >> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >> >>>
> >> >>> +
> >> >>> +       err = fuse_compound_send(compound);
> >> >>> +       if (err)
> >> >>> +               goto out;
> >> >>> +
> >> >>> +       err = fuse_compound_get_error(compound, 0);
> >> >>> +       if (err)
> >> >>> +               goto out;
> >> >>> +
> >> >>> +       err = fuse_compound_get_error(compound, 1);
> >> >>> +       if (err)
> >> >>> +               goto out;
> >> >>
> >> >> Hmm, if the open succeeds but the getattr fails, why not process it
> >> >> kernel-side as a success for the open? Especially since on the server
> >> >> side, libfuse will disassemble the compound request into separate
> >> >> ones, so the server has no idea the open is even part of a compound.
> >> >>
> >> >> I haven't looked at the rest of the patch yet but this caught my
> >> >> attention when i was looking at how fuse_compound_get_error() gets
> >> >> used.
> >> >>
> >> > After looking at this again ...
> >> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
> >> 
> >> 
> >> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
> >> the way Miklos proposes. To keep things simple, maybe not right now?
> >
> > I was thinking more along the lines of ... we would have more than one example
> > especially for the error handling. Otherwise it is easy to miss something
> > because the given example just doesn't need that special case.
> > Like the case above. There we would be perfectly fine with a function returning
> > the first error, which in the case of lookup+create is the opposite of success
> > and you would need to access every single error to check what actually happened.
> 
> Not sure if I can add a lot to this discussion, but I've been playing a
> bit with your patchset.
You already do ;-)

> 
> I was trying to understand how to implement the LOOKUP_HANDLE+STATX, and
> it doesn't look too difficult at the moment.  But I guess it'll take me some
> more time to figure out all the other unknowns (e.g. other operations such
> as readdirplus).
> 
> Anyway, the interface for compound operations seem to be quite usable in
> general.  I'll try to do a proper review soon, but regarding the specific
> comment of error handling, I find the interface a bit clumsy.  Have you
> thought about using something like an iterator?  Or maybe some sort of
> macro such as foreach_compound_error()?
Not in those terms, no.
But I don't think it would get any better. Do you have an idea you would want implemented in this context?

> 
> And regarding the error handling in general: it sounds like things can
> become really complex when some operations within a compound operation may
> succeed and others fail.  Because these examples are using two operations
> only, but there's nothing preventing us from having 3 or more in the
> future, right?  Wouldn't it be easier to have the compound operation
> itself fail or succeed, instead of each op?  (Although that would probably
> simply move the complexity into user-space, that would be required to do
> more clean-up work when there are failures.)

I think we need all the granularity we can get since different combinations mean different things in different contexts.
Imagine you have a compound as the current example. That is pretty much all or nothing and there is almost no way that one of the operations doesn't succeed, and if it goes wrong you can still fall back to separate operations.
There are certainly cases where the compound itself is the operation because it really has to be atomic, or for arguments sake they have to be started concurrently ... or whatnot.


> 
> Cheers,
> -- 
> Luís

Thanks,
Horst
Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Luis Henriques 3 weeks, 3 days ago
Hi Horst,

On Thu, Jan 15 2026, Horst Birthelmer wrote:

> Hi Luis,
>
> thanks for looking at this.
>
> On Thu, Jan 15, 2026 at 03:11:25PM +0000, Luis Henriques wrote:
>> On Thu, Jan 15 2026, Horst Birthelmer wrote:
>> 
>> > On Thu, Jan 15, 2026 at 02:41:49PM +0100, Bernd Schubert wrote:
>> >> 
>> >> 
>> >> On 1/15/26 14:38, Horst Birthelmer wrote:
>> >> > On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
>> >> >> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
>> >> >>>
>> >> >>> +
>> >> >>> +       err = fuse_compound_send(compound);
>> >> >>> +       if (err)
>> >> >>> +               goto out;
>> >> >>> +
>> >> >>> +       err = fuse_compound_get_error(compound, 0);
>> >> >>> +       if (err)
>> >> >>> +               goto out;
>> >> >>> +
>> >> >>> +       err = fuse_compound_get_error(compound, 1);
>> >> >>> +       if (err)
>> >> >>> +               goto out;
>> >> >>
>> >> >> Hmm, if the open succeeds but the getattr fails, why not process it
>> >> >> kernel-side as a success for the open? Especially since on the server
>> >> >> side, libfuse will disassemble the compound request into separate
>> >> >> ones, so the server has no idea the open is even part of a compound.
>> >> >>
>> >> >> I haven't looked at the rest of the patch yet but this caught my
>> >> >> attention when i was looking at how fuse_compound_get_error() gets
>> >> >> used.
>> >> >>
>> >> > After looking at this again ...
>> >> > Do you think it would make sense to add an example of lookup+create, or would that just convolute things?
>> >> 
>> >> 
>> >> I think that will be needed with the LOOKUP_HANDLE from Luis, if we go
>> >> the way Miklos proposes. To keep things simple, maybe not right now?
>> >
>> > I was thinking more along the lines of ... we would have more than one example
>> > especially for the error handling. Otherwise it is easy to miss something
>> > because the given example just doesn't need that special case.
>> > Like the case above. There we would be perfectly fine with a function returning
>> > the first error, which in the case of lookup+create is the opposite of success
>> > and you would need to access every single error to check what actually happened.
>> 
>> Not sure if I can add a lot to this discussion, but I've been playing a
>> bit with your patchset.
> You already do ;-)
>
>> 
>> I was trying to understand how to implement the LOOKUP_HANDLE+STATX, and
>> it doesn't look too difficult at the moment.  But I guess it'll take me some
>> more time to figure out all the other unknowns (e.g. other operations such
>> as readdirplus).
>> 
>> Anyway, the interface for compound operations seem to be quite usable in
>> general.  I'll try to do a proper review soon, but regarding the specific
>> comment of error handling, I find the interface a bit clumsy.  Have you
>> thought about using something like an iterator?  Or maybe some sort of
>> macro such as foreach_compound_error()?
> Not in those terms, no.
> But I don't think it would get any better. Do you have an idea you would want implemented in this context?

Yeah, I'm not really sure it would be any better, to be honest.  And I'm
afraid I'm just bikeshedding...  My suggestion was to have something like:

	err = fuse_compound_send(compound);
	if (err)
		goto out;

	for_each_compound_error(op_arg, compound, list) {
		err = handle_error(op_arg);
		if (err)
			goto out;
	}

But again, this is probably unnecessary.  Or at least not while we're
talking about having compound requests with 2 operations only.

>> And regarding the error handling in general: it sounds like things can
>> become really complex when some operations within a compound operation may
>> succeed and others fail.  Because these examples are using two operations
>> only, but there's nothing preventing us from having 3 or more in the
>> future, right?  Wouldn't it be easier to have the compound operation
>> itself fail or succeed, instead of each op?  (Although that would probably
>> simply move the complexity into user-space, that would be required to do
>> more clean-up work when there are failures.)
>
> I think we need all the granularity we can get since different combinations mean different things in different contexts.
> Imagine you have a compound as the current example. That is pretty much all or
> nothing and there is almost no way that one of the operations doesn't succeed,
> and if it goes wrong you can still fall back to separate operations.
> There are certainly cases where the compound itself is the operation because it
> really has to be atomic, or for arguments sake they have to be started
> concurrently ... or whatnot.

OK, got it.  I guess that errors on atomic compound operations will then
be handled that way (i.e. all or nothing), and it will always depend on
the combination of operations.  Anyway, the devil is always in the details
and I'm sure you've been putting a lot of thought into this ;-)

Cheers,
-- 
Luís
Re: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Posted by Horst Birthelmer 3 weeks, 4 days ago
On Wed, Jan 14, 2026 at 06:29:26PM -0800, Joanne Koong wrote:
> On Fri, Jan 9, 2026 at 10:27 AM Horst Birthelmer <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > 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   | 110 +++++++++++++++++++++++++++++++++++++++++++++++--------
> >  fs/fuse/fuse_i.h |   7 +++-
> >  fs/fuse/inode.c  |   6 +++
> >  fs/fuse/ioctl.c  |   2 +-
> >  4 files changed, 107 insertions(+), 18 deletions(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 53744559455d..c0375b32967d 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -152,8 +152,66 @@ static void fuse_file_put(struct fuse_file *ff, bool sync)
> >         }
> >  }
> >
...
> > +
> > +       err = fuse_compound_get_error(compound, 0);
> > +       if (err)
> > +               goto out;
> > +
> > +       err = fuse_compound_get_error(compound, 1);
> > +       if (err)
> > +               goto out;
> 
> Hmm, if the open succeeds but the getattr fails, why not process it
> kernel-side as a success for the open? Especially since on the server
> side, libfuse will disassemble the compound request into separate
> ones, so the server has no idea the open is even part of a compound.
> 
For this specific one (open+getattr) you are completely right. That makes total sense.
But there is the possibility in libfuse and a fuse server to process the compound as such. Especially when it has the (not defined in this patch) 'atomic' flag set.
This actually makes me think that there are multiple error handling scenarios.
1. The one we have here. Have errors for every compound request
2. it failed as a compound but no errors for the request (I have to think of a good example for this ;-) )
3. is not supported with the conditions set (like some atomic flags)

That's why I said in the other comment that we probably need two or three different functions here.
This gets complicated fast ....

Let me come up with something in the next version and see what you and the people here think.

> I haven't looked at the rest of the patch yet but this caught my
> attention when i was looking at how fuse_compound_get_error() gets
> used.
> 
> Thanks,
> Joanne
> 

Thanks,
Horst