On Tue, 20 Sept 2022 at 03:33, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Sep 19, 2022 at 04:10:27PM +0200, Miklos Szeredi wrote:
> > If tmpfile is used for copy up, then use this helper to create the tmpfile
> > and open it at the same time. This will later allow filesystems such as
> > fuse to do this operation atomically.
> >
> > Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
> > ---
> > fs/overlayfs/copy_up.c | 49 ++++++++++++++++++++++------------------
> > fs/overlayfs/overlayfs.h | 12 ++++++----
> > fs/overlayfs/super.c | 10 ++++----
> > 3 files changed, 40 insertions(+), 31 deletions(-)
> >
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index fdde6c56cc3d..ac087b48b5da 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -194,16 +194,16 @@ static int ovl_copy_fileattr(struct inode *inode, struct path *old,
> > }
> >
> > static int ovl_copy_up_data(struct ovl_fs *ofs, struct path *old,
> > - struct path *new, loff_t len)
> > + struct path *new, struct file *new_file, loff_t len)
> > {
>
> Ugh... Lift opening into both callers and get rid of struct path *new,
> please. Would be much easier to follow that way...
>
> > - err = ovl_copy_up_inode(c, temp);
> > + err = ovl_copy_up_inode(c, temp, NULL);
>
> FWIW, I would consider passing a struct file * in all cases, with O_PATH
> for non-regular ones...
OK.
>
> > - temp = ovl_do_tmpfile(ofs, ofs->workdir, S_IFREG | 0);
> > - ofs->tmpfile = !IS_ERR(temp);
> > + tmpfile = ovl_do_tmpfile(ofs, ofs->workdir, S_IFREG | 0);
> > + ofs->tmpfile = !IS_ERR(tmpfile);
> > if (ofs->tmpfile)
> > - dput(temp);
> > + fput(tmpfile);
>
> Careful - that part essentially checks if we have a working
> ->tmpfile(), but we rely upon more than just having it - we want
> dentry-based setxattr() and friends to work after O_TMPFILE.
> Are we making that a requirement for ->tmpfile()?
Yes, I think that should be expected of all sane filesystems. Adding
tmpfile support to a fuse filesystem will require explicit code, so no
regression risk there.
Thanks,
Miklos
