Re: [PATCH v4] ceph: add acl for cephfs
On Fri, Nov 08, 2013 at 05:40:32PM +0800, Li Wang wrote:
Hi,
It seems to me there are three issues, you can take a look below
if they are really there,
Thanks very much for your reviewing.
On 11/08/2013 01:23 PM, Guangliang Zhao wrote:
v4: check the validity before set/get_cached_acl()
v3: handle the attr change in ceph_set_acl()
v2: remove some redundant code in ceph_setattr()
Signed-off-by: Guangliang Zhao lucienc...@gmail.com
---
fs/ceph/Kconfig | 13 +++
fs/ceph/Makefile |1 +
fs/ceph/acl.c| 326
++
fs/ceph/dir.c|5 +
fs/ceph/inode.c | 14 +++
fs/ceph/super.c |4 +
fs/ceph/super.h | 35 ++
fs/ceph/xattr.c | 60 --
8 files changed, 446 insertions(+), 12 deletions(-)
create mode 100644 fs/ceph/acl.c
diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
index ac9a2ef..264e9bf 100644
--- a/fs/ceph/Kconfig
+++ b/fs/ceph/Kconfig
@@ -25,3 +25,16 @@ config CEPH_FSCACHE
caching support for Ceph clients using FS-Cache
endif
+
+config CEPH_FS_POSIX_ACL
+bool Ceph POSIX Access Control Lists
+depends on CEPH_FS
+select FS_POSIX_ACL
+help
+ POSIX Access Control Lists (ACLs) support permissions for users and
+ groups beyond the owner/group/world scheme.
+
+ To learn more about Access Control Lists, visit the POSIX ACLs for
+ Linux website http://acl.bestbits.at/.
+
+ If you don't know what Access Control Lists are, say N
diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
index 32e3010..85a4230 100644
--- a/fs/ceph/Makefile
+++ b/fs/ceph/Makefile
@@ -10,3 +10,4 @@ ceph-y := super.o inode.o dir.o file.o locks.o addr.o
ioctl.o \
debugfs.o
ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
+ceph-$(CONFIG_CEPH_FS_POSIX_ACL) += acl.o
diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
new file mode 100644
index 000..a474626
--- /dev/null
+++ b/fs/ceph/acl.c
@@ -0,0 +1,326 @@
+/*
+ * linux/fs/ceph/acl.c
+ *
+ * Copyright (C) 2013 Guangliang Zhao, lucienc...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include linux/ceph/ceph_debug.h
+#include linux/fs.h
+#include linux/string.h
+#include linux/xattr.h
+#include linux/posix_acl_xattr.h
+#include linux/posix_acl.h
+#include linux/sched.h
+#include linux/slab.h
+
+#include super.h
+
+static inline void ceph_set_cached_acl(struct inode *inode,
+int type, struct posix_acl *acl)
+{
+struct ceph_inode_info *ci = ceph_inode(inode);
+int issued;
+
+spin_lock(ci-i_ceph_lock);
+issued = __ceph_caps_issued(ci, NULL);
+if (issued (CEPH_CAP_XATTR_EXCL | CEPH_CAP_XATTR_SHARED)) {
+set_cached_acl(inode, type, acl);
+ci-i_aclcache_gen = ci-i_rdcache_gen;
+}
+spin_unlock(ci-i_ceph_lock);
+
+}
+
+static inline struct posix_acl *ceph_get_cached_acl(struct inode *inode,
+int type)
+{
+struct ceph_inode_info *ci = ceph_inode(inode);
+struct posix_acl *acl = NULL;
+
+spin_lock(ci-i_ceph_lock);
+if (ci-i_aclcache_gen == ci-i_rdcache_gen)
+acl = get_cached_acl(inode, type);
+spin_unlock(ci-i_ceph_lock);
+
+return acl;
+}
+
+struct posix_acl *ceph_get_acl(struct inode *inode, int type)
+{
+int size;
+const char *name;
+char *value = NULL;
+struct posix_acl *acl;
+
+if (!IS_POSIXACL(inode))
+return NULL;
+
+acl = ceph_get_cached_acl(inode, type);
+if (acl != ACL_NOT_CACHED)
+return acl;
If client does not own cap, it can not rely on the cached acl, in
that case, ceph_get_cached_acl() will return NULL rather than
ACL_NOT_CACHED.
It will forbid to do the following synchronous MDS consultation.
Yes, if the client doesn't own cap, the acl should be no cached. We
need change the default return value to ACL_NOT_CACHED in ceph_get_cached_acl().
+
+switch (type) {
+case ACL_TYPE_ACCESS:
+name = POSIX_ACL_XATTR_ACCESS;
+break;
+case ACL_TYPE_DEFAULT:
+name = POSIX_ACL_XATTR_DEFAULT;
+break;
+default:
+BUG();
+}
+
+size = __ceph_getxattr(inode, name, , 0);
+if
Re: [PATCH v4] ceph: add acl for cephfs
On 11/08/2013 05:40 PM, Li Wang wrote:
Hi,
It seems to me there are three issues, you can take a look below if they
are really there,
On 11/08/2013 01:23 PM, Guangliang Zhao wrote:
v4: check the validity before set/get_cached_acl()
v3: handle the attr change in ceph_set_acl()
v2: remove some redundant code in ceph_setattr()
Signed-off-by: Guangliang Zhao lucienc...@gmail.com
---
fs/ceph/Kconfig | 13 +++
fs/ceph/Makefile |1 +
fs/ceph/acl.c| 326
++
fs/ceph/dir.c|5 +
fs/ceph/inode.c | 14 +++
fs/ceph/super.c |4 +
fs/ceph/super.h | 35 ++
fs/ceph/xattr.c | 60 --
8 files changed, 446 insertions(+), 12 deletions(-)
create mode 100644 fs/ceph/acl.c
diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
index ac9a2ef..264e9bf 100644
--- a/fs/ceph/Kconfig
+++ b/fs/ceph/Kconfig
@@ -25,3 +25,16 @@ config CEPH_FSCACHE
caching support for Ceph clients using FS-Cache
endif
+
+config CEPH_FS_POSIX_ACL
+bool Ceph POSIX Access Control Lists
+depends on CEPH_FS
+select FS_POSIX_ACL
+help
+ POSIX Access Control Lists (ACLs) support permissions for users and
+ groups beyond the owner/group/world scheme.
+
+ To learn more about Access Control Lists, visit the POSIX ACLs for
+ Linux website http://acl.bestbits.at/.
+
+ If you don't know what Access Control Lists are, say N
diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
index 32e3010..85a4230 100644
--- a/fs/ceph/Makefile
+++ b/fs/ceph/Makefile
@@ -10,3 +10,4 @@ ceph-y := super.o inode.o dir.o file.o locks.o addr.o
ioctl.o \
debugfs.o
ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
+ceph-$(CONFIG_CEPH_FS_POSIX_ACL) += acl.o
diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
new file mode 100644
index 000..a474626
--- /dev/null
+++ b/fs/ceph/acl.c
@@ -0,0 +1,326 @@
+/*
+ * linux/fs/ceph/acl.c
+ *
+ * Copyright (C) 2013 Guangliang Zhao, lucienc...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include linux/ceph/ceph_debug.h
+#include linux/fs.h
+#include linux/string.h
+#include linux/xattr.h
+#include linux/posix_acl_xattr.h
+#include linux/posix_acl.h
+#include linux/sched.h
+#include linux/slab.h
+
+#include super.h
+
+static inline void ceph_set_cached_acl(struct inode *inode,
+int type, struct posix_acl *acl)
+{
+struct ceph_inode_info *ci = ceph_inode(inode);
+int issued;
+
+spin_lock(ci-i_ceph_lock);
+issued = __ceph_caps_issued(ci, NULL);
+if (issued (CEPH_CAP_XATTR_EXCL | CEPH_CAP_XATTR_SHARED)) {
+set_cached_acl(inode, type, acl);
+ci-i_aclcache_gen = ci-i_rdcache_gen;
+}
+spin_unlock(ci-i_ceph_lock);
+
+}
+
+static inline struct posix_acl *ceph_get_cached_acl(struct inode *inode,
+int type)
+{
+struct ceph_inode_info *ci = ceph_inode(inode);
+struct posix_acl *acl = NULL;
+
+spin_lock(ci-i_ceph_lock);
+if (ci-i_aclcache_gen == ci-i_rdcache_gen)
+acl = get_cached_acl(inode, type);
+spin_unlock(ci-i_ceph_lock);
+
+return acl;
+}
+
+struct posix_acl *ceph_get_acl(struct inode *inode, int type)
+{
+int size;
+const char *name;
+char *value = NULL;
+struct posix_acl *acl;
+
+if (!IS_POSIXACL(inode))
+return NULL;
+
+acl = ceph_get_cached_acl(inode, type);
+if (acl != ACL_NOT_CACHED)
+return acl;
If client does not own cap, it can not rely on the cached acl, in that case,
ceph_get_cached_acl() will return NULL rather than ACL_NOT_CACHED.
It will forbid to do the following synchronous MDS consultation.
+
+switch (type) {
+case ACL_TYPE_ACCESS:
+name = POSIX_ACL_XATTR_ACCESS;
+break;
+case ACL_TYPE_DEFAULT:
+name = POSIX_ACL_XATTR_DEFAULT;
+break;
+default:
+BUG();
+}
+
+size = __ceph_getxattr(inode, name, , 0);
+if (size 0) {
+value = kzalloc(size, GFP_NOFS);
+if (!value)
+return ERR_PTR(-ENOMEM);
+size = __ceph_getxattr(inode, name, value, size);
+}
+
+if (size 0)
+acl = posix_acl_from_xattr(init_user_ns, value, size);
+else if (size ==
Re: [PATCH v4] ceph: add acl for cephfs
Hi,
It seems to me there are three issues, you can take a look below if
they are really there,
On 11/08/2013 01:23 PM, Guangliang Zhao wrote:
v4: check the validity before set/get_cached_acl()
v3: handle the attr change in ceph_set_acl()
v2: remove some redundant code in ceph_setattr()
Signed-off-by: Guangliang Zhao lucienc...@gmail.com
---
fs/ceph/Kconfig | 13 +++
fs/ceph/Makefile |1 +
fs/ceph/acl.c| 326 ++
fs/ceph/dir.c|5 +
fs/ceph/inode.c | 14 +++
fs/ceph/super.c |4 +
fs/ceph/super.h | 35 ++
fs/ceph/xattr.c | 60 --
8 files changed, 446 insertions(+), 12 deletions(-)
create mode 100644 fs/ceph/acl.c
diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
index ac9a2ef..264e9bf 100644
--- a/fs/ceph/Kconfig
+++ b/fs/ceph/Kconfig
@@ -25,3 +25,16 @@ config CEPH_FSCACHE
caching support for Ceph clients using FS-Cache
endif
+
+config CEPH_FS_POSIX_ACL
+ bool Ceph POSIX Access Control Lists
+ depends on CEPH_FS
+ select FS_POSIX_ACL
+ help
+ POSIX Access Control Lists (ACLs) support permissions for users and
+ groups beyond the owner/group/world scheme.
+
+ To learn more about Access Control Lists, visit the POSIX ACLs for
+ Linux website http://acl.bestbits.at/.
+
+ If you don't know what Access Control Lists are, say N
diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
index 32e3010..85a4230 100644
--- a/fs/ceph/Makefile
+++ b/fs/ceph/Makefile
@@ -10,3 +10,4 @@ ceph-y := super.o inode.o dir.o file.o locks.o addr.o ioctl.o
\
debugfs.o
ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
+ceph-$(CONFIG_CEPH_FS_POSIX_ACL) += acl.o
diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
new file mode 100644
index 000..a474626
--- /dev/null
+++ b/fs/ceph/acl.c
@@ -0,0 +1,326 @@
+/*
+ * linux/fs/ceph/acl.c
+ *
+ * Copyright (C) 2013 Guangliang Zhao, lucienc...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include linux/ceph/ceph_debug.h
+#include linux/fs.h
+#include linux/string.h
+#include linux/xattr.h
+#include linux/posix_acl_xattr.h
+#include linux/posix_acl.h
+#include linux/sched.h
+#include linux/slab.h
+
+#include super.h
+
+static inline void ceph_set_cached_acl(struct inode *inode,
+ int type, struct posix_acl *acl)
+{
+ struct ceph_inode_info *ci = ceph_inode(inode);
+ int issued;
+
+ spin_lock(ci-i_ceph_lock);
+ issued = __ceph_caps_issued(ci, NULL);
+ if (issued (CEPH_CAP_XATTR_EXCL | CEPH_CAP_XATTR_SHARED)) {
+ set_cached_acl(inode, type, acl);
+ ci-i_aclcache_gen = ci-i_rdcache_gen;
+ }
+ spin_unlock(ci-i_ceph_lock);
+
+}
+
+static inline struct posix_acl *ceph_get_cached_acl(struct inode *inode,
+ int type)
+{
+ struct ceph_inode_info *ci = ceph_inode(inode);
+ struct posix_acl *acl = NULL;
+
+ spin_lock(ci-i_ceph_lock);
+ if (ci-i_aclcache_gen == ci-i_rdcache_gen)
+ acl = get_cached_acl(inode, type);
+ spin_unlock(ci-i_ceph_lock);
+
+ return acl;
+}
+
+struct posix_acl *ceph_get_acl(struct inode *inode, int type)
+{
+ int size;
+ const char *name;
+ char *value = NULL;
+ struct posix_acl *acl;
+
+ if (!IS_POSIXACL(inode))
+ return NULL;
+
+ acl = ceph_get_cached_acl(inode, type);
+ if (acl != ACL_NOT_CACHED)
+ return acl;
If client does not own cap, it can not rely on the cached acl, in that
case, ceph_get_cached_acl() will return NULL rather than ACL_NOT_CACHED.
It will forbid to do the following synchronous MDS consultation.
+
+ switch (type) {
+ case ACL_TYPE_ACCESS:
+ name = POSIX_ACL_XATTR_ACCESS;
+ break;
+ case ACL_TYPE_DEFAULT:
+ name = POSIX_ACL_XATTR_DEFAULT;
+ break;
+ default:
+ BUG();
+ }
+
+ size = __ceph_getxattr(inode, name, , 0);
+ if (size 0) {
+ value = kzalloc(size, GFP_NOFS);
+ if (!value)
+ return ERR_PTR(-ENOMEM);
+ size = __ceph_getxattr(inode, name, value, size);
+ }
+
+ if (size 0)
+
Re: [PATCH v4] ceph: add acl for cephfs
Hi,
thank you for working on this.
On 11/08/2013 01:23 PM, Guangliang Zhao wrote:
v4: check the validity before set/get_cached_acl()
v3: handle the attr change in ceph_set_acl()
v2: remove some redundant code in ceph_setattr()
Signed-off-by: Guangliang Zhao lucienc...@gmail.com
---
fs/ceph/Kconfig | 13 +++
fs/ceph/Makefile |1 +
fs/ceph/acl.c| 326
++
fs/ceph/dir.c|5 +
fs/ceph/inode.c | 14 +++
fs/ceph/super.c |4 +
fs/ceph/super.h | 35 ++
fs/ceph/xattr.c | 60 --
8 files changed, 446 insertions(+), 12 deletions(-)
create mode 100644 fs/ceph/acl.c
diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
index ac9a2ef..264e9bf 100644
--- a/fs/ceph/Kconfig
+++ b/fs/ceph/Kconfig
@@ -25,3 +25,16 @@ config CEPH_FSCACHE
caching support for Ceph clients using FS-Cache
endif
+
+config CEPH_FS_POSIX_ACL
+ bool Ceph POSIX Access Control Lists
+ depends on CEPH_FS
+ select FS_POSIX_ACL
+ help
+ POSIX Access Control Lists (ACLs) support permissions for users and
+ groups beyond the owner/group/world scheme.
+
+ To learn more about Access Control Lists, visit the POSIX ACLs for
+ Linux website http://acl.bestbits.at/.
+
+ If you don't know what Access Control Lists are, say N
diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
index 32e3010..85a4230 100644
--- a/fs/ceph/Makefile
+++ b/fs/ceph/Makefile
@@ -10,3 +10,4 @@ ceph-y := super.o inode.o dir.o file.o locks.o addr.o
ioctl.o \
debugfs.o
ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
+ceph-$(CONFIG_CEPH_FS_POSIX_ACL) += acl.o
diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
new file mode 100644
index 000..a474626
--- /dev/null
+++ b/fs/ceph/acl.c
@@ -0,0 +1,326 @@
+/*
+ * linux/fs/ceph/acl.c
+ *
+ * Copyright (C) 2013 Guangliang Zhao, lucienc...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 021110-1307, USA.
+ */
+
+#include linux/ceph/ceph_debug.h
+#include linux/fs.h
+#include linux/string.h
+#include linux/xattr.h
+#include linux/posix_acl_xattr.h
+#include linux/posix_acl.h
+#include linux/sched.h
+#include linux/slab.h
+
+#include super.h
+
+static inline void ceph_set_cached_acl(struct inode *inode,
+ int type, struct posix_acl *acl)
+{
+ struct ceph_inode_info *ci = ceph_inode(inode);
+ int issued;
+
+ spin_lock(ci-i_ceph_lock);
+ issued = __ceph_caps_issued(ci, NULL);
+ if (issued (CEPH_CAP_XATTR_EXCL | CEPH_CAP_XATTR_SHARED)) {
+ set_cached_acl(inode, type, acl);
+ ci-i_aclcache_gen = ci-i_rdcache_gen;
+ }
+ spin_unlock(ci-i_ceph_lock);
+
+}
+
+static inline struct posix_acl *ceph_get_cached_acl(struct inode *inode,
+ int type)
+{
+ struct ceph_inode_info *ci = ceph_inode(inode);
+ struct posix_acl *acl = NULL;
+
+ spin_lock(ci-i_ceph_lock);
+ if (ci-i_aclcache_gen == ci-i_rdcache_gen)
+ acl = get_cached_acl(inode, type);
+ spin_unlock(ci-i_ceph_lock);
+
+ return acl;
+}
i_rdcache_gen is for Fc cap, it's incorrect to use it to validate xattrs. how
about the incremental patch below.
---
diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index a474626..cfa47dc 100644
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -33,14 +33,10 @@ static inline void ceph_set_cached_acl(struct inode *inode,
int type, struct posix_acl *acl)
{
struct ceph_inode_info *ci = ceph_inode(inode);
- int issued;
spin_lock(ci-i_ceph_lock);
- issued = __ceph_caps_issued(ci, NULL);
- if (issued (CEPH_CAP_XATTR_EXCL | CEPH_CAP_XATTR_SHARED)) {
+ if (__ceph_caps_issued_mask(ci, CEPH_CAP_XATTR_SHARED, 0))
set_cached_acl(inode, type, acl);
- ci-i_aclcache_gen = ci-i_rdcache_gen;
- }
spin_unlock(ci-i_ceph_lock);
}
@@ -52,13 +48,18 @@ static inline struct posix_acl *ceph_get_cached_acl(struct
inode *inode,
struct posix_acl *acl = NULL;
spin_lock(ci-i_ceph_lock);
- if (ci-i_aclcache_gen == ci-i_rdcache_gen)
+ if (__ceph_caps_issued_mask(ci, CEPH_CAP_XATTR_SHARED, 0))
acl =
