Re: [PATCH v2 4/5] fuse: prepare for failing open response

[Amir Goldstein <amir73il@gmail.com>]

On Thu, Feb 1, 2024 at 11:23 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Thu, 1 Feb 2024 at 00:09, Bernd Schubert <bschubert@ddn.com> wrote:
> >
> > From: Amir Goldstein <amir73il@gmail.com>
> >
> > In preparation for inode io modes, a server open response could fail
> > due to conflicting inode io modes.
> >
> > Allow returning an error from fuse_finish_open() and handle the error in
> > the callers. fuse_dir_open() can now call fuse_sync_release(), so handle
> > the isdir case correctly.
>
> While that's true, it may be better to just decouple the dir/regular
> paths completely, since there isn't much sharing anyway and becoming
> even less.
>

I can look into it, but for now the fix to fuse_sync_release() is a simple
one liner, so I would rather limit the changes in this series.

> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index d19cbf34c634..d45d4a678351 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -692,13 +692,15 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
> >         d_instantiate(entry, inode);
> >         fuse_change_entry_timeout(entry, &outentry);
> >         fuse_dir_changed(dir);
> > -       err = finish_open(file, entry, generic_file_open);
> > +       err = generic_file_open(inode, file);
> > +       if (!err) {
> > +               file->private_data = ff;
> > +               err = finish_open(file, entry, fuse_finish_open);
>
> Need to be careful with moving fuse_finish_open() call inside
> finish_open() since various fields will be different.
>
> In particular O_TRUNC in f_flags will not be cleared and in this case
> it looks undesirable.

Why? coming from fuse_open_common(), fuse_finish_open() is
called before clearing O_TRUNC.

Is fuse_finish_open() supposed to be called after clearing O_TRUNC
in fuse_create_open()?

I realize that this is what the code is doing in upstream, but it does not
look correct.

Probably, nobody could notice it, because server would probably have
truncated the file before the CREATE response anyway?

Am I missing something?

Thanks,
Amir.