Re: [GIT PULL] Ceph updates for -rc1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 29, 2014 at 4:30 PM, Sage Weil <sage@xxxxxxxxxxx> wrote:
> On Tue, 28 Jan 2014, Sage Weil wrote:
>> Hi Linus,
>>
>> On Tue, 28 Jan 2014, Linus Torvalds wrote:
>> > On Tue, Jan 28, 2014 at 1:10 PM, Dave Jones <davej@xxxxxxxxxx> wrote:
>> > >
>> > > This breaks the build for me.
>> >
>> > It is my merge (Christoph's ACL changes came in today through the VFS
>> > tree from Al).
>> >
>> > I was doing the merges today on my laptop (I had jury duty yesterday
>> > and today), and so I didn't do the allmodconfig build I would normally
>> > do on my (much faster) desktop. Well, actually I did do the full fs
>> > builds for the earlier pulls that actually had some conflicts, but not
>> > for the ceph pull. The conflict was hidden by the fact that the whole
>> > cifs ACL support is new, so there was no data conflict, just a silent
>> > semantic conflict between the new smarter ACL helpers and the new ACL
>> > use in CIFS.
>>
>> s/cifs/ceph/ :)
>>
>> > I'm back home now (yay, all the afternoon cases got settled), and I
>> > see the problem now. I should have done an allmodconfig build
>> > immediately after coming home, but I never even thought of it.
>> >
>> > Anyway, here's an *untested* conversion to the new posix acl helper
>> > infrastructure. I do wonder if ceph_init_acl() could be converted to
>> > posix_acl_create(), right now that part is a "non-conversion" - it's
>> > just made to use __posix_acl_create() that implements the old
>> > interface.
>> >
>> > Al, Christoph, can you please check my conversion for sanity from a
>> > generic posix-acl standpoint?
>> >
>> > Sage, Guangliang, Li, can you check the actual cifs usage/sanity of
>> > the attached patch?
>>
>> Superficially at least the conversion looks okay to me, but it's not
>> passing my smoke test (it's giving me EOPNOTSUPP for chmod and setxattr
>> when setting an ACL).  I'll look at it tomorrow if Guangliang, Li, or Yan
>> don't get there first.
>
> The set_acl inode_operation wasn't getting set, and the prototype needed
> to be adjusted a bit (it doesn't take a dentry anymore).  All seems to be
> well with the below patch.
>
> Thanks!
> sage
>
>
> From 01baa54e113060eb9147548fe7beb572522a645a Mon Sep 17 00:00:00 2001
> From: Sage Weil <sage@xxxxxxxxxxx>
> Date: Wed, 29 Jan 2014 06:22:25 -0800
> Subject: [PATCH] ceph: fix posix ACL hooks
>
> The merge of 7221fe4c2 raced with upstream changes in the generic POSIX
> ACL code (2aeccbe95).  Update Ceph to use the new helpers as well by
> dropping the now-generic functions and setting the set_acl inode op.
>
> Signed-off-by: Sage Weil <sage@xxxxxxxxxxx>
> ---
>  fs/ceph/acl.c   | 112 +++-----------------------------------------------------
>  fs/ceph/dir.c   |   1 +
>  fs/ceph/inode.c |   5 ++-
>  fs/ceph/super.h |   5 +--
>  fs/ceph/xattr.c |   5 ++-
>  5 files changed, 15 insertions(+), 113 deletions(-)
>
> diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
> index 64fddbc..66d377a 100644
> --- a/fs/ceph/acl.c
> +++ b/fs/ceph/acl.c
> @@ -107,14 +107,14 @@ struct posix_acl *ceph_get_acl(struct inode *inode, int type)
>         return acl;
>  }
>
> -static int ceph_set_acl(struct dentry *dentry, struct inode *inode,
> -                               struct posix_acl *acl, int type)
> +int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type)
>  {
>         int ret = 0, size = 0;
>         const char *name = NULL;
>         char *value = NULL;
>         struct iattr newattrs;
>         umode_t new_mode = inode->i_mode, old_mode = inode->i_mode;
> +       struct dentry *dentry = d_find_alias(inode);
>
>         if (acl) {
>                 ret = posix_acl_valid(acl);
> @@ -208,16 +208,15 @@ int ceph_init_acl(struct dentry *dentry, struct inode *inode, struct inode *dir)
>
>         if (IS_POSIXACL(dir) && acl) {
>                 if (S_ISDIR(inode->i_mode)) {
> -                       ret = ceph_set_acl(dentry, inode, acl,
> -                                               ACL_TYPE_DEFAULT);
> +                       ret = ceph_set_acl(inode, acl, ACL_TYPE_DEFAULT);
>                         if (ret)
>                                 goto out_release;
>                 }
> -               ret = posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
> +               ret = __posix_acl_create(&acl, GFP_NOFS, &inode->i_mode);
>                 if (ret < 0)
>                         goto out;
>                 else if (ret > 0)
> -                       ret = ceph_set_acl(dentry, inode, acl, ACL_TYPE_ACCESS);
> +                       ret = ceph_set_acl(inode, acl, ACL_TYPE_ACCESS);
>                 else
>                         cache_no_acl(inode);
>         } else {
> @@ -229,104 +228,3 @@ out_release:
>  out:
>         return ret;
>  }
> -
> -int ceph_acl_chmod(struct dentry *dentry, struct inode *inode)
> -{
> -       struct posix_acl *acl;
> -       int ret = 0;
> -
> -       if (S_ISLNK(inode->i_mode)) {
> -               ret = -EOPNOTSUPP;
> -               goto out;
> -       }
> -
> -       if (!IS_POSIXACL(inode))
> -               goto out;
> -
> -       acl = ceph_get_acl(inode, ACL_TYPE_ACCESS);
> -       if (IS_ERR_OR_NULL(acl)) {
> -               ret = PTR_ERR(acl);
> -               goto out;
> -       }
> -
> -       ret = posix_acl_chmod(&acl, GFP_KERNEL, inode->i_mode);
> -       if (ret)
> -               goto out;
> -       ret = ceph_set_acl(dentry, inode, acl, ACL_TYPE_ACCESS);
> -       posix_acl_release(acl);
> -out:
> -       return ret;
> -}
> -
> -static int ceph_xattr_acl_get(struct dentry *dentry, const char *name,
> -                               void *value, size_t size, int type)
> -{
> -       struct posix_acl *acl;
> -       int ret = 0;
> -
> -       if (!IS_POSIXACL(dentry->d_inode))
> -               return -EOPNOTSUPP;
> -
> -       acl = ceph_get_acl(dentry->d_inode, type);
> -       if (IS_ERR(acl))
> -               return PTR_ERR(acl);
> -       if (acl == NULL)
> -               return -ENODATA;
> -
> -       ret = posix_acl_to_xattr(&init_user_ns, acl, value, size);
> -       posix_acl_release(acl);
> -
> -       return ret;
> -}
> -
> -static int ceph_xattr_acl_set(struct dentry *dentry, const char *name,
> -                       const void *value, size_t size, int flags, int type)
> -{
> -       int ret = 0;
> -       struct posix_acl *acl = NULL;
> -
> -       if (!inode_owner_or_capable(dentry->d_inode)) {
> -               ret = -EPERM;
> -               goto out;
> -       }
> -
> -       if (!IS_POSIXACL(dentry->d_inode)) {
> -               ret = -EOPNOTSUPP;
> -               goto out;
> -       }
> -
> -       if (value) {
> -               acl = posix_acl_from_xattr(&init_user_ns, value, size);
> -               if (IS_ERR(acl)) {
> -                       ret = PTR_ERR(acl);
> -                       goto out;
> -               }
> -
> -               if (acl) {
> -                       ret = posix_acl_valid(acl);
> -                       if (ret)
> -                               goto out_release;
> -               }
> -       }
> -
> -       ret = ceph_set_acl(dentry, dentry->d_inode, acl, type);
> -
> -out_release:
> -       posix_acl_release(acl);
> -out:
> -       return ret;
> -}
> -
> -const struct xattr_handler ceph_xattr_acl_default_handler = {
> -       .prefix = POSIX_ACL_XATTR_DEFAULT,
> -       .flags  = ACL_TYPE_DEFAULT,
> -       .get    = ceph_xattr_acl_get,
> -       .set    = ceph_xattr_acl_set,
> -};
> -
> -const struct xattr_handler ceph_xattr_acl_access_handler = {
> -       .prefix = POSIX_ACL_XATTR_ACCESS,
> -       .flags  = ACL_TYPE_ACCESS,
> -       .get    = ceph_xattr_acl_get,
> -       .set    = ceph_xattr_acl_set,
> -};
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 619616d..6da4df8 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -1303,6 +1303,7 @@ const struct inode_operations ceph_dir_iops = {
>         .listxattr = ceph_listxattr,
>         .removexattr = ceph_removexattr,
>         .get_acl = ceph_get_acl,
> +       .set_acl = ceph_set_acl,
>         .mknod = ceph_mknod,
>         .symlink = ceph_symlink,
>         .mkdir = ceph_mkdir,
> diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c
> index 6fc10a7..32d519d 100644
> --- a/fs/ceph/inode.c
> +++ b/fs/ceph/inode.c
> @@ -9,6 +9,7 @@
>  #include <linux/namei.h>
>  #include <linux/writeback.h>
>  #include <linux/vmalloc.h>
> +#include <linux/posix_acl.h>
>
>  #include "super.h"
>  #include "mds_client.h"
> @@ -96,6 +97,7 @@ const struct inode_operations ceph_file_iops = {
>         .listxattr = ceph_listxattr,
>         .removexattr = ceph_removexattr,
>         .get_acl = ceph_get_acl,
> +       .set_acl = ceph_set_acl,
>  };
>
>
> @@ -1615,6 +1617,7 @@ static const struct inode_operations ceph_symlink_iops = {
>         .listxattr = ceph_listxattr,
>         .removexattr = ceph_removexattr,
>         .get_acl = ceph_get_acl,
> +       .set_acl = ceph_set_acl,
>  };
>
>  /*
> @@ -1805,7 +1808,7 @@ int ceph_setattr(struct dentry *dentry, struct iattr *attr)
>                 __mark_inode_dirty(inode, inode_dirty_flags);
>
>         if (ia_valid & ATTR_MODE) {
> -               err = ceph_acl_chmod(dentry, inode);
> +               err = posix_acl_chmod(inode, attr->ia_mode);
>                 if (err)
>                         goto out_put;
>         }
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index c299f7d..eabf601 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -718,6 +718,7 @@ extern void ceph_queue_writeback(struct inode *inode);
>  extern int ceph_do_getattr(struct inode *inode, int mask);
>  extern int ceph_permission(struct inode *inode, int mask);
>  extern int ceph_setattr(struct dentry *dentry, struct iattr *attr);
> +extern int ceph_setattr(struct dentry *dentry, struct iattr *attr);
>  extern int ceph_getattr(struct vfsmount *mnt, struct dentry *dentry,
>                         struct kstat *stat);
>
> @@ -736,15 +737,13 @@ extern void __init ceph_xattr_init(void);
>  extern void ceph_xattr_exit(void);
>
>  /* acl.c */
> -extern const struct xattr_handler ceph_xattr_acl_access_handler;
> -extern const struct xattr_handler ceph_xattr_acl_default_handler;
>  extern const struct xattr_handler *ceph_xattr_handlers[];
>
>  #ifdef CONFIG_CEPH_FS_POSIX_ACL
>
>  struct posix_acl *ceph_get_acl(struct inode *, int);
> +int ceph_set_acl(struct inode *inode, struct posix_acl *acl, int type);
>  int ceph_init_acl(struct dentry *, struct inode *, struct inode *);
> -int ceph_acl_chmod(struct dentry *, struct inode *);
>  void ceph_forget_all_cached_acls(struct inode *inode);
>
>  #else

Sage missed the '#define ceph_set_acl NULL' for the !CEPH_FS_POSIX_ACL
case.  v2 should be in-reply-to this message.

Thanks,

                Ilya
--
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




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux