| Message ID | alpine.DEB.2.00.1401290623040.27886@cobra.newdream.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 29, 2014 at 4:30 PM, Sage Weil <sage@inktank.com> 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@redhat.com> 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@inktank.com> > 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@inktank.com> > --- > 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 ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c index c7581f37..898b656 100644 --- a/fs/ceph/xattr.c +++ b/fs/ceph/xattr.c @@ -6,6 +6,7 @@ #include <linux/ceph/decode.h> #include <linux/xattr.h> +#include <linux/posix_acl_xattr.h> #include <linux/slab.h> #define XATTR_CEPH_PREFIX "ceph." @@ -17,8 +18,8 @@ */ const struct xattr_handler *ceph_xattr_handlers[] = { #ifdef CONFIG_CEPH_FS_POSIX_ACL - &ceph_xattr_acl_access_handler, - &ceph_xattr_acl_default_handler, + &posix_acl_access_xattr_handler, + &posix_acl_default_xattr_handler, #endif NULL, };