* [PATCH v2 0/3] Minor orangefs xattr cleanups
@ 2016-05-30 9:25 Andreas Gruenbacher
2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andreas Gruenbacher @ 2016-05-30 9:25 UTC (permalink / raw)
To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel
Rebased onto current mainline.
Andreas
Andreas Gruenbacher (3):
orangefs: Remove useless defines
orangefs: Remove redundant "trusted." xattr handler
orangefs: Remove useless xattr prefix arguments
fs/orangefs/acl.c | 17 +++---
fs/orangefs/file.c | 2 -
fs/orangefs/orangefs-kernel.h | 13 -----
fs/orangefs/xattr.c | 125 +++++++++---------------------------------
4 files changed, 34 insertions(+), 123 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] orangefs: Remove useless defines
2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher
@ 2016-05-30 9:25 ` Andreas Gruenbacher
2016-05-30 9:26 ` [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler Andreas Gruenbacher
2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher
2 siblings, 0 replies; 6+ messages in thread
From: Andreas Gruenbacher @ 2016-05-30 9:25 UTC (permalink / raw)
To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel
The ORANGEFS_XATTR_INDEX_ defines are unused; the ORANGEFS_XATTR_NAME_
defines only obfuscate the code.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/orangefs/acl.c | 8 ++++----
fs/orangefs/file.c | 4 ++--
fs/orangefs/orangefs-kernel.h | 11 -----------
fs/orangefs/xattr.c | 17 ++++++-----------
4 files changed, 12 insertions(+), 28 deletions(-)
diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
index 03f89db..df24864 100644
--- a/fs/orangefs/acl.c
+++ b/fs/orangefs/acl.c
@@ -18,10 +18,10 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type)
switch (type) {
case ACL_TYPE_ACCESS:
- key = ORANGEFS_XATTR_NAME_ACL_ACCESS;
+ key = XATTR_NAME_POSIX_ACL_ACCESS;
break;
case ACL_TYPE_DEFAULT:
- key = ORANGEFS_XATTR_NAME_ACL_DEFAULT;
+ key = XATTR_NAME_POSIX_ACL_DEFAULT;
break;
default:
gossip_err("orangefs_get_acl: bogus value of type %d\n", type);
@@ -74,7 +74,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
switch (type) {
case ACL_TYPE_ACCESS:
- name = ORANGEFS_XATTR_NAME_ACL_ACCESS;
+ name = XATTR_NAME_POSIX_ACL_ACCESS;
if (acl) {
umode_t mode = inode->i_mode;
/*
@@ -98,7 +98,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
}
break;
case ACL_TYPE_DEFAULT:
- name = ORANGEFS_XATTR_NAME_ACL_DEFAULT;
+ name = XATTR_NAME_POSIX_ACL_DEFAULT;
break;
default:
gossip_err("%s: invalid type %d!\n", __func__, type);
diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index 491e82c..5160a3f 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -516,7 +516,7 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
if (cmd == FS_IOC_GETFLAGS) {
val = 0;
ret = orangefs_inode_getxattr(file_inode(file),
- ORANGEFS_XATTR_NAME_DEFAULT_PREFIX,
+ "",
"user.pvfs2.meta_hint",
&val, sizeof(val));
if (ret < 0 && ret != -ENODATA)
@@ -549,7 +549,7 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
"orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n",
(unsigned long long)val);
ret = orangefs_inode_setxattr(file_inode(file),
- ORANGEFS_XATTR_NAME_DEFAULT_PREFIX,
+ "",
"user.pvfs2.meta_hint",
&val, sizeof(val), 0);
}
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index 2281882..6503e37 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -119,17 +119,6 @@ struct client_debug_mask {
#define ORANGEFS_CACHE_CREATE_FLAGS 0
#endif /* ((defined ORANGEFS_KERNEL_DEBUG) && (defined CONFIG_DEBUG_SLAB)) */
-/* orangefs xattr and acl related defines */
-#define ORANGEFS_XATTR_INDEX_POSIX_ACL_ACCESS 1
-#define ORANGEFS_XATTR_INDEX_POSIX_ACL_DEFAULT 2
-#define ORANGEFS_XATTR_INDEX_TRUSTED 3
-#define ORANGEFS_XATTR_INDEX_DEFAULT 4
-
-#define ORANGEFS_XATTR_NAME_ACL_ACCESS XATTR_NAME_POSIX_ACL_ACCESS
-#define ORANGEFS_XATTR_NAME_ACL_DEFAULT XATTR_NAME_POSIX_ACL_DEFAULT
-#define ORANGEFS_XATTR_NAME_TRUSTED_PREFIX "trusted."
-#define ORANGEFS_XATTR_NAME_DEFAULT_PREFIX ""
-
/* these functions are defined in orangefs-utils.c */
int orangefs_prepare_cdm_array(char *debug_array_string);
int orangefs_prepare_debugfs_help_string(int);
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index 5893ddd..f387e8a 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -456,7 +456,7 @@ static int orangefs_xattr_set_default(const struct xattr_handler *handler,
int flags)
{
return orangefs_inode_setxattr(inode,
- ORANGEFS_XATTR_NAME_DEFAULT_PREFIX,
+ "",
name,
buffer,
size,
@@ -471,7 +471,7 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler,
size_t size)
{
return orangefs_inode_getxattr(inode,
- ORANGEFS_XATTR_NAME_DEFAULT_PREFIX,
+ "",
name,
buffer,
size);
@@ -487,7 +487,7 @@ static int orangefs_xattr_set_trusted(const struct xattr_handler *handler,
int flags)
{
return orangefs_inode_setxattr(inode,
- ORANGEFS_XATTR_NAME_TRUSTED_PREFIX,
+ XATTR_TRUSTED_PREFIX,
name,
buffer,
size,
@@ -502,25 +502,20 @@ static int orangefs_xattr_get_trusted(const struct xattr_handler *handler,
size_t size)
{
return orangefs_inode_getxattr(inode,
- ORANGEFS_XATTR_NAME_TRUSTED_PREFIX,
+ XATTR_TRUSTED_PREFIX,
name,
buffer,
size);
}
static struct xattr_handler orangefs_xattr_trusted_handler = {
- .prefix = ORANGEFS_XATTR_NAME_TRUSTED_PREFIX,
+ .prefix = XATTR_TRUSTED_PREFIX,
.get = orangefs_xattr_get_trusted,
.set = orangefs_xattr_set_trusted,
};
static struct xattr_handler orangefs_xattr_default_handler = {
- /*
- * NOTE: this is set to be the empty string.
- * so that all un-prefixed xattrs keys get caught
- * here!
- */
- .prefix = ORANGEFS_XATTR_NAME_DEFAULT_PREFIX,
+ .prefix = "", /* match any name => handlers called with full name */
.get = orangefs_xattr_get_default,
.set = orangefs_xattr_set_default,
};
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler
2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher
2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher
@ 2016-05-30 9:26 ` Andreas Gruenbacher
2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher
2 siblings, 0 replies; 6+ messages in thread
From: Andreas Gruenbacher @ 2016-05-30 9:26 UTC (permalink / raw)
To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel
Orangefs has a catch-all xattr handler that effectively does what the
trusted handler does already.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/orangefs/xattr.c | 37 -------------------------------------
1 file changed, 37 deletions(-)
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index f387e8a..640a98f 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -478,42 +478,6 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler,
}
-static int orangefs_xattr_set_trusted(const struct xattr_handler *handler,
- struct dentry *unused,
- struct inode *inode,
- const char *name,
- const void *buffer,
- size_t size,
- int flags)
-{
- return orangefs_inode_setxattr(inode,
- XATTR_TRUSTED_PREFIX,
- name,
- buffer,
- size,
- flags);
-}
-
-static int orangefs_xattr_get_trusted(const struct xattr_handler *handler,
- struct dentry *unused,
- struct inode *inode,
- const char *name,
- void *buffer,
- size_t size)
-{
- return orangefs_inode_getxattr(inode,
- XATTR_TRUSTED_PREFIX,
- name,
- buffer,
- size);
-}
-
-static struct xattr_handler orangefs_xattr_trusted_handler = {
- .prefix = XATTR_TRUSTED_PREFIX,
- .get = orangefs_xattr_get_trusted,
- .set = orangefs_xattr_set_trusted,
-};
-
static struct xattr_handler orangefs_xattr_default_handler = {
.prefix = "", /* match any name => handlers called with full name */
.get = orangefs_xattr_get_default,
@@ -523,7 +487,6 @@ static struct xattr_handler orangefs_xattr_default_handler = {
const struct xattr_handler *orangefs_xattr_handlers[] = {
&posix_acl_access_xattr_handler,
&posix_acl_default_xattr_handler,
- &orangefs_xattr_trusted_handler,
&orangefs_xattr_default_handler,
NULL
};
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments
2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher
2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher
2016-05-30 9:26 ` [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler Andreas Gruenbacher
@ 2016-05-30 9:26 ` Andreas Gruenbacher
2016-06-03 19:44 ` Mike Marshall
2 siblings, 1 reply; 6+ messages in thread
From: Andreas Gruenbacher @ 2016-05-30 9:26 UTC (permalink / raw)
To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel
Passing the attribute prefix and name to orangefs_inode_*xattr as
separate arguments was unnecessary before, but now that the trusted
xattr handler is gone, the prefix is even always the empty string.
Remove it.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/orangefs/acl.c | 9 ++---
fs/orangefs/file.c | 2 --
fs/orangefs/orangefs-kernel.h | 2 --
fs/orangefs/xattr.c | 81 ++++++++++++++-----------------------------
4 files changed, 29 insertions(+), 65 deletions(-)
diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
index df24864..28f2195 100644
--- a/fs/orangefs/acl.c
+++ b/fs/orangefs/acl.c
@@ -43,11 +43,8 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type)
get_khandle_from_ino(inode),
key,
type);
- ret = orangefs_inode_getxattr(inode,
- "",
- key,
- value,
- ORANGEFS_MAX_XATTR_VALUELEN);
+ ret = orangefs_inode_getxattr(inode, key, value,
+ ORANGEFS_MAX_XATTR_VALUELEN);
/* if the key exists, convert it to an in-memory rep */
if (ret > 0) {
acl = posix_acl_from_xattr(&init_user_ns, value, ret);
@@ -131,7 +128,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
* will xlate to a removexattr. However, we don't want removexattr
* complain if attributes does not exist.
*/
- error = orangefs_inode_setxattr(inode, "", name, value, size, 0);
+ error = orangefs_inode_setxattr(inode, name, value, size, 0);
out:
kfree(value);
diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index 5160a3f..526040e 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -516,7 +516,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
if (cmd == FS_IOC_GETFLAGS) {
val = 0;
ret = orangefs_inode_getxattr(file_inode(file),
- "",
"user.pvfs2.meta_hint",
&val, sizeof(val));
if (ret < 0 && ret != -ENODATA)
@@ -549,7 +548,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
"orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n",
(unsigned long long)val);
ret = orangefs_inode_setxattr(file_inode(file),
- "",
"user.pvfs2.meta_hint",
&val, sizeof(val), 0);
}
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index 6503e37..7b542f1 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -517,13 +517,11 @@ __s32 fsid_of_op(struct orangefs_kernel_op_s *op);
int orangefs_flush_inode(struct inode *inode);
ssize_t orangefs_inode_getxattr(struct inode *inode,
- const char *prefix,
const char *name,
void *buffer,
size_t size);
int orangefs_inode_setxattr(struct inode *inode,
- const char *prefix,
const char *name,
const void *value,
size_t size,
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index 640a98f..1165047 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -59,8 +59,8 @@ static inline int convert_to_internal_xattr_flags(int setxattr_flags)
* unless the key does not exist for the file and/or if
* there were errors in fetching the attribute value.
*/
-ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
- const char *name, void *buffer, size_t size)
+ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name,
+ void *buffer, size_t size)
{
struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
struct orangefs_kernel_op_s *new_op = NULL;
@@ -70,12 +70,12 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
int fsgid;
gossip_debug(GOSSIP_XATTR_DEBUG,
- "%s: prefix %s name %s, buffer_size %zd\n",
- __func__, prefix, name, size);
+ "%s: name %s, buffer_size %zd\n",
+ __func__, name, size);
- if ((strlen(name) + strlen(prefix)) >= ORANGEFS_MAX_XATTR_NAMELEN) {
+ if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
gossip_err("Invalid key length (%d)\n",
- (int)(strlen(name) + strlen(prefix)));
+ (int)strlen(name));
return -EINVAL;
}
@@ -97,8 +97,7 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
goto out_unlock;
new_op->upcall.req.getxattr.refn = orangefs_inode->refn;
- ret = snprintf((char *)new_op->upcall.req.getxattr.key,
- ORANGEFS_MAX_XATTR_NAMELEN, "%s%s", prefix, name);
+ strcpy(new_op->upcall.req.getxattr.key, name);
/*
* NOTE: Although keys are meant to be NULL terminated textual
@@ -163,10 +162,8 @@ out_unlock:
return ret;
}
-static int orangefs_inode_removexattr(struct inode *inode,
- const char *prefix,
- const char *name,
- int flags)
+static int orangefs_inode_removexattr(struct inode *inode, const char *name,
+ int flags)
{
struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
struct orangefs_kernel_op_s *new_op = NULL;
@@ -183,12 +180,8 @@ static int orangefs_inode_removexattr(struct inode *inode,
* textual strings, I am going to explicitly pass the
* length just in case we change this later on...
*/
- ret = snprintf((char *)new_op->upcall.req.removexattr.key,
- ORANGEFS_MAX_XATTR_NAMELEN,
- "%s%s",
- (prefix ? prefix : ""),
- name);
- new_op->upcall.req.removexattr.key_sz = ret + 1;
+ strcpy(new_op->upcall.req.removexattr.key, name);
+ new_op->upcall.req.removexattr.key_sz = strlen(name) + 1;
gossip_debug(GOSSIP_XATTR_DEBUG,
"orangefs_inode_removexattr: key %s, key_sz %d\n",
@@ -223,8 +216,8 @@ out_unlock:
* Returns a -ve number on error and 0 on success. Key is text, but value
* can be binary!
*/
-int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
- const char *name, const void *value, size_t size, int flags)
+int orangefs_inode_setxattr(struct inode *inode, const char *name,
+ const void *value, size_t size, int flags)
{
struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
struct orangefs_kernel_op_s *new_op;
@@ -232,8 +225,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
int ret = -ENOMEM;
gossip_debug(GOSSIP_XATTR_DEBUG,
- "%s: prefix %s, name %s, buffer_size %zd\n",
- __func__, prefix, name, size);
+ "%s: name %s, buffer_size %zd\n",
+ __func__, name, size);
if (size >= ORANGEFS_MAX_XATTR_VALUELEN ||
flags < 0) {
@@ -245,29 +238,19 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
internal_flag = convert_to_internal_xattr_flags(flags);
- if (prefix) {
- if (strlen(name) + strlen(prefix) >= ORANGEFS_MAX_XATTR_NAMELEN) {
- gossip_err
- ("orangefs_inode_setxattr: bogus key size (%d)\n",
- (int)(strlen(name) + strlen(prefix)));
- return -EINVAL;
- }
- } else {
- if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
- gossip_err
- ("orangefs_inode_setxattr: bogus key size (%d)\n",
- (int)(strlen(name)));
- return -EINVAL;
- }
+ if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
+ gossip_err
+ ("orangefs_inode_setxattr: bogus key size (%d)\n",
+ (int)(strlen(name)));
+ return -EINVAL;
}
/* This is equivalent to a removexattr */
if (size == 0 && value == NULL) {
gossip_debug(GOSSIP_XATTR_DEBUG,
- "removing xattr (%s%s)\n",
- prefix,
+ "removing xattr (%s)\n",
name);
- return orangefs_inode_removexattr(inode, prefix, name, flags);
+ return orangefs_inode_removexattr(inode, name, flags);
}
gossip_debug(GOSSIP_XATTR_DEBUG,
@@ -288,11 +271,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
* strings, I am going to explicitly pass the length just in
* case we change this later on...
*/
- ret = snprintf((char *)new_op->upcall.req.setxattr.keyval.key,
- ORANGEFS_MAX_XATTR_NAMELEN,
- "%s%s",
- prefix, name);
- new_op->upcall.req.setxattr.keyval.key_sz = ret + 1;
+ strcpy(new_op->upcall.req.setxattr.keyval.key, name);
+ new_op->upcall.req.setxattr.keyval.key_sz = strlen(name) + 1;
memcpy(new_op->upcall.req.setxattr.keyval.val, value, size);
new_op->upcall.req.setxattr.keyval.val_sz = size;
@@ -455,12 +435,7 @@ static int orangefs_xattr_set_default(const struct xattr_handler *handler,
size_t size,
int flags)
{
- return orangefs_inode_setxattr(inode,
- "",
- name,
- buffer,
- size,
- flags);
+ return orangefs_inode_setxattr(inode, name, buffer, size, flags);
}
static int orangefs_xattr_get_default(const struct xattr_handler *handler,
@@ -470,11 +445,7 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler,
void *buffer,
size_t size)
{
- return orangefs_inode_getxattr(inode,
- "",
- name,
- buffer,
- size);
+ return orangefs_inode_getxattr(inode, name, buffer, size);
}
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments
2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher
@ 2016-06-03 19:44 ` Mike Marshall
2016-06-04 9:02 ` Andreas Gruenbacher
0 siblings, 1 reply; 6+ messages in thread
From: Mike Marshall @ 2016-06-03 19:44 UTC (permalink / raw)
To: Andreas Gruenbacher; +Cc: linux-fsdevel, Mike Marshall
Hi Andreas...
I'm sorry I didn't notice this right away. And that I missed the merge window
with your patch <g>... anyhow...
We use the return value in this one line you changed, our userspace code gets
ill when we send it (-ENOMEM +1) as a key length... would you change the
patch one more time, or ack my change please... I'll get these in at the next
merge window...
# git diff
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index 1165047..bdb407e 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -97,7 +97,11 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const ch
goto out_unlock;
new_op->upcall.req.getxattr.refn = orangefs_inode->refn;
- strcpy(new_op->upcall.req.getxattr.key, name);
+
+ ret = snprintf((char *)new_op->upcall.req.getxattr.key,
+ ORANGEFS_MAX_XATTR_NAMELEN,
+ "%s",
+ name);
/*
* NOTE: Although keys are meant to be NULL terminated textual
On Mon, May 30, 2016 at 5:26 AM, Andreas Gruenbacher
<agruenba@redhat.com> wrote:
> Passing the attribute prefix and name to orangefs_inode_*xattr as
> separate arguments was unnecessary before, but now that the trusted
> xattr handler is gone, the prefix is even always the empty string.
> Remove it.
>
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
> fs/orangefs/acl.c | 9 ++---
> fs/orangefs/file.c | 2 --
> fs/orangefs/orangefs-kernel.h | 2 --
> fs/orangefs/xattr.c | 81 ++++++++++++++-----------------------------
> 4 files changed, 29 insertions(+), 65 deletions(-)
>
> diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
> index df24864..28f2195 100644
> --- a/fs/orangefs/acl.c
> +++ b/fs/orangefs/acl.c
> @@ -43,11 +43,8 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type)
> get_khandle_from_ino(inode),
> key,
> type);
> - ret = orangefs_inode_getxattr(inode,
> - "",
> - key,
> - value,
> - ORANGEFS_MAX_XATTR_VALUELEN);
> + ret = orangefs_inode_getxattr(inode, key, value,
> + ORANGEFS_MAX_XATTR_VALUELEN);
> /* if the key exists, convert it to an in-memory rep */
> if (ret > 0) {
> acl = posix_acl_from_xattr(&init_user_ns, value, ret);
> @@ -131,7 +128,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
> * will xlate to a removexattr. However, we don't want removexattr
> * complain if attributes does not exist.
> */
> - error = orangefs_inode_setxattr(inode, "", name, value, size, 0);
> + error = orangefs_inode_setxattr(inode, name, value, size, 0);
>
> out:
> kfree(value);
> diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
> index 5160a3f..526040e 100644
> --- a/fs/orangefs/file.c
> +++ b/fs/orangefs/file.c
> @@ -516,7 +516,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
> if (cmd == FS_IOC_GETFLAGS) {
> val = 0;
> ret = orangefs_inode_getxattr(file_inode(file),
> - "",
> "user.pvfs2.meta_hint",
> &val, sizeof(val));
> if (ret < 0 && ret != -ENODATA)
> @@ -549,7 +548,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
> "orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n",
> (unsigned long long)val);
> ret = orangefs_inode_setxattr(file_inode(file),
> - "",
> "user.pvfs2.meta_hint",
> &val, sizeof(val), 0);
> }
> diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
> index 6503e37..7b542f1 100644
> --- a/fs/orangefs/orangefs-kernel.h
> +++ b/fs/orangefs/orangefs-kernel.h
> @@ -517,13 +517,11 @@ __s32 fsid_of_op(struct orangefs_kernel_op_s *op);
> int orangefs_flush_inode(struct inode *inode);
>
> ssize_t orangefs_inode_getxattr(struct inode *inode,
> - const char *prefix,
> const char *name,
> void *buffer,
> size_t size);
>
> int orangefs_inode_setxattr(struct inode *inode,
> - const char *prefix,
> const char *name,
> const void *value,
> size_t size,
> diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
> index 640a98f..1165047 100644
> --- a/fs/orangefs/xattr.c
> +++ b/fs/orangefs/xattr.c
> @@ -59,8 +59,8 @@ static inline int convert_to_internal_xattr_flags(int setxattr_flags)
> * unless the key does not exist for the file and/or if
> * there were errors in fetching the attribute value.
> */
> -ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
> - const char *name, void *buffer, size_t size)
> +ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name,
> + void *buffer, size_t size)
> {
> struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
> struct orangefs_kernel_op_s *new_op = NULL;
> @@ -70,12 +70,12 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
> int fsgid;
>
> gossip_debug(GOSSIP_XATTR_DEBUG,
> - "%s: prefix %s name %s, buffer_size %zd\n",
> - __func__, prefix, name, size);
> + "%s: name %s, buffer_size %zd\n",
> + __func__, name, size);
>
> - if ((strlen(name) + strlen(prefix)) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> + if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> gossip_err("Invalid key length (%d)\n",
> - (int)(strlen(name) + strlen(prefix)));
> + (int)strlen(name));
> return -EINVAL;
> }
>
> @@ -97,8 +97,7 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
> goto out_unlock;
>
> new_op->upcall.req.getxattr.refn = orangefs_inode->refn;
> - ret = snprintf((char *)new_op->upcall.req.getxattr.key,
> - ORANGEFS_MAX_XATTR_NAMELEN, "%s%s", prefix, name);
> + strcpy(new_op->upcall.req.getxattr.key, name);
>
> /*
> * NOTE: Although keys are meant to be NULL terminated textual
> @@ -163,10 +162,8 @@ out_unlock:
> return ret;
> }
>
> -static int orangefs_inode_removexattr(struct inode *inode,
> - const char *prefix,
> - const char *name,
> - int flags)
> +static int orangefs_inode_removexattr(struct inode *inode, const char *name,
> + int flags)
> {
> struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
> struct orangefs_kernel_op_s *new_op = NULL;
> @@ -183,12 +180,8 @@ static int orangefs_inode_removexattr(struct inode *inode,
> * textual strings, I am going to explicitly pass the
> * length just in case we change this later on...
> */
> - ret = snprintf((char *)new_op->upcall.req.removexattr.key,
> - ORANGEFS_MAX_XATTR_NAMELEN,
> - "%s%s",
> - (prefix ? prefix : ""),
> - name);
> - new_op->upcall.req.removexattr.key_sz = ret + 1;
> + strcpy(new_op->upcall.req.removexattr.key, name);
> + new_op->upcall.req.removexattr.key_sz = strlen(name) + 1;
>
> gossip_debug(GOSSIP_XATTR_DEBUG,
> "orangefs_inode_removexattr: key %s, key_sz %d\n",
> @@ -223,8 +216,8 @@ out_unlock:
> * Returns a -ve number on error and 0 on success. Key is text, but value
> * can be binary!
> */
> -int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
> - const char *name, const void *value, size_t size, int flags)
> +int orangefs_inode_setxattr(struct inode *inode, const char *name,
> + const void *value, size_t size, int flags)
> {
> struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
> struct orangefs_kernel_op_s *new_op;
> @@ -232,8 +225,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
> int ret = -ENOMEM;
>
> gossip_debug(GOSSIP_XATTR_DEBUG,
> - "%s: prefix %s, name %s, buffer_size %zd\n",
> - __func__, prefix, name, size);
> + "%s: name %s, buffer_size %zd\n",
> + __func__, name, size);
>
> if (size >= ORANGEFS_MAX_XATTR_VALUELEN ||
> flags < 0) {
> @@ -245,29 +238,19 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
>
> internal_flag = convert_to_internal_xattr_flags(flags);
>
> - if (prefix) {
> - if (strlen(name) + strlen(prefix) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> - gossip_err
> - ("orangefs_inode_setxattr: bogus key size (%d)\n",
> - (int)(strlen(name) + strlen(prefix)));
> - return -EINVAL;
> - }
> - } else {
> - if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> - gossip_err
> - ("orangefs_inode_setxattr: bogus key size (%d)\n",
> - (int)(strlen(name)));
> - return -EINVAL;
> - }
> + if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
> + gossip_err
> + ("orangefs_inode_setxattr: bogus key size (%d)\n",
> + (int)(strlen(name)));
> + return -EINVAL;
> }
>
> /* This is equivalent to a removexattr */
> if (size == 0 && value == NULL) {
> gossip_debug(GOSSIP_XATTR_DEBUG,
> - "removing xattr (%s%s)\n",
> - prefix,
> + "removing xattr (%s)\n",
> name);
> - return orangefs_inode_removexattr(inode, prefix, name, flags);
> + return orangefs_inode_removexattr(inode, name, flags);
> }
>
> gossip_debug(GOSSIP_XATTR_DEBUG,
> @@ -288,11 +271,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
> * strings, I am going to explicitly pass the length just in
> * case we change this later on...
> */
> - ret = snprintf((char *)new_op->upcall.req.setxattr.keyval.key,
> - ORANGEFS_MAX_XATTR_NAMELEN,
> - "%s%s",
> - prefix, name);
> - new_op->upcall.req.setxattr.keyval.key_sz = ret + 1;
> + strcpy(new_op->upcall.req.setxattr.keyval.key, name);
> + new_op->upcall.req.setxattr.keyval.key_sz = strlen(name) + 1;
> memcpy(new_op->upcall.req.setxattr.keyval.val, value, size);
> new_op->upcall.req.setxattr.keyval.val_sz = size;
>
> @@ -455,12 +435,7 @@ static int orangefs_xattr_set_default(const struct xattr_handler *handler,
> size_t size,
> int flags)
> {
> - return orangefs_inode_setxattr(inode,
> - "",
> - name,
> - buffer,
> - size,
> - flags);
> + return orangefs_inode_setxattr(inode, name, buffer, size, flags);
> }
>
> static int orangefs_xattr_get_default(const struct xattr_handler *handler,
> @@ -470,11 +445,7 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler,
> void *buffer,
> size_t size)
> {
> - return orangefs_inode_getxattr(inode,
> - "",
> - name,
> - buffer,
> - size);
> + return orangefs_inode_getxattr(inode, name, buffer, size);
>
> }
>
> --
> 2.5.5
>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments
2016-06-03 19:44 ` Mike Marshall
@ 2016-06-04 9:02 ` Andreas Gruenbacher
0 siblings, 0 replies; 6+ messages in thread
From: Andreas Gruenbacher @ 2016-06-04 9:02 UTC (permalink / raw)
To: Mike Marshall; +Cc: Andreas Gruenbacher, linux-fsdevel
Mike,
On Fri, Jun 3, 2016 at 9:44 PM, Mike Marshall <hubcap@omnibond.com> wrote:
> We use the return value in this one line you changed, our userspace code gets
> ill when we send it (-ENOMEM +1) as a key length...
ah, my mistake. Here's a fixed version.
Thanks,
Andreas
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/orangefs/acl.c | 9 ++---
fs/orangefs/file.c | 2 --
fs/orangefs/orangefs-kernel.h | 2 --
fs/orangefs/xattr.c | 83 ++++++++++++++-----------------------------
4 files changed, 30 insertions(+), 66 deletions(-)
diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
index df24864..28f2195 100644
--- a/fs/orangefs/acl.c
+++ b/fs/orangefs/acl.c
@@ -43,11 +43,8 @@ struct posix_acl *orangefs_get_acl(struct inode *inode, int type)
get_khandle_from_ino(inode),
key,
type);
- ret = orangefs_inode_getxattr(inode,
- "",
- key,
- value,
- ORANGEFS_MAX_XATTR_VALUELEN);
+ ret = orangefs_inode_getxattr(inode, key, value,
+ ORANGEFS_MAX_XATTR_VALUELEN);
/* if the key exists, convert it to an in-memory rep */
if (ret > 0) {
acl = posix_acl_from_xattr(&init_user_ns, value, ret);
@@ -131,7 +128,7 @@ int orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
* will xlate to a removexattr. However, we don't want removexattr
* complain if attributes does not exist.
*/
- error = orangefs_inode_setxattr(inode, "", name, value, size, 0);
+ error = orangefs_inode_setxattr(inode, name, value, size, 0);
out:
kfree(value);
diff --git a/fs/orangefs/file.c b/fs/orangefs/file.c
index 5160a3f..526040e 100644
--- a/fs/orangefs/file.c
+++ b/fs/orangefs/file.c
@@ -516,7 +516,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
if (cmd == FS_IOC_GETFLAGS) {
val = 0;
ret = orangefs_inode_getxattr(file_inode(file),
- "",
"user.pvfs2.meta_hint",
&val, sizeof(val));
if (ret < 0 && ret != -ENODATA)
@@ -549,7 +548,6 @@ static long orangefs_ioctl(struct file *file, unsigned int cmd, unsigned long ar
"orangefs_ioctl: FS_IOC_SETFLAGS: %llu\n",
(unsigned long long)val);
ret = orangefs_inode_setxattr(file_inode(file),
- "",
"user.pvfs2.meta_hint",
&val, sizeof(val), 0);
}
diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h
index 6503e37..7b542f1 100644
--- a/fs/orangefs/orangefs-kernel.h
+++ b/fs/orangefs/orangefs-kernel.h
@@ -517,13 +517,11 @@ __s32 fsid_of_op(struct orangefs_kernel_op_s *op);
int orangefs_flush_inode(struct inode *inode);
ssize_t orangefs_inode_getxattr(struct inode *inode,
- const char *prefix,
const char *name,
void *buffer,
size_t size);
int orangefs_inode_setxattr(struct inode *inode,
- const char *prefix,
const char *name,
const void *value,
size_t size,
diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c
index 640a98f..73a0c34 100644
--- a/fs/orangefs/xattr.c
+++ b/fs/orangefs/xattr.c
@@ -59,8 +59,8 @@ static inline int convert_to_internal_xattr_flags(int setxattr_flags)
* unless the key does not exist for the file and/or if
* there were errors in fetching the attribute value.
*/
-ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
- const char *name, void *buffer, size_t size)
+ssize_t orangefs_inode_getxattr(struct inode *inode, const char *name,
+ void *buffer, size_t size)
{
struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
struct orangefs_kernel_op_s *new_op = NULL;
@@ -70,12 +70,12 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
int fsgid;
gossip_debug(GOSSIP_XATTR_DEBUG,
- "%s: prefix %s name %s, buffer_size %zd\n",
- __func__, prefix, name, size);
+ "%s: name %s, buffer_size %zd\n",
+ __func__, name, size);
- if ((strlen(name) + strlen(prefix)) >= ORANGEFS_MAX_XATTR_NAMELEN) {
+ if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
gossip_err("Invalid key length (%d)\n",
- (int)(strlen(name) + strlen(prefix)));
+ (int)strlen(name));
return -EINVAL;
}
@@ -97,15 +97,14 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix,
goto out_unlock;
new_op->upcall.req.getxattr.refn = orangefs_inode->refn;
- ret = snprintf((char *)new_op->upcall.req.getxattr.key,
- ORANGEFS_MAX_XATTR_NAMELEN, "%s%s", prefix, name);
+ strcpy(new_op->upcall.req.getxattr.key, name);
/*
* NOTE: Although keys are meant to be NULL terminated textual
* strings, I am going to explicitly pass the length just in case
* we change this later on...
*/
- new_op->upcall.req.getxattr.key_sz = ret + 1;
+ new_op->upcall.req.getxattr.key_sz = strlen(name) + 1;
ret = service_operation(new_op, "orangefs_inode_getxattr",
get_interruptible_flag(inode));
@@ -163,10 +162,8 @@ out_unlock:
return ret;
}
-static int orangefs_inode_removexattr(struct inode *inode,
- const char *prefix,
- const char *name,
- int flags)
+static int orangefs_inode_removexattr(struct inode *inode, const char *name,
+ int flags)
{
struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
struct orangefs_kernel_op_s *new_op = NULL;
@@ -183,12 +180,8 @@ static int orangefs_inode_removexattr(struct inode *inode,
* textual strings, I am going to explicitly pass the
* length just in case we change this later on...
*/
- ret = snprintf((char *)new_op->upcall.req.removexattr.key,
- ORANGEFS_MAX_XATTR_NAMELEN,
- "%s%s",
- (prefix ? prefix : ""),
- name);
- new_op->upcall.req.removexattr.key_sz = ret + 1;
+ strcpy(new_op->upcall.req.removexattr.key, name);
+ new_op->upcall.req.removexattr.key_sz = strlen(name) + 1;
gossip_debug(GOSSIP_XATTR_DEBUG,
"orangefs_inode_removexattr: key %s, key_sz %d\n",
@@ -223,8 +216,8 @@ out_unlock:
* Returns a -ve number on error and 0 on success. Key is text, but value
* can be binary!
*/
-int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
- const char *name, const void *value, size_t size, int flags)
+int orangefs_inode_setxattr(struct inode *inode, const char *name,
+ const void *value, size_t size, int flags)
{
struct orangefs_inode_s *orangefs_inode = ORANGEFS_I(inode);
struct orangefs_kernel_op_s *new_op;
@@ -232,8 +225,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
int ret = -ENOMEM;
gossip_debug(GOSSIP_XATTR_DEBUG,
- "%s: prefix %s, name %s, buffer_size %zd\n",
- __func__, prefix, name, size);
+ "%s: name %s, buffer_size %zd\n",
+ __func__, name, size);
if (size >= ORANGEFS_MAX_XATTR_VALUELEN ||
flags < 0) {
@@ -245,29 +238,19 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
internal_flag = convert_to_internal_xattr_flags(flags);
- if (prefix) {
- if (strlen(name) + strlen(prefix) >= ORANGEFS_MAX_XATTR_NAMELEN) {
- gossip_err
- ("orangefs_inode_setxattr: bogus key size (%d)\n",
- (int)(strlen(name) + strlen(prefix)));
- return -EINVAL;
- }
- } else {
- if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
- gossip_err
- ("orangefs_inode_setxattr: bogus key size (%d)\n",
- (int)(strlen(name)));
- return -EINVAL;
- }
+ if (strlen(name) >= ORANGEFS_MAX_XATTR_NAMELEN) {
+ gossip_err
+ ("orangefs_inode_setxattr: bogus key size (%d)\n",
+ (int)(strlen(name)));
+ return -EINVAL;
}
/* This is equivalent to a removexattr */
if (size == 0 && value == NULL) {
gossip_debug(GOSSIP_XATTR_DEBUG,
- "removing xattr (%s%s)\n",
- prefix,
+ "removing xattr (%s)\n",
name);
- return orangefs_inode_removexattr(inode, prefix, name, flags);
+ return orangefs_inode_removexattr(inode, name, flags);
}
gossip_debug(GOSSIP_XATTR_DEBUG,
@@ -288,11 +271,8 @@ int orangefs_inode_setxattr(struct inode *inode, const char *prefix,
* strings, I am going to explicitly pass the length just in
* case we change this later on...
*/
- ret = snprintf((char *)new_op->upcall.req.setxattr.keyval.key,
- ORANGEFS_MAX_XATTR_NAMELEN,
- "%s%s",
- prefix, name);
- new_op->upcall.req.setxattr.keyval.key_sz = ret + 1;
+ strcpy(new_op->upcall.req.setxattr.keyval.key, name);
+ new_op->upcall.req.setxattr.keyval.key_sz = strlen(name) + 1;
memcpy(new_op->upcall.req.setxattr.keyval.val, value, size);
new_op->upcall.req.setxattr.keyval.val_sz = size;
@@ -455,12 +435,7 @@ static int orangefs_xattr_set_default(const struct xattr_handler *handler,
size_t size,
int flags)
{
- return orangefs_inode_setxattr(inode,
- "",
- name,
- buffer,
- size,
- flags);
+ return orangefs_inode_setxattr(inode, name, buffer, size, flags);
}
static int orangefs_xattr_get_default(const struct xattr_handler *handler,
@@ -470,11 +445,7 @@ static int orangefs_xattr_get_default(const struct xattr_handler *handler,
void *buffer,
size_t size)
{
- return orangefs_inode_getxattr(inode,
- "",
- name,
- buffer,
- size);
+ return orangefs_inode_getxattr(inode, name, buffer, size);
}
--
2.5.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-04 9:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-30 9:25 [PATCH v2 0/3] Minor orangefs xattr cleanups Andreas Gruenbacher
2016-05-30 9:25 ` [PATCH v2 1/3] orangefs: Remove useless defines Andreas Gruenbacher
2016-05-30 9:26 ` [PATCH v2 2/3] orangefs: Remove redundant "trusted." xattr handler Andreas Gruenbacher
2016-05-30 9:26 ` [PATCH v2 3/3] orangefs: Remove useless xattr prefix arguments Andreas Gruenbacher
2016-06-03 19:44 ` Mike Marshall
2016-06-04 9:02 ` Andreas Gruenbacher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).