LU-11109 mdt: handle zero length xattr values correctly

In mdt_getxattr(), set OBD_MD_FLXATTR in mbo_valid of the reply's MDT
body so that the client can distinguish between nonexistent extended
attributes and zero length values. In ll_xattr_list() and
ll_getxattr_common() test for OBD_MD_FLXATTR and return 0 rather than
-ENODATA in the appropriate cases. Add sanity test_102t() to test that
zero length values are handled correctly.

Signed-off-by: John L. Hammond <jhammond@whamcloud.com>
Change-Id: I15649581c26dc52e83ca714b44f8372f29954ed5
Reviewed-on: https://review.whamcloud.com/32755
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: Mike Pershin <mpershin@whamcloud.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>

diff --git a/lustre/llite/xattr.c b/lustre/llite/xattr.c
index ac1dd91..7d98825 100644 (file)
--- a/lustre/llite/xattr.c
+++ b/lustre/llite/xattr.c
@@ -379,8 +379,13 @@ getxattr_nocache:
                LASSERT(body);
 
                /* only detect the xattr size */
-               if (size == 0)
+               if (size == 0) {
+                       /* LU-11109: Older MDTs do not distinguish
+                        * between nonexistent xattrs and zero length
+                        * values in this case. Newer MDTs will return
+                        * -ENODATA or set OBD_MD_FLXATTR. */
                        GOTO(out, rc = body->mbo_eadatasize);
+               }
 
                if (size < body->mbo_eadatasize) {
                        CERROR("server bug: replied size %u > %u\n",
@@ -388,8 +393,23 @@ getxattr_nocache:
                        GOTO(out, rc = -ERANGE);
                }
 
-               if (body->mbo_eadatasize == 0)
-                       GOTO(out, rc = -ENODATA);
+               if (body->mbo_eadatasize == 0) {
+                       /* LU-11109: Newer MDTs set OBD_MD_FLXATTR on
+                        * success so that we can distinguish between
+                        * zero length value and nonexistent xattr.
+                        *
+                        * If OBD_MD_FLXATTR is not set then we keep
+                        * the old behavior and return -ENODATA for
+                        * getxattr() when mbo_eadatasize is 0. But
+                        * -ENODATA only makes sense for getxattr()
+                        * and not for listxattr(). */
+                       if (body->mbo_valid & OBD_MD_FLXATTR)
+                               GOTO(out, rc = 0);
+                       else if (valid == OBD_MD_FLXATTR)
+                               GOTO(out, rc = -ENODATA);
+                       else
+                               GOTO(out, rc = 0);
+               }
 
                /* do not need swab xattr data */
                xdata = req_capsule_server_sized_get(&req->rq_pill, &RMF_EADATA,

diff --git a/lustre/llite/xattr26.c b/lustre/llite/xattr26.c
index c6c3396..d3e18a5 100644 (file)
--- a/lustre/llite/xattr26.c
+++ b/lustre/llite/xattr26.c
@@ -414,8 +414,13 @@ getxattr_nocache:
                LASSERT(body);
 
                /* only detect the xattr size */
-               if (size == 0)
+               if (size == 0) {
+                       /* LU-11109: Older MDTs do not distinguish
+                        * between nonexistent xattrs and zero length
+                        * values in this case. Newer MDTs will return
+                        * -ENODATA or set OBD_MD_FLXATTR. */
                        GOTO(out, rc = body->mbo_eadatasize);
+               }
 
                if (size < body->mbo_eadatasize) {
                        CERROR("server bug: replied size %u > %u\n",
@@ -423,8 +428,23 @@ getxattr_nocache:
                        GOTO(out, rc = -ERANGE);
                }
 
-               if (body->mbo_eadatasize == 0)
-                       GOTO(out, rc = -ENODATA);
+               if (body->mbo_eadatasize == 0) {
+                       /* LU-11109: Newer MDTs set OBD_MD_FLXATTR on
+                        * success so that we can distinguish between
+                        * zero length value and nonexistent xattr.
+                        *
+                        * If OBD_MD_FLXATTR is not set then we keep
+                        * the old behavior and return -ENODATA for
+                        * getxattr() when mbo_eadatasize is 0. But
+                        * -ENODATA only makes sense for getxattr()
+                        * and not for listxattr(). */
+                       if (body->mbo_valid & OBD_MD_FLXATTR)
+                               GOTO(out, rc = 0);
+                       else if (valid == OBD_MD_FLXATTR)
+                               GOTO(out, rc = -ENODATA);
+                       else
+                               GOTO(out, rc = 0);
+               }
 
                /* do not need swab xattr data */
                xdata = req_capsule_server_sized_get(&req->rq_pill, &RMF_EADATA,

diff --git a/lustre/mdt/mdt_xattr.c b/lustre/mdt/mdt_xattr.c
index 142d0db..29271e9 100644 (file)
--- a/lustre/mdt/mdt_xattr.c
+++ b/lustre/mdt/mdt_xattr.c
@@ -294,6 +294,10 @@ int mdt_getxattr(struct mdt_thread_info *info)
 out:
        if (rc >= 0) {
                mdt_counter_incr(req, LPROC_MDT_GETXATTR);
+               /* LU-11109: Set OBD_MD_FLXATTR on success so that
+                * newer clients can distinguish between nonexistent
+                * xattrs and zero length values. */
+               repbody->mbo_valid |= OBD_MD_FLXATTR;
                repbody->mbo_eadatasize = rc;
                rc = 0;
        }

diff --git a/lustre/tests/multiop.c b/lustre/tests/multiop.c
index 9cb65d0..6f4169c 100644 (file)
--- a/lustre/tests/multiop.c
+++ b/lustre/tests/multiop.c
@@ -36,6 +36,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <malloc.h>
 #include <stdio.h>
 #include <string.h>
 #include <sys/types.h>
@@ -67,7 +68,7 @@ char usage[] =
 "Usage: %s filename command-sequence [path...]\n"
 "    command-sequence items:\n"
 "       A  fsetxattr(\"user.multiop\")\n"
-"       a  fgetxattr(\"user.multiop\")\n"
+"       a[num] fgetxattr(\"user.multiop\") [optional buffer size, default 0]\n"
 "       c  close\n"
 "       B[num] call setstripe ioctl to create stripes\n"
 "       C[num] create with optional stripes\n"
@@ -221,6 +222,8 @@ int main(int argc, char **argv)
        struct lu_fid            fid;
        struct timespec          ts;
        struct lov_user_md_v3    lum;
+       char *xattr_buf = NULL;
+       size_t xattr_buf_size = 0;
        long long rc = 0;
        long long last_rc;
 
@@ -264,7 +267,19 @@ int main(int argc, char **argv)
                        }
                        break;
                case 'a':
-                       rc = fgetxattr(fd, XATTR, NULL, 0);
+                       len = atoi(commands + 1);
+                       if (xattr_buf_size < len) {
+                               xattr_buf = realloc(xattr_buf, len);
+                               if (xattr_buf == NULL) {
+                                       save_errno = errno;
+                                       perror("allocating xattr buffer\n");
+                                       exit(save_errno);
+                               }
+
+                               xattr_buf_size = len;
+                       }
+
+                       rc = fgetxattr(fd, XATTR, xattr_buf, len);
                        if (rc < 0) {
                                save_errno = errno;
                                perror("fgetxattr");

diff --git a/lustre/tests/sanity.sh b/lustre/tests/sanity.sh
index dda88ca..6f91721 100755 (executable)
--- a/lustre/tests/sanity.sh
+++ b/lustre/tests/sanity.sh
@@ -8394,6 +8394,30 @@ test_102s() {
 }
 run_test 102s "getting nonexistent xattrs should fail"
 
+test_102t() {
+       [ $(lustre_version_code $SINGLEMDS) -lt $(version_code 2.11.52) ] &&
+               skip "MDS needs to be at least 2.11.52"
+
+       local save="$TMP/$TESTSUITE-$TESTNAME.parameters"
+
+       save_lustre_params client "llite.*.xattr_cache" > $save
+
+       for cache in 0 1; do
+               lctl set_param llite.*.xattr_cache=$cache
+
+               for buf_size in 0 256; do
+                       rm -f $DIR/$tfile
+                       touch $DIR/$tfile || error "touch"
+                       setfattr -n user.multiop $DIR/$tfile
+                       $MULTIOP $DIR/$tfile oa$buf_size ||
+                               error "cannot get zero length xattr value (buf_size = $buf_size)"
+               done
+       done
+
+       restore_lustre_params < $save
+}
+run_test 102t "zero length xattr values handled correctly"
+
 run_acl_subtest()
 {
     $LUSTRE/tests/acl/run $LUSTRE/tests/acl/$1.test