| Message ID | 20220916194416.1657716-8-mszeredi@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/8] cachefiles: tmpfile error handling cleanup | expand |
On 9/16/22 21:44, Miklos Szeredi wrote: > +static int fuse_tmpfile(struct user_namespace *mnt_userns, struct inode *dir, > + struct file *file, umode_t mode) > +{ > + struct fuse_conn *fc = get_fuse_conn(dir); > + int err; > + > + if (fc->no_tmpfile) > + goto no_tmpfile; > + > + err = fuse_create_open(dir, file->f_path.dentry, file, file->f_flags, mode, FUSE_TMPFILE); > + if (err == -ENOSYS) { > + fc->no_tmpfile = 1; > +no_tmpfile: > + err = -EOPNOTSUPP; > + } > + return err; > +} A bit confusing part is that the other file systems are calling your new finish_tmpfile(), while fuse_create_open() calls finish_open() for tmpfiles as well. Seems to be identical but won't this easily miss possible changes done in the future to finish_tmpfile()? Thanks, Bernd
On Fri, 16 Sept 2022 at 23:52, Bernd Schubert <bernd.schubert@fastmail.fm> wrote: > > > > On 9/16/22 21:44, Miklos Szeredi wrote: > > > > +static int fuse_tmpfile(struct user_namespace *mnt_userns, struct inode *dir, > > + struct file *file, umode_t mode) > > +{ > > + struct fuse_conn *fc = get_fuse_conn(dir); > > + int err; > > + > > + if (fc->no_tmpfile) > > + goto no_tmpfile; > > + > > + err = fuse_create_open(dir, file->f_path.dentry, file, file->f_flags, mode, FUSE_TMPFILE); > > + if (err == -ENOSYS) { > > + fc->no_tmpfile = 1; > > +no_tmpfile: > > + err = -EOPNOTSUPP; > > + } > > + return err; > > +} > > A bit confusing part is that the other file systems are calling your new > finish_tmpfile(), while fuse_create_open() calls finish_open() for > tmpfiles as well. Seems to be identical but won't this easily miss > possible changes done in the future to finish_tmpfile()? There shouldn't be any such changes. It's really just a shorthand form of finish_open(). Would calling it finish_open_simple() help? It really has nothing to do with tmpfile and .atomic_open instances could call it as well. Thanks, Miklos
On 9/19/22 08:30, Miklos Szeredi wrote: > On Fri, 16 Sept 2022 at 23:52, Bernd Schubert > <bernd.schubert@fastmail.fm> wrote: >> >> >> >> On 9/16/22 21:44, Miklos Szeredi wrote: >> >> >>> +static int fuse_tmpfile(struct user_namespace *mnt_userns, struct inode *dir, >>> + struct file *file, umode_t mode) >>> +{ >>> + struct fuse_conn *fc = get_fuse_conn(dir); >>> + int err; >>> + >>> + if (fc->no_tmpfile) >>> + goto no_tmpfile; >>> + >>> + err = fuse_create_open(dir, file->f_path.dentry, file, file->f_flags, mode, FUSE_TMPFILE); >>> + if (err == -ENOSYS) { >>> + fc->no_tmpfile = 1; >>> +no_tmpfile: >>> + err = -EOPNOTSUPP; >>> + } >>> + return err; >>> +} >> >> A bit confusing part is that the other file systems are calling your new >> finish_tmpfile(), while fuse_create_open() calls finish_open() for >> tmpfiles as well. Seems to be identical but won't this easily miss >> possible changes done in the future to finish_tmpfile()? > > There shouldn't be any such changes. It's really just a shorthand > form of finish_open(). It is just a minor concern for future maintenance, from my point of view it is better to be entirely consistent. > > Would calling it finish_open_simple() help? It really has nothing to > do with tmpfile and .atomic_open instances could call it as well. Yeah, I think that would be a better name, maybe others could add their opinion here? Thanks, Bernd
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c index b585b04e815e..01b2d5c5a64a 100644 --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -529,7 +529,7 @@ static int get_security_context(struct dentry *entry, umode_t mode, */ static int fuse_create_open(struct inode *dir, struct dentry *entry, struct file *file, unsigned int flags, - umode_t mode) + umode_t mode, u32 opcode) { int err; struct inode *inode; @@ -573,7 +573,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry, inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID; } - args.opcode = FUSE_CREATE; + args.opcode = opcode; args.nodeid = get_node_id(dir); args.in_numargs = 2; args.in_args[0].size = sizeof(inarg); @@ -676,7 +676,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry, if (fc->no_create) goto mknod; - err = fuse_create_open(dir, entry, file, flags, mode); + err = fuse_create_open(dir, entry, file, flags, mode, FUSE_CREATE); if (err == -ENOSYS) { fc->no_create = 1; goto mknod; @@ -802,6 +802,24 @@ static int fuse_create(struct user_namespace *mnt_userns, struct inode *dir, return fuse_mknod(&init_user_ns, dir, entry, mode, 0); } +static int fuse_tmpfile(struct user_namespace *mnt_userns, struct inode *dir, + struct file *file, umode_t mode) +{ + struct fuse_conn *fc = get_fuse_conn(dir); + int err; + + if (fc->no_tmpfile) + goto no_tmpfile; + + err = fuse_create_open(dir, file->f_path.dentry, file, file->f_flags, mode, FUSE_TMPFILE); + if (err == -ENOSYS) { + fc->no_tmpfile = 1; +no_tmpfile: + err = -EOPNOTSUPP; + } + return err; +} + static int fuse_mkdir(struct user_namespace *mnt_userns, struct inode *dir, struct dentry *entry, umode_t mode) { @@ -1913,6 +1931,7 @@ static const struct inode_operations fuse_dir_inode_operations = { .setattr = fuse_setattr, .create = fuse_create, .atomic_open = fuse_atomic_open, + .tmpfile = fuse_tmpfile, .mknod = fuse_mknod, .permission = fuse_permission, .getattr = fuse_getattr, diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h index 488b460e046f..98a9cf531873 100644 --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -784,6 +784,9 @@ struct fuse_conn { /* Does the filesystem support per inode DAX? */ unsigned int inode_dax:1; + /* Is tmpfile not implemented by fs? */ + unsigned int no_tmpfile:1; + /** The number of requests waiting for completion */ atomic_t num_waiting; diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h index d6ccee961891..76ee8f9e024a 100644 --- a/include/uapi/linux/fuse.h +++ b/include/uapi/linux/fuse.h @@ -194,6 +194,9 @@ * - add FUSE_SECURITY_CTX init flag * - add security context to create, mkdir, symlink, and mknod requests * - add FUSE_HAS_INODE_DAX, FUSE_ATTR_DAX + * + * 7.37 + * - add FUSE_TMPFILE */ #ifndef _LINUX_FUSE_H @@ -229,7 +232,7 @@ #define FUSE_KERNEL_VERSION 7 /** Minor version number of this interface */ -#define FUSE_KERNEL_MINOR_VERSION 36 +#define FUSE_KERNEL_MINOR_VERSION 37 /** The node ID of the root inode */ #define FUSE_ROOT_ID 1 @@ -537,6 +540,7 @@ enum fuse_opcode { FUSE_SETUPMAPPING = 48, FUSE_REMOVEMAPPING = 49, FUSE_SYNCFS = 50, + FUSE_TMPFILE = 51, /* CUSE specific operations */ CUSE_INIT = 4096,
This is basically equivalent to the FUSE_CREATE operation which creates and opens a regular file. Add a new FUSE_TMPFILE operation, otherwise just reuse the protocol and the code for FUSE_CREATE. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/fuse/dir.c | 25 ++++++++++++++++++++++--- fs/fuse/fuse_i.h | 3 +++ include/uapi/linux/fuse.h | 6 +++++- 3 files changed, 30 insertions(+), 4 deletions(-)