* [PATCH 4/5] [fs/9P] Add acl mount option
@ 2011-01-26 1:12 Venkateswararao Jujjuri (JV)
2011-01-26 10:06 ` Aneesh Kumar K. V
0 siblings, 1 reply; 3+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2011-01-26 1:12 UTC (permalink / raw)
To: v9fs-developer; +Cc: linux-fsdevel, Venkateswararao Jujjuri (JV)
The mount option access=client is overloaded as it assumes acl too.
Adding acl=on option to enable ACL, anyother option or absense of this
flag turns off ACLs at the client.
Ideally, the access mode 'client' should be just like V9FS_ACCESS_USER
except it underscores the location of access check.
Traditional 9P protocol lets the server perform access checks but with
this mode, all the access checks will be performed on the client itself.
Server just follows the client's directive.
Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
---
fs/9p/acl.c | 10 +++++-----
fs/9p/v9fs.c | 34 +++++++++++++++++++++++++++-------
fs/9p/v9fs.h | 6 +++++-
fs/9p/vfs_super.c | 2 +-
4 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index 0a2e480..48be5c3 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -59,7 +59,7 @@ int v9fs_get_acl(struct inode *inode, struct p9_fid *fid)
struct v9fs_session_info *v9ses;
v9ses = v9fs_inode2v9ses(inode);
- if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
+ if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON) {
set_cached_acl(inode, ACL_TYPE_DEFAULT, NULL);
set_cached_acl(inode, ACL_TYPE_ACCESS, NULL);
return 0;
@@ -104,9 +104,9 @@ int v9fs_check_acl(struct inode *inode, int mask, unsigned int flags)
return -ECHILD;
v9ses = v9fs_inode2v9ses(inode);
- if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
+ if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON) {
/*
- * On access = client mode get the acl
+ * On access = client and acl = on mode get the acl
* values from the server
*/
return 0;
@@ -264,7 +264,7 @@ static int v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
/*
* We allow set/get/list of acl when access=client is not specified
*/
- if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
+ if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)
return v9fs_remote_get_acl(dentry, name, buffer, size, type);
acl = v9fs_get_cached_acl(dentry->d_inode, type);
@@ -315,7 +315,7 @@ static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name,
* set the attribute on the remote. Without even looking at the
* xattr value. We leave it to the server to validate
*/
- if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
+ if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)
return v9fs_remote_set_acl(dentry, name,
value, size, flags, type);
diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
index d34f293..f936433 100644
--- a/fs/9p/v9fs.c
+++ b/fs/9p/v9fs.c
@@ -55,7 +55,7 @@ enum {
/* Cache options */
Opt_cache_loose, Opt_fscache,
/* Access options */
- Opt_access,
+ Opt_access, Opt_acl,
/* Error token */
Opt_err
};
@@ -73,6 +73,7 @@ static const match_table_t tokens = {
{Opt_fscache, "fscache"},
{Opt_cachetag, "cachetag=%s"},
{Opt_access, "access=%s"},
+ {Opt_acl, "acl=%s"},
{Opt_err, NULL}
};
@@ -194,13 +195,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
else if (strcmp(s, "any") == 0)
v9ses->flags |= V9FS_ACCESS_ANY;
else if (strcmp(s, "client") == 0) {
-#ifdef CONFIG_9P_FS_POSIX_ACL
v9ses->flags |= V9FS_ACCESS_CLIENT;
-#else
- P9_DPRINTK(P9_DEBUG_ERROR,
- "Not defined CONFIG_9P_FS_POSIX_ACL. "
- "Ignoring access=client option\n");
-#endif
} else {
v9ses->flags |= V9FS_ACCESS_SINGLE;
v9ses->uid = simple_strtoul(s, &e, 10);
@@ -210,6 +205,27 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
kfree(s);
break;
+ case Opt_acl:
+ s = match_strdup(&args[0]);
+ if (!s) {
+ ret = -ENOMEM;
+ P9_DPRINTK(P9_DEBUG_ERROR,
+ "problem allocating copy of acl arg\n");
+ goto free_and_return;
+ }
+ v9ses->flags &= ~V9FS_ACL_MASK;
+ if (strcmp(s, "on") == 0) {
+#ifdef CONFIG_9P_FS_POSIX_ACL
+ v9ses->flags |= V9FS_ACL_ON;
+#else
+ P9_DPRINTK(P9_DEBUG_ERROR,
+ "Not defined CONFIG_9P_FS_POSIX_ACL. "
+ "Ignoring acl=on option\n");
+#endif
+ }
+ kfree(s);
+ break;
+
default:
continue;
}
@@ -294,6 +310,10 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
*/
v9ses->flags &= ~V9FS_ACCESS_MASK;
v9ses->flags |= V9FS_ACCESS_USER;
+ /*
+ * We support ACLs only in dotl and V9FS_ACCESS_CLIENT
+ */
+ v9ses->flags &= ~V9FS_ACL_MASK;
}
/*FIXME !! */
/* for legacy mode, fall back to V9FS_ACCESS_ANY */
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index c4b5d88..f3bad79 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -28,8 +28,10 @@
* @V9FS_PROTO_2000L: whether or not to use 9P2000.l extensions
* @V9FS_ACCESS_SINGLE: only the mounting user can access the hierarchy
* @V9FS_ACCESS_USER: a new attach will be issued for every user (default)
+ * @V9FS_ACCESS_CLIENT: Just like user, but access check is performed on client.
* @V9FS_ACCESS_ANY: use a single attach for all users
* @V9FS_ACCESS_MASK: bit mask of different ACCESS options
+ * @V9FS_ACL_ON: If ACLs are enforced
*
* Session flags reflect options selected by users at mount time
*/
@@ -37,13 +39,15 @@
V9FS_ACCESS_USER | \
V9FS_ACCESS_CLIENT)
#define V9FS_ACCESS_MASK V9FS_ACCESS_ANY
+#define V9FS_ACL_MASK V9FS_ACL_ON
enum p9_session_flags {
V9FS_PROTO_2000U = 0x01,
V9FS_PROTO_2000L = 0x02,
V9FS_ACCESS_SINGLE = 0x04,
V9FS_ACCESS_USER = 0x08,
- V9FS_ACCESS_CLIENT = 0x10
+ V9FS_ACCESS_CLIENT = 0x10,
+ V9FS_ACL_ON = 0x20
};
/* possible values of ->cache */
diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
index dbaabe3..357d3b4 100644
--- a/fs/9p/vfs_super.c
+++ b/fs/9p/vfs_super.c
@@ -91,7 +91,7 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
MS_NOATIME;
#ifdef CONFIG_9P_FS_POSIX_ACL
- if ((v9ses->flags & V9FS_ACCESS_MASK) == V9FS_ACCESS_CLIENT)
+ if ((v9ses->flags & V9FS_ACL_MASK) == V9FS_ACL_ON)
sb->s_flags |= MS_POSIXACL;
#endif
--
1.6.5.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 4/5] [fs/9P] Add acl mount option
2011-01-26 1:12 [PATCH 4/5] [fs/9P] Add acl mount option Venkateswararao Jujjuri (JV)
@ 2011-01-26 10:06 ` Aneesh Kumar K. V
2011-01-26 20:09 ` Venkateswararao Jujjuri (JV)
0 siblings, 1 reply; 3+ messages in thread
From: Aneesh Kumar K. V @ 2011-01-26 10:06 UTC (permalink / raw)
To: Venkateswararao Jujjuri (JV), v9fs-developer
Cc: linux-fsdevel, Venkateswararao Jujjuri (JV)
On Tue, 25 Jan 2011 17:12:42 -0800, "Venkateswararao Jujjuri (JV)" <jvrao@linux.vnet.ibm.com> wrote:
> The mount option access=client is overloaded as it assumes acl too.
> Adding acl=on option to enable ACL, anyother option or absense of this
> flag turns off ACLs at the client.
>
> Ideally, the access mode 'client' should be just like V9FS_ACCESS_USER
> except it underscores the location of access check.
> Traditional 9P protocol lets the server perform access checks but with
> this mode, all the access checks will be performed on the client itself.
> Server just follows the client's directive.
>
> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
> ---
> fs/9p/acl.c | 10 +++++-----
> fs/9p/v9fs.c | 34 +++++++++++++++++++++++++++-------
> fs/9p/v9fs.h | 6 +++++-
> fs/9p/vfs_super.c | 2 +-
> 4 files changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
> index 0a2e480..48be5c3 100644
> --- a/fs/9p/acl.c
> +++ b/fs/9p/acl.c
> @@ -59,7 +59,7 @@ int v9fs_get_acl(struct inode *inode, struct p9_fid *fid)
> struct v9fs_session_info *v9ses;
>
> v9ses = v9fs_inode2v9ses(inode);
> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON) {
I guess what we need is
if (((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) &&
((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)) {
the current feature should restrict acl option only with access=client,
and access=client should be default enabled for dotl.
> set_cached_acl(inode, ACL_TYPE_DEFAULT, NULL);
> set_cached_acl(inode, ACL_TYPE_ACCESS, NULL);
> return 0;
> @@ -104,9 +104,9 @@ int v9fs_check_acl(struct inode *inode, int mask, unsigned int flags)
> return -ECHILD;
>
> v9ses = v9fs_inode2v9ses(inode);
> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON) {
> /*
> - * On access = client mode get the acl
> + * On access = client and acl = on mode get the acl
> * values from the server
> */
> return 0;
> @@ -264,7 +264,7 @@ static int v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
> /*
> * We allow set/get/list of acl when access=client is not specified
> */
> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)
> return v9fs_remote_get_acl(dentry, name, buffer, size, type);
>
> acl = v9fs_get_cached_acl(dentry->d_inode, type);
> @@ -315,7 +315,7 @@ static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name,
> * set the attribute on the remote. Without even looking at the
> * xattr value. We leave it to the server to validate
> */
> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)
> return v9fs_remote_set_acl(dentry, name,
> value, size, flags, type);
>
> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
> index d34f293..f936433 100644
> --- a/fs/9p/v9fs.c
> +++ b/fs/9p/v9fs.c
> @@ -55,7 +55,7 @@ enum {
> /* Cache options */
> Opt_cache_loose, Opt_fscache,
> /* Access options */
> - Opt_access,
> + Opt_access, Opt_acl,
> /* Error token */
> Opt_err
> };
> @@ -73,6 +73,7 @@ static const match_table_t tokens = {
> {Opt_fscache, "fscache"},
> {Opt_cachetag, "cachetag=%s"},
> {Opt_access, "access=%s"},
> + {Opt_acl, "acl=%s"},
why not just say -o posix_acl ?. That way i can later say -o richacl -o
selinux etc.
> {Opt_err, NULL}
> };
>
> @@ -194,13 +195,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
> else if (strcmp(s, "any") == 0)
> v9ses->flags |= V9FS_ACCESS_ANY;
> else if (strcmp(s, "client") == 0) {
> -#ifdef CONFIG_9P_FS_POSIX_ACL
> v9ses->flags |= V9FS_ACCESS_CLIENT;
> -#else
> - P9_DPRINTK(P9_DEBUG_ERROR,
> - "Not defined CONFIG_9P_FS_POSIX_ACL. "
> - "Ignoring access=client option\n");
> -#endif
> } else {
> v9ses->flags |= V9FS_ACCESS_SINGLE;
> v9ses->uid = simple_strtoul(s, &e, 10);
> @@ -210,6 +205,27 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
> kfree(s);
> break;
>
> + case Opt_acl:
> + s = match_strdup(&args[0]);
> + if (!s) {
> + ret = -ENOMEM;
> + P9_DPRINTK(P9_DEBUG_ERROR,
> + "problem allocating copy of acl arg\n");
> + goto free_and_return;
> + }
> + v9ses->flags &= ~V9FS_ACL_MASK;
is this to support acl=off ? Local file system needs a method to disable
acl because most of them support changing default mount options, For
9p i guess default is what we have in the code, so if default is
disabled acl, then we need an option to enable. or if decide to enable
acl by default we need to have an option to disable it.
We also need to make sure this is option is available only for dotl .
> + if (strcmp(s, "on") == 0) {
> +#ifdef CONFIG_9P_FS_POSIX_ACL
> + v9ses->flags |= V9FS_ACL_ON;
It would be better
v9ses->flags |= V9FS_POSIX_ACL;
Presence of the bit indicate whether acl is enabled or not, why do
we need the #define to say an "_ON" ?
> +#else
> + P9_DPRINTK(P9_DEBUG_ERROR,
> + "Not defined CONFIG_9P_FS_POSIX_ACL. "
> + "Ignoring acl=on option\n");
> +#endif
> + }
> + kfree(s);
> + break;
> +
> default:
> continue;
> }
> @@ -294,6 +310,10 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
> */
> v9ses->flags &= ~V9FS_ACCESS_MASK;
> v9ses->flags |= V9FS_ACCESS_USER;
> + /*
> + * We support ACLs only in dotl and V9FS_ACCESS_CLIENT
> + */
> + v9ses->flags &= ~V9FS_ACL_MASK;
> }
> /*FIXME !! */
> /* for legacy mode, fall back to V9FS_ACCESS_ANY */
> diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
> index c4b5d88..f3bad79 100644
> --- a/fs/9p/v9fs.h
> +++ b/fs/9p/v9fs.h
> @@ -28,8 +28,10 @@
> * @V9FS_PROTO_2000L: whether or not to use 9P2000.l extensions
> * @V9FS_ACCESS_SINGLE: only the mounting user can access the hierarchy
> * @V9FS_ACCESS_USER: a new attach will be issued for every user (default)
> + * @V9FS_ACCESS_CLIENT: Just like user, but access check is performed on client.
> * @V9FS_ACCESS_ANY: use a single attach for all users
> * @V9FS_ACCESS_MASK: bit mask of different ACCESS options
> + * @V9FS_ACL_ON: If ACLs are enforced
> *
> * Session flags reflect options selected by users at mount time
> */
> @@ -37,13 +39,15 @@
> V9FS_ACCESS_USER | \
> V9FS_ACCESS_CLIENT)
> #define V9FS_ACCESS_MASK V9FS_ACCESS_ANY
> +#define V9FS_ACL_MASK V9FS_ACL_ON
>
> enum p9_session_flags {
> V9FS_PROTO_2000U = 0x01,
> V9FS_PROTO_2000L = 0x02,
> V9FS_ACCESS_SINGLE = 0x04,
> V9FS_ACCESS_USER = 0x08,
> - V9FS_ACCESS_CLIENT = 0x10
> + V9FS_ACCESS_CLIENT = 0x10,
> + V9FS_ACL_ON = 0x20
> };
>
> /* possible values of ->cache */
> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
> index dbaabe3..357d3b4 100644
> --- a/fs/9p/vfs_super.c
> +++ b/fs/9p/vfs_super.c
> @@ -91,7 +91,7 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
> MS_NOATIME;
>
> #ifdef CONFIG_9P_FS_POSIX_ACL
> - if ((v9ses->flags & V9FS_ACCESS_MASK) == V9FS_ACCESS_CLIENT)
> + if ((v9ses->flags & V9FS_ACL_MASK) == V9FS_ACL_ON)
> sb->s_flags |= MS_POSIXACL;
> #endif
>
Also in the patch is would be nice if we could be explicit about posix
acl.
-aneesh
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 4/5] [fs/9P] Add acl mount option
2011-01-26 10:06 ` Aneesh Kumar K. V
@ 2011-01-26 20:09 ` Venkateswararao Jujjuri (JV)
0 siblings, 0 replies; 3+ messages in thread
From: Venkateswararao Jujjuri (JV) @ 2011-01-26 20:09 UTC (permalink / raw)
To: Aneesh Kumar K. V; +Cc: v9fs-developer, linux-fsdevel
On 1/26/2011 2:06 AM, Aneesh Kumar K. V wrote:
> On Tue, 25 Jan 2011 17:12:42 -0800, "Venkateswararao Jujjuri (JV)" <jvrao@linux.vnet.ibm.com> wrote:
>> The mount option access=client is overloaded as it assumes acl too.
>> Adding acl=on option to enable ACL, anyother option or absense of this
>> flag turns off ACLs at the client.
>>
>> Ideally, the access mode 'client' should be just like V9FS_ACCESS_USER
>> except it underscores the location of access check.
>> Traditional 9P protocol lets the server perform access checks but with
>> this mode, all the access checks will be performed on the client itself.
>> Server just follows the client's directive.
>>
>> Signed-off-by: Venkateswararao Jujjuri <jvrao@linux.vnet.ibm.com>
>> ---
>> fs/9p/acl.c | 10 +++++-----
>> fs/9p/v9fs.c | 34 +++++++++++++++++++++++++++-------
>> fs/9p/v9fs.h | 6 +++++-
>> fs/9p/vfs_super.c | 2 +-
>> 4 files changed, 38 insertions(+), 14 deletions(-)
>>
>> diff --git a/fs/9p/acl.c b/fs/9p/acl.c
>> index 0a2e480..48be5c3 100644
>> --- a/fs/9p/acl.c
>> +++ b/fs/9p/acl.c
>> @@ -59,7 +59,7 @@ int v9fs_get_acl(struct inode *inode, struct p9_fid *fid)
>> struct v9fs_session_info *v9ses;
>>
>> v9ses = v9fs_inode2v9ses(inode);
>> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
>> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON) {
>
> I guess what we need is
>
> if (((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) &&
> ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)) {
>
> the current feature should restrict acl option only with access=client,
> and access=client should be default enabled for dotl.
It does; if the access is not client I turn off the acl bit. See below
v9fs_session_init() change.
>
>
>> set_cached_acl(inode, ACL_TYPE_DEFAULT, NULL);
>> set_cached_acl(inode, ACL_TYPE_ACCESS, NULL);
>> return 0;
>> @@ -104,9 +104,9 @@ int v9fs_check_acl(struct inode *inode, int mask, unsigned int flags)
>> return -ECHILD;
>>
>> v9ses = v9fs_inode2v9ses(inode);
>> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT) {
>> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON) {
>> /*
>> - * On access = client mode get the acl
>> + * On access = client and acl = on mode get the acl
>> * values from the server
>> */
>> return 0;
>> @@ -264,7 +264,7 @@ static int v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
>> /*
>> * We allow set/get/list of acl when access=client is not specified
>> */
>> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
>> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)
>> return v9fs_remote_get_acl(dentry, name, buffer, size, type);
>>
>> acl = v9fs_get_cached_acl(dentry->d_inode, type);
>> @@ -315,7 +315,7 @@ static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name,
>> * set the attribute on the remote. Without even looking at the
>> * xattr value. We leave it to the server to validate
>> */
>> - if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
>> + if ((v9ses->flags & V9FS_ACL_MASK) != V9FS_ACL_ON)
>> return v9fs_remote_set_acl(dentry, name,
>> value, size, flags, type);
>>
>> diff --git a/fs/9p/v9fs.c b/fs/9p/v9fs.c
>> index d34f293..f936433 100644
>> --- a/fs/9p/v9fs.c
>> +++ b/fs/9p/v9fs.c
>> @@ -55,7 +55,7 @@ enum {
>> /* Cache options */
>> Opt_cache_loose, Opt_fscache,
>> /* Access options */
>> - Opt_access,
>> + Opt_access, Opt_acl,
>> /* Error token */
>> Opt_err
>> };
>> @@ -73,6 +73,7 @@ static const match_table_t tokens = {
>> {Opt_fscache, "fscache"},
>> {Opt_cachetag, "cachetag=%s"},
>> {Opt_access, "access=%s"},
>> + {Opt_acl, "acl=%s"},
>
> why not just say -o posix_acl ?. That way i can later say -o richacl -o
> selinux etc.
Good point. How about no underscore posixacl?
>
>
>> {Opt_err, NULL}
>> };
>>
>> @@ -194,13 +195,7 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
>> else if (strcmp(s, "any") == 0)
>> v9ses->flags |= V9FS_ACCESS_ANY;
>> else if (strcmp(s, "client") == 0) {
>> -#ifdef CONFIG_9P_FS_POSIX_ACL
>> v9ses->flags |= V9FS_ACCESS_CLIENT;
>> -#else
>> - P9_DPRINTK(P9_DEBUG_ERROR,
>> - "Not defined CONFIG_9P_FS_POSIX_ACL. "
>> - "Ignoring access=client option\n");
>> -#endif
>> } else {
>> v9ses->flags |= V9FS_ACCESS_SINGLE;
>> v9ses->uid = simple_strtoul(s, &e, 10);
>> @@ -210,6 +205,27 @@ static int v9fs_parse_options(struct v9fs_session_info *v9ses, char *opts)
>> kfree(s);
>> break;
>>
>> + case Opt_acl:
>> + s = match_strdup(&args[0]);
>> + if (!s) {
>> + ret = -ENOMEM;
>> + P9_DPRINTK(P9_DEBUG_ERROR,
>> + "problem allocating copy of acl arg\n");
>> + goto free_and_return;
>> + }
>> + v9ses->flags &= ~V9FS_ACL_MASK;
>
> is this to support acl=off ? Local file system needs a method to disable
> acl because most of them support changing default mount options, For
> 9p i guess default is what we have in the code, so if default is
> disabled acl, then we need an option to enable. or if decide to enable
> acl by default we need to have an option to disable it.
>
> We also need to make sure this is option is available only for dotl .
By default we are off; this statement is just for completeness.
>
>> + if (strcmp(s, "on") == 0) {
>> +#ifdef CONFIG_9P_FS_POSIX_ACL
>> + v9ses->flags |= V9FS_ACL_ON;
>
>
> It would be better
> v9ses->flags |= V9FS_POSIX_ACL;
yes; will change it.
>
>
> Presence of the bit indicate whether acl is enabled or not, why do
> we need the #define to say an "_ON" ?
>
>
>> +#else
>> + P9_DPRINTK(P9_DEBUG_ERROR,
>> + "Not defined CONFIG_9P_FS_POSIX_ACL. "
>> + "Ignoring acl=on option\n");
>> +#endif
>> + }
>> + kfree(s);
>> + break;
>> +
>> default:
>> continue;
>> }
>> @@ -294,6 +310,10 @@ struct p9_fid *v9fs_session_init(struct v9fs_session_info *v9ses,
>> */
>> v9ses->flags &= ~V9FS_ACCESS_MASK;
>> v9ses->flags |= V9FS_ACCESS_USER;
>> + /*
>> + * We support ACLs only in dotl and V9FS_ACCESS_CLIENT
>> + */
>> + v9ses->flags &= ~V9FS_ACL_MASK;
>> }
>> /*FIXME !! */
>> /* for legacy mode, fall back to V9FS_ACCESS_ANY */
>> diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
>> index c4b5d88..f3bad79 100644
>> --- a/fs/9p/v9fs.h
>> +++ b/fs/9p/v9fs.h
>> @@ -28,8 +28,10 @@
>> * @V9FS_PROTO_2000L: whether or not to use 9P2000.l extensions
>> * @V9FS_ACCESS_SINGLE: only the mounting user can access the hierarchy
>> * @V9FS_ACCESS_USER: a new attach will be issued for every user (default)
>> + * @V9FS_ACCESS_CLIENT: Just like user, but access check is performed on client.
>> * @V9FS_ACCESS_ANY: use a single attach for all users
>> * @V9FS_ACCESS_MASK: bit mask of different ACCESS options
>> + * @V9FS_ACL_ON: If ACLs are enforced
>> *
>> * Session flags reflect options selected by users at mount time
>> */
>> @@ -37,13 +39,15 @@
>> V9FS_ACCESS_USER | \
>> V9FS_ACCESS_CLIENT)
>> #define V9FS_ACCESS_MASK V9FS_ACCESS_ANY
>> +#define V9FS_ACL_MASK V9FS_ACL_ON
>>
>> enum p9_session_flags {
>> V9FS_PROTO_2000U = 0x01,
>> V9FS_PROTO_2000L = 0x02,
>> V9FS_ACCESS_SINGLE = 0x04,
>> V9FS_ACCESS_USER = 0x08,
>> - V9FS_ACCESS_CLIENT = 0x10
>> + V9FS_ACCESS_CLIENT = 0x10,
>> + V9FS_ACL_ON = 0x20
>> };
>>
>> /* possible values of ->cache */
>> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
>> index dbaabe3..357d3b4 100644
>> --- a/fs/9p/vfs_super.c
>> +++ b/fs/9p/vfs_super.c
>> @@ -91,7 +91,7 @@ v9fs_fill_super(struct super_block *sb, struct v9fs_session_info *v9ses,
>> MS_NOATIME;
>>
>> #ifdef CONFIG_9P_FS_POSIX_ACL
>> - if ((v9ses->flags & V9FS_ACCESS_MASK) == V9FS_ACCESS_CLIENT)
>> + if ((v9ses->flags & V9FS_ACL_MASK) == V9FS_ACL_ON)
>> sb->s_flags |= MS_POSIXACL;
>> #endif
>>
>
> Also in the patch is would be nice if we could be explicit about posix
> acl.
Yes; Also I will send another patch making access=client default for 9P2000.L
for all other versions we can leave the default to user.
Thanks for your comments.
- JV
>
> -aneesh
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-01-26 20:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-26 1:12 [PATCH 4/5] [fs/9P] Add acl mount option Venkateswararao Jujjuri (JV)
2011-01-26 10:06 ` Aneesh Kumar K. V
2011-01-26 20:09 ` Venkateswararao Jujjuri (JV)
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.