On Mon, Aug 1, 2016 at 11:27 PM, Seth Forshee
<seth.forshee@xxxxxxxxxxxxx> wrote:
> In preparation for posix acl support, rework fuse to use xattr
> handlers and the generic setxattr/getxattr/listxattr callbacks.
> Split the xattr code out into it's own file, and promote symbols
> to module-global scope as needed.
>
> Functionally these changes have no impact, as fuse still uses a
> single handler for all xattrs which uses the old callbacks.
One comment at the very bottom of the patch.
Otherwise it looks okay.
Thanks,
Miklos
>
> Signed-off-by: Seth Forshee <seth.forshee@xxxxxxxxxxxxx>
> ---
> fs/fuse/Makefile | 2 +-
> fs/fuse/dir.c | 167 ++++----------------------------------------------
> fs/fuse/fuse_i.h | 5 ++
> fs/fuse/inode.c | 1 +
> fs/fuse/xattr.c | 183 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 201 insertions(+), 157 deletions(-)
> create mode 100644 fs/fuse/xattr.c
>
> diff --git a/fs/fuse/Makefile b/fs/fuse/Makefile
> index e95eeb445e58..448aa27ada00 100644
> --- a/fs/fuse/Makefile
> +++ b/fs/fuse/Makefile
> @@ -5,4 +5,4 @@
> obj-$(CONFIG_FUSE_FS) += fuse.o
> obj-$(CONFIG_CUSE) += cuse.o
>
> -fuse-objs := dev.o dir.o file.o inode.o control.o
> +fuse-objs := dev.o dir.o file.o inode.o control.o xattr.o
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 8466e122ee62..9df55b8e776a 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -13,6 +13,7 @@
> #include <linux/sched.h>
> #include <linux/namei.h>
> #include <linux/slab.h>
> +#include <linux/xattr.h>
>
> static bool fuse_use_readdirplus(struct inode *dir, struct dir_context *ctx)
> {
> @@ -632,7 +633,7 @@ static int fuse_symlink(struct inode *dir, struct dentry *entry,
> return create_new_entry(fc, &args, dir, entry, S_IFLNK);
> }
>
> -static inline void fuse_update_ctime(struct inode *inode)
> +void fuse_update_ctime(struct inode *inode)
> {
> if (!IS_NOCMTIME(inode)) {
> inode->i_ctime = current_fs_time(inode->i_sb);
> @@ -1719,152 +1720,6 @@ static int fuse_getattr(struct vfsmount *mnt, struct dentry *entry,
> return fuse_update_attributes(inode, stat, NULL, NULL);
> }
>
> -static int fuse_setxattr(struct dentry *unused, struct inode *inode,
> - const char *name, const void *value,
> - size_t size, int flags)
> -{
> - struct fuse_conn *fc = get_fuse_conn(inode);
> - FUSE_ARGS(args);
> - struct fuse_setxattr_in inarg;
> - int err;
> -
> - if (fc->no_setxattr)
> - return -EOPNOTSUPP;
> -
> - memset(&inarg, 0, sizeof(inarg));
> - inarg.size = size;
> - inarg.flags = flags;
> - args.in.h.opcode = FUSE_SETXATTR;
> - args.in.h.nodeid = get_node_id(inode);
> - args.in.numargs = 3;
> - args.in.args[0].size = sizeof(inarg);
> - args.in.args[0].value = &inarg;
> - args.in.args[1].size = strlen(name) + 1;
> - args.in.args[1].value = name;
> - args.in.args[2].size = size;
> - args.in.args[2].value = value;
> - err = fuse_simple_request(fc, &args);
> - if (err == -ENOSYS) {
> - fc->no_setxattr = 1;
> - err = -EOPNOTSUPP;
> - }
> - if (!err) {
> - fuse_invalidate_attr(inode);
> - fuse_update_ctime(inode);
> - }
> - return err;
> -}
> -
> -static ssize_t fuse_getxattr(struct dentry *entry, struct inode *inode,
> - const char *name, void *value, size_t size)
> -{
> - struct fuse_conn *fc = get_fuse_conn(inode);
> - FUSE_ARGS(args);
> - struct fuse_getxattr_in inarg;
> - struct fuse_getxattr_out outarg;
> - ssize_t ret;
> -
> - if (fc->no_getxattr)
> - return -EOPNOTSUPP;
> -
> - memset(&inarg, 0, sizeof(inarg));
> - inarg.size = size;
> - args.in.h.opcode = FUSE_GETXATTR;
> - args.in.h.nodeid = get_node_id(inode);
> - args.in.numargs = 2;
> - args.in.args[0].size = sizeof(inarg);
> - args.in.args[0].value = &inarg;
> - args.in.args[1].size = strlen(name) + 1;
> - args.in.args[1].value = name;
> - /* This is really two different operations rolled into one */
> - args.out.numargs = 1;
> - if (size) {
> - args.out.argvar = 1;
> - args.out.args[0].size = size;
> - args.out.args[0].value = value;
> - } else {
> - args.out.args[0].size = sizeof(outarg);
> - args.out.args[0].value = &outarg;
> - }
> - ret = fuse_simple_request(fc, &args);
> - if (!ret && !size)
> - ret = outarg.size;
> - if (ret == -ENOSYS) {
> - fc->no_getxattr = 1;
> - ret = -EOPNOTSUPP;
> - }
> - return ret;
> -}
> -
> -static ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)
> -{
> - struct inode *inode = d_inode(entry);
> - struct fuse_conn *fc = get_fuse_conn(inode);
> - FUSE_ARGS(args);
> - struct fuse_getxattr_in inarg;
> - struct fuse_getxattr_out outarg;
> - ssize_t ret;
> -
> - if (!fuse_allow_current_process(fc))
> - return -EACCES;
> -
> - if (fc->no_listxattr)
> - return -EOPNOTSUPP;
> -
> - memset(&inarg, 0, sizeof(inarg));
> - inarg.size = size;
> - args.in.h.opcode = FUSE_LISTXATTR;
> - args.in.h.nodeid = get_node_id(inode);
> - args.in.numargs = 1;
> - args.in.args[0].size = sizeof(inarg);
> - args.in.args[0].value = &inarg;
> - /* This is really two different operations rolled into one */
> - args.out.numargs = 1;
> - if (size) {
> - args.out.argvar = 1;
> - args.out.args[0].size = size;
> - args.out.args[0].value = list;
> - } else {
> - args.out.args[0].size = sizeof(outarg);
> - args.out.args[0].value = &outarg;
> - }
> - ret = fuse_simple_request(fc, &args);
> - if (!ret && !size)
> - ret = outarg.size;
> - if (ret == -ENOSYS) {
> - fc->no_listxattr = 1;
> - ret = -EOPNOTSUPP;
> - }
> - return ret;
> -}
> -
> -static int fuse_removexattr(struct dentry *entry, const char *name)
> -{
> - struct inode *inode = d_inode(entry);
> - struct fuse_conn *fc = get_fuse_conn(inode);
> - FUSE_ARGS(args);
> - int err;
> -
> - if (fc->no_removexattr)
> - return -EOPNOTSUPP;
> -
> - args.in.h.opcode = FUSE_REMOVEXATTR;
> - args.in.h.nodeid = get_node_id(inode);
> - args.in.numargs = 1;
> - args.in.args[0].size = strlen(name) + 1;
> - args.in.args[0].value = name;
> - err = fuse_simple_request(fc, &args);
> - if (err == -ENOSYS) {
> - fc->no_removexattr = 1;
> - err = -EOPNOTSUPP;
> - }
> - if (!err) {
> - fuse_invalidate_attr(inode);
> - fuse_update_ctime(inode);
> - }
> - return err;
> -}
> -
> static const struct inode_operations fuse_dir_inode_operations = {
> .lookup = fuse_lookup,
> .mkdir = fuse_mkdir,
> @@ -1879,10 +1734,10 @@ static const struct inode_operations fuse_dir_inode_operations = {
> .mknod = fuse_mknod,
> .permission = fuse_permission,
> .getattr = fuse_getattr,
> - .setxattr = fuse_setxattr,
> - .getxattr = fuse_getxattr,
> + .setxattr = generic_setxattr,
> + .getxattr = generic_getxattr,
> .listxattr = fuse_listxattr,
> - .removexattr = fuse_removexattr,
> + .removexattr = generic_removexattr,
> };
>
> static const struct file_operations fuse_dir_operations = {
> @@ -1900,10 +1755,10 @@ static const struct inode_operations fuse_common_inode_operations = {
> .setattr = fuse_setattr,
> .permission = fuse_permission,
> .getattr = fuse_getattr,
> - .setxattr = fuse_setxattr,
> - .getxattr = fuse_getxattr,
> + .setxattr = generic_setxattr,
> + .getxattr = generic_getxattr,
> .listxattr = fuse_listxattr,
> - .removexattr = fuse_removexattr,
> + .removexattr = generic_removexattr,
> };
>
> static const struct inode_operations fuse_symlink_inode_operations = {
> @@ -1911,10 +1766,10 @@ static const struct inode_operations fuse_symlink_inode_operations = {
> .get_link = fuse_get_link,
> .readlink = generic_readlink,
> .getattr = fuse_getattr,
> - .setxattr = fuse_setxattr,
> - .getxattr = fuse_getxattr,
> + .setxattr = generic_setxattr,
> + .getxattr = generic_getxattr,
> .listxattr = fuse_listxattr,
> - .removexattr = fuse_removexattr,
> + .removexattr = generic_removexattr,
> };
>
> void fuse_init_common(struct inode *inode)
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 9f4c3c82edd6..93a5e8191da1 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -903,6 +903,8 @@ int fuse_allow_current_process(struct fuse_conn *fc);
>
> u64 fuse_lock_owner_id(struct fuse_conn *fc, fl_owner_t id);
>
> +void fuse_update_ctime(struct inode *inode);
> +
> int fuse_update_attributes(struct inode *inode, struct kstat *stat,
> struct file *file, bool *refreshed);
>
> @@ -964,4 +966,7 @@ int fuse_do_setattr(struct inode *inode, struct iattr *attr,
>
> void fuse_set_initialized(struct fuse_conn *fc);
>
> +ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size);
> +extern const struct xattr_handler *fuse_xattr_handlers[];
> +
> #endif /* _FS_FUSE_I_H */
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 254f1944ee98..fee06e48157d 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -1066,6 +1066,7 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
> }
> sb->s_magic = FUSE_SUPER_MAGIC;
> sb->s_op = &fuse_super_operations;
> + sb->s_xattr = fuse_xattr_handlers;
> sb->s_maxbytes = MAX_LFS_FILESIZE;
> sb->s_time_gran = 1;
> sb->s_export_op = &fuse_export_operations;
> diff --git a/fs/fuse/xattr.c b/fs/fuse/xattr.c
> new file mode 100644
> index 000000000000..d30e99610217
> --- /dev/null
> +++ b/fs/fuse/xattr.c
> @@ -0,0 +1,183 @@
> +/*
> + FUSE: Filesystem in Userspace
> + Copyright (C) 2001-2008 Miklos Szeredi <miklos@xxxxxxxxxx>
> +
> + This program can be distributed under the terms of the GNU GPL.
> + See the file COPYING.
> +*/
> +
> +#include "fuse_i.h"
> +
> +#include <linux/xattr.h>
> +
> +static int fuse_setxattr(struct inode *inode, const char *name,
> + const void *value, size_t size, int flags)
> +{
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + FUSE_ARGS(args);
> + struct fuse_setxattr_in inarg;
> + int err;
> +
> + if (fc->no_setxattr)
> + return -EOPNOTSUPP;
> +
> + memset(&inarg, 0, sizeof(inarg));
> + inarg.size = size;
> + inarg.flags = flags;
> + args.in.h.opcode = FUSE_SETXATTR;
> + args.in.h.nodeid = get_node_id(inode);
> + args.in.numargs = 3;
> + args.in.args[0].size = sizeof(inarg);
> + args.in.args[0].value = &inarg;
> + args.in.args[1].size = strlen(name) + 1;
> + args.in.args[1].value = name;
> + args.in.args[2].size = size;
> + args.in.args[2].value = value;
> + err = fuse_simple_request(fc, &args);
> + if (err == -ENOSYS) {
> + fc->no_setxattr = 1;
> + err = -EOPNOTSUPP;
> + }
> + if (!err) {
> + fuse_invalidate_attr(inode);
> + fuse_update_ctime(inode);
> + }
> + return err;
> +}
> +
> +static ssize_t fuse_getxattr(struct inode *inode, const char *name,
> + void *value, size_t size)
> +{
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + FUSE_ARGS(args);
> + struct fuse_getxattr_in inarg;
> + struct fuse_getxattr_out outarg;
> + ssize_t ret;
> +
> + if (fc->no_getxattr)
> + return -EOPNOTSUPP;
> +
> + memset(&inarg, 0, sizeof(inarg));
> + inarg.size = size;
> + args.in.h.opcode = FUSE_GETXATTR;
> + args.in.h.nodeid = get_node_id(inode);
> + args.in.numargs = 2;
> + args.in.args[0].size = sizeof(inarg);
> + args.in.args[0].value = &inarg;
> + args.in.args[1].size = strlen(name) + 1;
> + args.in.args[1].value = name;
> + /* This is really two different operations rolled into one */
> + args.out.numargs = 1;
> + if (size) {
> + args.out.argvar = 1;
> + args.out.args[0].size = size;
> + args.out.args[0].value = value;
> + } else {
> + args.out.args[0].size = sizeof(outarg);
> + args.out.args[0].value = &outarg;
> + }
> + ret = fuse_simple_request(fc, &args);
> + if (!ret && !size)
> + ret = outarg.size;
> + if (ret == -ENOSYS) {
> + fc->no_getxattr = 1;
> + ret = -EOPNOTSUPP;
> + }
> + return ret;
> +}
> +
> +ssize_t fuse_listxattr(struct dentry *entry, char *list, size_t size)
> +{
> + struct inode *inode = d_inode(entry);
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + FUSE_ARGS(args);
> + struct fuse_getxattr_in inarg;
> + struct fuse_getxattr_out outarg;
> + ssize_t ret;
> +
> + if (!fuse_allow_current_process(fc))
> + return -EACCES;
> +
> + if (fc->no_listxattr)
> + return -EOPNOTSUPP;
> +
> + memset(&inarg, 0, sizeof(inarg));
> + inarg.size = size;
> + args.in.h.opcode = FUSE_LISTXATTR;
> + args.in.h.nodeid = get_node_id(inode);
> + args.in.numargs = 1;
> + args.in.args[0].size = sizeof(inarg);
> + args.in.args[0].value = &inarg;
> + /* This is really two different operations rolled into one */
> + args.out.numargs = 1;
> + if (size) {
> + args.out.argvar = 1;
> + args.out.args[0].size = size;
> + args.out.args[0].value = list;
> + } else {
> + args.out.args[0].size = sizeof(outarg);
> + args.out.args[0].value = &outarg;
> + }
> + ret = fuse_simple_request(fc, &args);
> + if (!ret && !size)
> + ret = outarg.size;
> + if (ret == -ENOSYS) {
> + fc->no_listxattr = 1;
> + ret = -EOPNOTSUPP;
> + }
> + return ret;
> +}
> +
> +static int fuse_removexattr(struct inode *inode, const char *name)
> +{
> + struct fuse_conn *fc = get_fuse_conn(inode);
> + FUSE_ARGS(args);
> + int err;
> +
> + if (fc->no_removexattr)
> + return -EOPNOTSUPP;
> +
> + args.in.h.opcode = FUSE_REMOVEXATTR;
> + args.in.h.nodeid = get_node_id(inode);
> + args.in.numargs = 1;
> + args.in.args[0].size = strlen(name) + 1;
> + args.in.args[0].value = name;
> + err = fuse_simple_request(fc, &args);
> + if (err == -ENOSYS) {
> + fc->no_removexattr = 1;
> + err = -EOPNOTSUPP;
> + }
> + if (!err) {
> + fuse_invalidate_attr(inode);
> + fuse_update_ctime(inode);
> + }
> + return err;
> +}
> +
> +static int fuse_xattr_get(const struct xattr_handler *handler,
> + struct dentry *dentry, struct inode *inode,
> + const char *name, void *value, size_t size)
> +{
> + return fuse_getxattr(inode, name, value, size);
> +}
> +
> +static int fuse_xattr_set(const struct xattr_handler *handler,
> + struct dentry *dentry, struct inode *inode,
> + const char *name, const void *value, size_t size,
> + int flags)
> +{
> + if (!value)
> + return fuse_removexattr(inode, name);
> +
> + return fuse_setxattr(inode, name, value, size, flags);
> +}
> +
> +static const struct xattr_handler fuse_xattr_handler = {
> + .prefix = "",
> + .get = fuse_xattr_get,
> + .set = fuse_xattr_set,
> +};
> +
> +const struct xattr_handler *fuse_xattr_handlers[] = {
> + &fuse_xattr_handler,
Please terminate array with NULL, despite the catch-all prefix. The
less subtle the better.
Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
