All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] several misc patches
@ 2014-02-14  5:29 Guangliang Zhao
  2014-02-14  5:29 ` [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline Guangliang Zhao
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Guangliang Zhao @ 2014-02-14  5:29 UTC (permalink / raw)
  To: ceph-devel; +Cc: sage

Hi,

These are several misc changes: acl option for ceph mount,
ACL inheritable related bug fix, some have in Zheng's patches, 
so this only include symlink's.

Guangliang Zhao (3):
  ceph: make ceph_forget_all_cached_acls() static inline
  ceph: add acl, noacl options for cephfs mount
  ceph: make ceph ACL for symlink inheritable

 fs/ceph/acl.c   |    5 -----
 fs/ceph/dir.c   |    4 ++++
 fs/ceph/super.c |   24 ++++++++++++++++++++----
 fs/ceph/super.h |    6 +++++-
 4 files changed, 29 insertions(+), 10 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline
  2014-02-14  5:29 [PATCH 0/3] several misc patches Guangliang Zhao
@ 2014-02-14  5:29 ` Guangliang Zhao
  2014-02-14 12:29   ` Alex Elder
  2014-02-14  5:29 ` [PATCH 2/3] ceph: add acl, noacl options for cephfs mount Guangliang Zhao
  2014-02-14  5:29 ` [PATCH 3/3] ceph: make ceph ACL for symlink inheritable Guangliang Zhao
  2 siblings, 1 reply; 12+ messages in thread
From: Guangliang Zhao @ 2014-02-14  5:29 UTC (permalink / raw)
  To: ceph-devel; +Cc: sage

Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
---
 fs/ceph/acl.c   |    5 -----
 fs/ceph/super.h |    6 +++++-
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index 64fddbc..48af0b3 100644
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -54,11 +54,6 @@ static inline struct posix_acl *ceph_get_cached_acl(struct inode *inode,
 	return acl;
 }
 
-void ceph_forget_all_cached_acls(struct inode *inode)
-{
-	forget_all_cached_acls(inode);
-}
-
 struct posix_acl *ceph_get_acl(struct inode *inode, int type)
 {
 	int size;
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index c299f7d..8851dc2 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -13,6 +13,7 @@
 #include <linux/wait.h>
 #include <linux/writeback.h>
 #include <linux/slab.h>
+#include <linux/posix_acl.h>
 
 #include <linux/ceph/libceph.h>
 
@@ -745,7 +746,10 @@ extern const struct xattr_handler *ceph_xattr_handlers[];
 struct posix_acl *ceph_get_acl(struct inode *, int);
 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);
+static inline void ceph_forget_all_cached_acls(struct inode *inode)
+{
+	forget_all_cached_acls(inode);
+}
 
 #else
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] ceph: add acl, noacl options for cephfs mount
  2014-02-14  5:29 [PATCH 0/3] several misc patches Guangliang Zhao
  2014-02-14  5:29 ` [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline Guangliang Zhao
@ 2014-02-14  5:29 ` Guangliang Zhao
  2014-02-14 13:00   ` Alex Elder
  2014-02-14  5:29 ` [PATCH 3/3] ceph: make ceph ACL for symlink inheritable Guangliang Zhao
  2 siblings, 1 reply; 12+ messages in thread
From: Guangliang Zhao @ 2014-02-14  5:29 UTC (permalink / raw)
  To: ceph-devel; +Cc: sage

Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
---
 fs/ceph/super.c |   24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 2df963f..9d65c08 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -144,7 +144,9 @@ enum {
 	Opt_ino32,
 	Opt_noino32,
 	Opt_fscache,
-	Opt_nofscache
+	Opt_nofscache,
+	Opt_acl,
+	Opt_noacl
 };
 
 static match_table_t fsopt_tokens = {
@@ -172,6 +174,8 @@ static match_table_t fsopt_tokens = {
 	{Opt_noino32, "noino32"},
 	{Opt_fscache, "fsc"},
 	{Opt_nofscache, "nofsc"},
+	{Opt_acl, "acl"},
+	{Opt_noacl, "noacl"},
 	{-1, NULL}
 };
 
@@ -271,6 +275,12 @@ static int parse_fsopt_token(char *c, void *private)
 	case Opt_nofscache:
 		fsopt->flags &= ~CEPH_MOUNT_OPT_FSCACHE;
 		break;
+	case Opt_acl:
+		fsopt->sb_flags |= MS_POSIXACL;
+		break;
+	case Opt_noacl:
+		fsopt->sb_flags &= ~MS_POSIXACL;
+		break;
 	default:
 		BUG_ON(token);
 	}
@@ -438,6 +448,11 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
 	else
 		seq_puts(m, ",nofsc");
 
+	if (fsopt->sb_flags & MS_POSIXACL)
+		seq_puts(m, ",acl");
+	else
+		seq_puts(m, ",noacl");
+
 	if (fsopt->wsize)
 		seq_printf(m, ",wsize=%d", fsopt->wsize);
 	if (fsopt->rsize != CEPH_RSIZE_DEFAULT)
@@ -819,9 +834,6 @@ static int ceph_set_super(struct super_block *s, void *data)
 
 	s->s_flags = fsc->mount_options->sb_flags;
 	s->s_maxbytes = 1ULL << 40;  /* temp value until we get mdsmap */
-#ifdef CONFIG_CEPH_FS_POSIX_ACL
-	s->s_flags |= MS_POSIXACL;
-#endif
 
 	s->s_xattr = ceph_xattr_handlers;
 	s->s_fs_info = fsc;
@@ -911,6 +923,10 @@ static struct dentry *ceph_mount(struct file_system_type *fs_type,
 	struct ceph_options *opt = NULL;
 
 	dout("ceph_mount\n");
+
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+       flags |= MS_POSIXACL;
+#endif
 	err = parse_mount_options(&fsopt, &opt, flags, data, dev_name, &path);
 	if (err < 0) {
 		res = ERR_PTR(err);
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] ceph: make ceph ACL for symlink inheritable
  2014-02-14  5:29 [PATCH 0/3] several misc patches Guangliang Zhao
  2014-02-14  5:29 ` [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline Guangliang Zhao
  2014-02-14  5:29 ` [PATCH 2/3] ceph: add acl, noacl options for cephfs mount Guangliang Zhao
@ 2014-02-14  5:29 ` Guangliang Zhao
  2014-02-14 13:06   ` Alex Elder
  2 siblings, 1 reply; 12+ messages in thread
From: Guangliang Zhao @ 2014-02-14  5:29 UTC (permalink / raw)
  To: ceph-devel; +Cc: sage

Default ACL couldn't be inherited by the symlink in the
parent directory. This resolve it.

Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
---
 fs/ceph/dir.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
index 619616d..2650570 100644
--- a/fs/ceph/dir.c
+++ b/fs/ceph/dir.c
@@ -735,6 +735,10 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
 	if (!err && !req->r_reply_info.head->is_dentry)
 		err = ceph_handle_notrace_create(dir, dentry);
 	ceph_mdsc_put_request(req);
+
+	if (!err)
+		err = ceph_init_acl(dentry, dentry->d_inode, dir);
+
 	if (err)
 		d_drop(dentry);
 	return err;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline
  2014-02-14  5:29 ` [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline Guangliang Zhao
@ 2014-02-14 12:29   ` Alex Elder
  2014-02-16 18:27     ` Sage Weil
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2014-02-14 12:29 UTC (permalink / raw)
  To: Guangliang Zhao, ceph-devel; +Cc: sage

On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
> Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>

Looks good.  Is there a reason we can't just call
forget_all_cached_acls(inode) directly?  Or is it
just so that we have our own complete private ACL
interface?  (I have no problem with it, just asking.)

Reviewed-by: Alex Elder <elder@linaro.org>

> ---
>  fs/ceph/acl.c   |    5 -----
>  fs/ceph/super.h |    6 +++++-
>  2 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
> index 64fddbc..48af0b3 100644
> --- a/fs/ceph/acl.c
> +++ b/fs/ceph/acl.c
> @@ -54,11 +54,6 @@ static inline struct posix_acl *ceph_get_cached_acl(struct inode *inode,
>  	return acl;
>  }
>  
> -void ceph_forget_all_cached_acls(struct inode *inode)
> -{
> -	forget_all_cached_acls(inode);
> -}
> -
>  struct posix_acl *ceph_get_acl(struct inode *inode, int type)
>  {
>  	int size;
> diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> index c299f7d..8851dc2 100644
> --- a/fs/ceph/super.h
> +++ b/fs/ceph/super.h
> @@ -13,6 +13,7 @@
>  #include <linux/wait.h>
>  #include <linux/writeback.h>
>  #include <linux/slab.h>
> +#include <linux/posix_acl.h>
>  
>  #include <linux/ceph/libceph.h>
>  
> @@ -745,7 +746,10 @@ extern const struct xattr_handler *ceph_xattr_handlers[];
>  struct posix_acl *ceph_get_acl(struct inode *, int);
>  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);
> +static inline void ceph_forget_all_cached_acls(struct inode *inode)
> +{
> +	forget_all_cached_acls(inode);
> +}
>  
>  #else
>  
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ceph: add acl, noacl options for cephfs mount
  2014-02-14  5:29 ` [PATCH 2/3] ceph: add acl, noacl options for cephfs mount Guangliang Zhao
@ 2014-02-14 13:00   ` Alex Elder
  2014-02-16 18:26     ` Sage Weil
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2014-02-14 13:00 UTC (permalink / raw)
  To: Guangliang Zhao, ceph-devel; +Cc: sage

On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
> Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>

If CONFIG_CEPH_FS_POSIX_ACL is defined, the default is to
have them enabled; if it is not, the default is to have it
disabled.  But now, whether enabled or not is possible to
enable/disable them using a mount option.

Well, it appears that way.  However fs/ceph/acl.o is not
built unless CONFIG_CEP_FS_POSIX_ACL is enabled.  So the
mount options will appear to be supported but will be
silently ignored.

I don't think you want to require CEPH_FS_POSIX_ACL,
because that requires the inclusion of FS_POSIX_ACL
which may not be desired.

So you should probably make all the code related to
the the acl options (or at least the acl enabled option)
be conditional on CEPH_FS_POSIX_ACL, even though I think
that won't look all that nice.

I have another question/suggestion for you to consider,
below, which is sort of separate from this particular
change.

> ---
>  fs/ceph/super.c |   24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index 2df963f..9d65c08 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -144,7 +144,9 @@ enum {
>  	Opt_ino32,
>  	Opt_noino32,
>  	Opt_fscache,
> -	Opt_nofscache
> +	Opt_nofscache,
> +	Opt_acl,
> +	Opt_noacl
>  };
>  
>  static match_table_t fsopt_tokens = {
> @@ -172,6 +174,8 @@ static match_table_t fsopt_tokens = {
>  	{Opt_noino32, "noino32"},
>  	{Opt_fscache, "fsc"},
>  	{Opt_nofscache, "nofsc"},
> +	{Opt_acl, "acl"},
> +	{Opt_noacl, "noacl"},
>  	{-1, NULL}
>  };
>  
> @@ -271,6 +275,12 @@ static int parse_fsopt_token(char *c, void *private)
>  	case Opt_nofscache:
>  		fsopt->flags &= ~CEPH_MOUNT_OPT_FSCACHE;
>  		break;
> +	case Opt_acl:
> +		fsopt->sb_flags |= MS_POSIXACL;
> +		break;
> +	case Opt_noacl:
> +		fsopt->sb_flags &= ~MS_POSIXACL;

This hunk would be affected by my suggestion, below.

> +		break;
>  	default:
>  		BUG_ON(token);
>  	}
> @@ -438,6 +448,11 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
>  	else
>  		seq_puts(m, ",nofsc");
>  
> +	if (fsopt->sb_flags & MS_POSIXACL)
> +		seq_puts(m, ",acl");
> +	else
> +		seq_puts(m, ",noacl");
> +
>  	if (fsopt->wsize)
>  		seq_printf(m, ",wsize=%d", fsopt->wsize);
>  	if (fsopt->rsize != CEPH_RSIZE_DEFAULT)
> @@ -819,9 +834,6 @@ static int ceph_set_super(struct super_block *s, void *data)
>  
>  	s->s_flags = fsc->mount_options->sb_flags;
>  	s->s_maxbytes = 1ULL << 40;  /* temp value until we get mdsmap */
> -#ifdef CONFIG_CEPH_FS_POSIX_ACL
> -	s->s_flags |= MS_POSIXACL;
> -#endif
>  
>  	s->s_xattr = ceph_xattr_handlers;
>  	s->s_fs_info = fsc;
> @@ -911,6 +923,10 @@ static struct dentry *ceph_mount(struct file_system_type *fs_type,
>  	struct ceph_options *opt = NULL;
>  
>  	dout("ceph_mount\n");
> +
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +       flags |= MS_POSIXACL;

(You appear to have spaces here instead of a tab.)

> +#endif
>  	err = parse_mount_options(&fsopt, &opt, flags, data, dev_name, &path);
>  	if (err < 0) {
>  		res = ERR_PTR(err);
> 

OK, here's my question/suggestion.  Why are superblock/mount options
not treated the same way as the rest?  I think there should be a
CEPH_MOUNT_OPT_FS_ACL flag added to the set available for
ceph_mount_options->flags, and just get rid of the fs_flags field
in that structure.  Then a new function like ceph_set_super_flags()
could be called from ceph_set_super() which would pick out superblock
related flags from the ceph_mount_option->flags field and set the
appropriate bits in super_block->flags.

					-Alex


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] ceph: make ceph ACL for symlink inheritable
  2014-02-14  5:29 ` [PATCH 3/3] ceph: make ceph ACL for symlink inheritable Guangliang Zhao
@ 2014-02-14 13:06   ` Alex Elder
  2014-02-16 18:27     ` Sage Weil
  0 siblings, 1 reply; 12+ messages in thread
From: Alex Elder @ 2014-02-14 13:06 UTC (permalink / raw)
  To: Guangliang Zhao, ceph-devel; +Cc: sage

On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
> Default ACL couldn't be inherited by the symlink in the
> parent directory. This resolve it.

This looks good to me, and it seems to be consistent
with what other file systems do.

Reviewed-by: Alex Elder <elder@linaro.org>

> Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
> ---
>  fs/ceph/dir.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> index 619616d..2650570 100644
> --- a/fs/ceph/dir.c
> +++ b/fs/ceph/dir.c
> @@ -735,6 +735,10 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
>  	if (!err && !req->r_reply_info.head->is_dentry)
>  		err = ceph_handle_notrace_create(dir, dentry);
>  	ceph_mdsc_put_request(req);
> +
> +	if (!err)
> +		err = ceph_init_acl(dentry, dentry->d_inode, dir);
> +
>  	if (err)
>  		d_drop(dentry);
>  	return err;
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ceph: add acl, noacl options for cephfs mount
  2014-02-14 13:00   ` Alex Elder
@ 2014-02-16 18:26     ` Sage Weil
  2014-02-17 14:27       ` Guangliang Zhao
  2014-02-18  0:08       ` Alex Elder
  0 siblings, 2 replies; 12+ messages in thread
From: Sage Weil @ 2014-02-16 18:26 UTC (permalink / raw)
  To: Alex Elder; +Cc: Guangliang Zhao, ceph-devel

Hi Alex, Guangliang,

On Fri, 14 Feb 2014, Alex Elder wrote:
> On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
> > Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
> 
> If CONFIG_CEPH_FS_POSIX_ACL is defined, the default is to
> have them enabled; if it is not, the default is to have it
> disabled.  But now, whether enabled or not is possible to
> enable/disable them using a mount option.
> 
> Well, it appears that way.  However fs/ceph/acl.o is not
> built unless CONFIG_CEP_FS_POSIX_ACL is enabled.  So the
> mount options will appear to be supported but will be
> silently ignored.
> 
> I don't think you want to require CEPH_FS_POSIX_ACL,
> because that requires the inclusion of FS_POSIX_ACL
> which may not be desired.
> 
> So you should probably make all the code related to
> the the acl options (or at least the acl enabled option)
> be conditional on CEPH_FS_POSIX_ACL, even though I think
> that won't look all that nice.

I've gone ahead and done this.  Please see the patch below...

sage

From bc199dc7a5e891fe7d95bf9cd583a9e222a8e1fd Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@inktank.com>
Date: Sun, 16 Feb 2014 10:05:29 -0800
Subject: [PATCH] ceph: add acl, noacl options for cephfs mount

Make the 'acl' option dependent on having ACL support compiled in.  Make
the 'noacl' option work even without it so that one can always ask it to
be off and not error out on mount when it is not supported.

Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
Signed-off-by: Sage Weil <sage@inktank.com>
---
 fs/ceph/super.c | 32 ++++++++++++++++++++++++++++----
 1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/fs/ceph/super.c b/fs/ceph/super.c
index 2df963f..10a4ccb 100644
--- a/fs/ceph/super.c
+++ b/fs/ceph/super.c
@@ -144,7 +144,11 @@ enum {
 	Opt_ino32,
 	Opt_noino32,
 	Opt_fscache,
-	Opt_nofscache
+	Opt_nofscache,
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	Opt_acl,
+#endif
+	Opt_noacl
 };
 
 static match_table_t fsopt_tokens = {
@@ -172,6 +176,10 @@ static match_table_t fsopt_tokens = {
 	{Opt_noino32, "noino32"},
 	{Opt_fscache, "fsc"},
 	{Opt_nofscache, "nofsc"},
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	{Opt_acl, "acl"},
+#endif
+	{Opt_noacl, "noacl"},
 	{-1, NULL}
 };
 
@@ -271,6 +279,14 @@ static int parse_fsopt_token(char *c, void *private)
 	case Opt_nofscache:
 		fsopt->flags &= ~CEPH_MOUNT_OPT_FSCACHE;
 		break;
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	case Opt_acl:
+		fsopt->sb_flags |= MS_POSIXACL;
+		break;
+#endif
+	case Opt_noacl:
+		fsopt->sb_flags &= ~MS_POSIXACL;
+		break;
 	default:
 		BUG_ON(token);
 	}
@@ -438,6 +454,13 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
 	else
 		seq_puts(m, ",nofsc");
 
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	if (fsopt->sb_flags & MS_POSIXACL)
+		seq_puts(m, ",acl");
+	else
+		seq_puts(m, ",noacl");
+#endif
+
 	if (fsopt->wsize)
 		seq_printf(m, ",wsize=%d", fsopt->wsize);
 	if (fsopt->rsize != CEPH_RSIZE_DEFAULT)
@@ -819,9 +842,6 @@ static int ceph_set_super(struct super_block *s, void *data)
 
 	s->s_flags = fsc->mount_options->sb_flags;
 	s->s_maxbytes = 1ULL << 40;  /* temp value until we get mdsmap */
-#ifdef CONFIG_CEPH_FS_POSIX_ACL
-	s->s_flags |= MS_POSIXACL;
-#endif
 
 	s->s_xattr = ceph_xattr_handlers;
 	s->s_fs_info = fsc;
@@ -911,6 +931,10 @@ static struct dentry *ceph_mount(struct file_system_type *fs_type,
 	struct ceph_options *opt = NULL;
 
 	dout("ceph_mount\n");
+
+#ifdef CONFIG_CEPH_FS_POSIX_ACL
+	flags |= MS_POSIXACL;
+#endif
 	err = parse_mount_options(&fsopt, &opt, flags, data, dev_name, &path);
 	if (err < 0) {
 		res = ERR_PTR(err);
-- 
1.8.5.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline
  2014-02-14 12:29   ` Alex Elder
@ 2014-02-16 18:27     ` Sage Weil
  0 siblings, 0 replies; 12+ messages in thread
From: Sage Weil @ 2014-02-16 18:27 UTC (permalink / raw)
  To: Alex Elder; +Cc: Guangliang Zhao, ceph-devel

Applied.

On Fri, 14 Feb 2014, Alex Elder wrote:

> On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
> > Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
> 
> Looks good.  Is there a reason we can't just call
> forget_all_cached_acls(inode) directly?  Or is it
> just so that we have our own complete private ACL
> interface?  (I have no problem with it, just asking.)
> 
> Reviewed-by: Alex Elder <elder@linaro.org>
> 
> > ---
> >  fs/ceph/acl.c   |    5 -----
> >  fs/ceph/super.h |    6 +++++-
> >  2 files changed, 5 insertions(+), 6 deletions(-)
> > 
> > diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
> > index 64fddbc..48af0b3 100644
> > --- a/fs/ceph/acl.c
> > +++ b/fs/ceph/acl.c
> > @@ -54,11 +54,6 @@ static inline struct posix_acl *ceph_get_cached_acl(struct inode *inode,
> >  	return acl;
> >  }
> >  
> > -void ceph_forget_all_cached_acls(struct inode *inode)
> > -{
> > -	forget_all_cached_acls(inode);
> > -}
> > -
> >  struct posix_acl *ceph_get_acl(struct inode *inode, int type)
> >  {
> >  	int size;
> > diff --git a/fs/ceph/super.h b/fs/ceph/super.h
> > index c299f7d..8851dc2 100644
> > --- a/fs/ceph/super.h
> > +++ b/fs/ceph/super.h
> > @@ -13,6 +13,7 @@
> >  #include <linux/wait.h>
> >  #include <linux/writeback.h>
> >  #include <linux/slab.h>
> > +#include <linux/posix_acl.h>
> >  
> >  #include <linux/ceph/libceph.h>
> >  
> > @@ -745,7 +746,10 @@ extern const struct xattr_handler *ceph_xattr_handlers[];
> >  struct posix_acl *ceph_get_acl(struct inode *, int);
> >  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);
> > +static inline void ceph_forget_all_cached_acls(struct inode *inode)
> > +{
> > +	forget_all_cached_acls(inode);
> > +}
> >  
> >  #else
> >  
> > 
> 
> --
> 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
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] ceph: make ceph ACL for symlink inheritable
  2014-02-14 13:06   ` Alex Elder
@ 2014-02-16 18:27     ` Sage Weil
  0 siblings, 0 replies; 12+ messages in thread
From: Sage Weil @ 2014-02-16 18:27 UTC (permalink / raw)
  To: Alex Elder; +Cc: Guangliang Zhao, ceph-devel

This one is subsumed by Zheng's updated patch.

All of this is now in the testing branch.  Assuming all goes well I'll 
send it to Linus in the next day or two.

Thanks!
sage

On Fri, 14 Feb 2014, Alex Elder wrote:

> On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
> > Default ACL couldn't be inherited by the symlink in the
> > parent directory. This resolve it.
> 
> This looks good to me, and it seems to be consistent
> with what other file systems do.
> 
> Reviewed-by: Alex Elder <elder@linaro.org>
> 
> > Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
> > ---
> >  fs/ceph/dir.c |    4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/fs/ceph/dir.c b/fs/ceph/dir.c
> > index 619616d..2650570 100644
> > --- a/fs/ceph/dir.c
> > +++ b/fs/ceph/dir.c
> > @@ -735,6 +735,10 @@ static int ceph_symlink(struct inode *dir, struct dentry *dentry,
> >  	if (!err && !req->r_reply_info.head->is_dentry)
> >  		err = ceph_handle_notrace_create(dir, dentry);
> >  	ceph_mdsc_put_request(req);
> > +
> > +	if (!err)
> > +		err = ceph_init_acl(dentry, dentry->d_inode, dir);
> > +
> >  	if (err)
> >  		d_drop(dentry);
> >  	return err;
> > 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ceph: add acl, noacl options for cephfs mount
  2014-02-16 18:26     ` Sage Weil
@ 2014-02-17 14:27       ` Guangliang Zhao
  2014-02-18  0:08       ` Alex Elder
  1 sibling, 0 replies; 12+ messages in thread
From: Guangliang Zhao @ 2014-02-17 14:27 UTC (permalink / raw)
  To: Sage Weil; +Cc: Alex Elder, ceph-devel

On Sun, Feb 16, 2014 at 10:26:54AM -0800, Sage Weil wrote:
> Hi Alex, Guangliang,
> 
> On Fri, 14 Feb 2014, Alex Elder wrote:
> > On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
> > > Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
> > 
> > If CONFIG_CEPH_FS_POSIX_ACL is defined, the default is to
> > have them enabled; if it is not, the default is to have it
> > disabled.  But now, whether enabled or not is possible to
> > enable/disable them using a mount option.
> > 
> > Well, it appears that way.  However fs/ceph/acl.o is not
> > built unless CONFIG_CEP_FS_POSIX_ACL is enabled.  So the
> > mount options will appear to be supported but will be
> > silently ignored.
> > 
> > I don't think you want to require CEPH_FS_POSIX_ACL,
> > because that requires the inclusion of FS_POSIX_ACL
> > which may not be desired.
> > 
> > So you should probably make all the code related to
> > the the acl options (or at least the acl enabled option)
> > be conditional on CEPH_FS_POSIX_ACL, even though I think
> > that won't look all that nice.
> 
> I've gone ahead and done this.  Please see the patch below...

It looks good to me.

> 
> sage
> 
> From bc199dc7a5e891fe7d95bf9cd583a9e222a8e1fd Mon Sep 17 00:00:00 2001
> From: Sage Weil <sage@inktank.com>
> Date: Sun, 16 Feb 2014 10:05:29 -0800
> Subject: [PATCH] ceph: add acl, noacl options for cephfs mount
> 
> Make the 'acl' option dependent on having ACL support compiled in.  Make
> the 'noacl' option work even without it so that one can always ask it to
> be off and not error out on mount when it is not supported.
> 
> Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
> Signed-off-by: Sage Weil <sage@inktank.com>
> ---
>  fs/ceph/super.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index 2df963f..10a4ccb 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -144,7 +144,11 @@ enum {
>  	Opt_ino32,
>  	Opt_noino32,
>  	Opt_fscache,
> -	Opt_nofscache
> +	Opt_nofscache,
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	Opt_acl,
> +#endif
> +	Opt_noacl
>  };
>  
>  static match_table_t fsopt_tokens = {
> @@ -172,6 +176,10 @@ static match_table_t fsopt_tokens = {
>  	{Opt_noino32, "noino32"},
>  	{Opt_fscache, "fsc"},
>  	{Opt_nofscache, "nofsc"},
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	{Opt_acl, "acl"},
> +#endif
> +	{Opt_noacl, "noacl"},
>  	{-1, NULL}
>  };
>  
> @@ -271,6 +279,14 @@ static int parse_fsopt_token(char *c, void *private)
>  	case Opt_nofscache:
>  		fsopt->flags &= ~CEPH_MOUNT_OPT_FSCACHE;
>  		break;
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	case Opt_acl:
> +		fsopt->sb_flags |= MS_POSIXACL;
> +		break;
> +#endif
> +	case Opt_noacl:
> +		fsopt->sb_flags &= ~MS_POSIXACL;
> +		break;
>  	default:
>  		BUG_ON(token);
>  	}
> @@ -438,6 +454,13 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
>  	else
>  		seq_puts(m, ",nofsc");
>  
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	if (fsopt->sb_flags & MS_POSIXACL)
> +		seq_puts(m, ",acl");
> +	else
> +		seq_puts(m, ",noacl");
> +#endif
> +
>  	if (fsopt->wsize)
>  		seq_printf(m, ",wsize=%d", fsopt->wsize);
>  	if (fsopt->rsize != CEPH_RSIZE_DEFAULT)
> @@ -819,9 +842,6 @@ static int ceph_set_super(struct super_block *s, void *data)
>  
>  	s->s_flags = fsc->mount_options->sb_flags;
>  	s->s_maxbytes = 1ULL << 40;  /* temp value until we get mdsmap */
> -#ifdef CONFIG_CEPH_FS_POSIX_ACL
> -	s->s_flags |= MS_POSIXACL;
> -#endif
>  
>  	s->s_xattr = ceph_xattr_handlers;
>  	s->s_fs_info = fsc;
> @@ -911,6 +931,10 @@ static struct dentry *ceph_mount(struct file_system_type *fs_type,
>  	struct ceph_options *opt = NULL;
>  
>  	dout("ceph_mount\n");
> +
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	flags |= MS_POSIXACL;
> +#endif
>  	err = parse_mount_options(&fsopt, &opt, flags, data, dev_name, &path);
>  	if (err < 0) {
>  		res = ERR_PTR(err);
> -- 
> 1.8.5.1
> 

-- 
Best regards,
Guangliang

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/3] ceph: add acl, noacl options for cephfs mount
  2014-02-16 18:26     ` Sage Weil
  2014-02-17 14:27       ` Guangliang Zhao
@ 2014-02-18  0:08       ` Alex Elder
  1 sibling, 0 replies; 12+ messages in thread
From: Alex Elder @ 2014-02-18  0:08 UTC (permalink / raw)
  To: Sage Weil; +Cc: Guangliang Zhao, ceph-devel

On 02/16/2014 12:26 PM, Sage Weil wrote:
> Hi Alex, Guangliang,

Looks good to me.

Reviewed-by: Alex Elder <elder@linaro.org>


> On Fri, 14 Feb 2014, Alex Elder wrote:
>> On 02/13/2014 11:29 PM, Guangliang Zhao wrote:
>>> Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
>>
>> If CONFIG_CEPH_FS_POSIX_ACL is defined, the default is to
>> have them enabled; if it is not, the default is to have it
>> disabled.  But now, whether enabled or not is possible to
>> enable/disable them using a mount option.
>>
>> Well, it appears that way.  However fs/ceph/acl.o is not
>> built unless CONFIG_CEP_FS_POSIX_ACL is enabled.  So the
>> mount options will appear to be supported but will be
>> silently ignored.
>>
>> I don't think you want to require CEPH_FS_POSIX_ACL,
>> because that requires the inclusion of FS_POSIX_ACL
>> which may not be desired.
>>
>> So you should probably make all the code related to
>> the the acl options (or at least the acl enabled option)
>> be conditional on CEPH_FS_POSIX_ACL, even though I think
>> that won't look all that nice.
> 
> I've gone ahead and done this.  Please see the patch below...
> 
> sage
> 
> From bc199dc7a5e891fe7d95bf9cd583a9e222a8e1fd Mon Sep 17 00:00:00 2001
> From: Sage Weil <sage@inktank.com>
> Date: Sun, 16 Feb 2014 10:05:29 -0800
> Subject: [PATCH] ceph: add acl, noacl options for cephfs mount
> 
> Make the 'acl' option dependent on having ACL support compiled in.  Make
> the 'noacl' option work even without it so that one can always ask it to
> be off and not error out on mount when it is not supported.
> 
> Signed-off-by: Guangliang Zhao <lucienchao@gmail.com>
> Signed-off-by: Sage Weil <sage@inktank.com>
> ---
>  fs/ceph/super.c | 32 ++++++++++++++++++++++++++++----
>  1 file changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ceph/super.c b/fs/ceph/super.c
> index 2df963f..10a4ccb 100644
> --- a/fs/ceph/super.c
> +++ b/fs/ceph/super.c
> @@ -144,7 +144,11 @@ enum {
>  	Opt_ino32,
>  	Opt_noino32,
>  	Opt_fscache,
> -	Opt_nofscache
> +	Opt_nofscache,
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	Opt_acl,
> +#endif
> +	Opt_noacl
>  };
>  
>  static match_table_t fsopt_tokens = {
> @@ -172,6 +176,10 @@ static match_table_t fsopt_tokens = {
>  	{Opt_noino32, "noino32"},
>  	{Opt_fscache, "fsc"},
>  	{Opt_nofscache, "nofsc"},
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	{Opt_acl, "acl"},
> +#endif
> +	{Opt_noacl, "noacl"},
>  	{-1, NULL}
>  };
>  
> @@ -271,6 +279,14 @@ static int parse_fsopt_token(char *c, void *private)
>  	case Opt_nofscache:
>  		fsopt->flags &= ~CEPH_MOUNT_OPT_FSCACHE;
>  		break;
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	case Opt_acl:
> +		fsopt->sb_flags |= MS_POSIXACL;
> +		break;
> +#endif
> +	case Opt_noacl:
> +		fsopt->sb_flags &= ~MS_POSIXACL;
> +		break;
>  	default:
>  		BUG_ON(token);
>  	}
> @@ -438,6 +454,13 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root)
>  	else
>  		seq_puts(m, ",nofsc");
>  
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	if (fsopt->sb_flags & MS_POSIXACL)
> +		seq_puts(m, ",acl");
> +	else
> +		seq_puts(m, ",noacl");
> +#endif
> +
>  	if (fsopt->wsize)
>  		seq_printf(m, ",wsize=%d", fsopt->wsize);
>  	if (fsopt->rsize != CEPH_RSIZE_DEFAULT)
> @@ -819,9 +842,6 @@ static int ceph_set_super(struct super_block *s, void *data)
>  
>  	s->s_flags = fsc->mount_options->sb_flags;
>  	s->s_maxbytes = 1ULL << 40;  /* temp value until we get mdsmap */
> -#ifdef CONFIG_CEPH_FS_POSIX_ACL
> -	s->s_flags |= MS_POSIXACL;
> -#endif
>  
>  	s->s_xattr = ceph_xattr_handlers;
>  	s->s_fs_info = fsc;
> @@ -911,6 +931,10 @@ static struct dentry *ceph_mount(struct file_system_type *fs_type,
>  	struct ceph_options *opt = NULL;
>  
>  	dout("ceph_mount\n");
> +
> +#ifdef CONFIG_CEPH_FS_POSIX_ACL
> +	flags |= MS_POSIXACL;
> +#endif
>  	err = parse_mount_options(&fsopt, &opt, flags, data, dev_name, &path);
>  	if (err < 0) {
>  		res = ERR_PTR(err);
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-02-18  0:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-14  5:29 [PATCH 0/3] several misc patches Guangliang Zhao
2014-02-14  5:29 ` [PATCH 1/3] ceph: make ceph_forget_all_cached_acls() static inline Guangliang Zhao
2014-02-14 12:29   ` Alex Elder
2014-02-16 18:27     ` Sage Weil
2014-02-14  5:29 ` [PATCH 2/3] ceph: add acl, noacl options for cephfs mount Guangliang Zhao
2014-02-14 13:00   ` Alex Elder
2014-02-16 18:26     ` Sage Weil
2014-02-17 14:27       ` Guangliang Zhao
2014-02-18  0:08       ` Alex Elder
2014-02-14  5:29 ` [PATCH 3/3] ceph: make ceph ACL for symlink inheritable Guangliang Zhao
2014-02-14 13:06   ` Alex Elder
2014-02-16 18:27     ` Sage Weil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.