public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: Luis Henriques <luis@igalia.com>
To: Horst Birthelmer <horst@birthelmer.de>
Cc: Bernd Schubert <bernd@bsbernd.com>,
	 Joanne Koong <joannelkoong@gmail.com>,
	 Horst Birthelmer <horst@birthelmer.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	 linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	 Horst Birthelmer <hbirthelmer@ddn.com>
Subject: Re: [PATCH v4 3/3] fuse: add an implementation of open+getattr
Date: Thu, 15 Jan 2026 15:11:25 +0000	[thread overview]
Message-ID: <87wm1i52si.fsf@wotan.olymp> (raw)
In-Reply-To: <aWju_kqgdiOZt8gn@fedora.fritz.box> (Horst Birthelmer's message of "Thu, 15 Jan 2026 14:46:27 +0100")

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

  reply	other threads:[~2026-01-15 15:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-09 18:26 [PATCH v4 0/3] fuse: compound commands Horst Birthelmer
2026-01-09 18:26 ` [PATCH v4 1/3] fuse: add compound command to combine multiple requests Horst Birthelmer
2026-01-15  2:40   ` Joanne Koong
2026-01-15  8:01     ` Horst Birthelmer
2026-01-09 18:27 ` [PATCH v4 2/3] fuse: create helper functions for filling in fuse args for open and getattr Horst Birthelmer
2026-01-15  2:37   ` Joanne Koong
2026-01-15  8:06     ` Horst Birthelmer
2026-01-09 18:27 ` [PATCH v4 3/3] fuse: add an implementation of open+getattr Horst Birthelmer
2026-01-15  2:29   ` Joanne Koong
2026-01-15  8:19     ` Horst Birthelmer
2026-01-15 13:38     ` Horst Birthelmer
2026-01-15 13:41       ` Bernd Schubert
2026-01-15 13:46         ` Horst Birthelmer
2026-01-15 15:11           ` Luis Henriques [this message]
2026-01-15 15:25             ` Horst Birthelmer
2026-01-15 17:14               ` Luis Henriques

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=87wm1i52si.fsf@wotan.olymp \
    --to=luis@igalia.com \
    --cc=bernd@bsbernd.com \
    --cc=hbirthelmer@ddn.com \
    --cc=horst@birthelmer.com \
    --cc=horst@birthelmer.de \
    --cc=joannelkoong@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox