From 1e4164a1254d7fb75e49af73994d2e53864536ab Mon Sep 17 00:00:00 2001 From: "John L. Hammond" Date: Mon, 2 Jul 2018 14:52:01 -0500 Subject: [PATCH] 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 Change-Id: I15649581c26dc52e83ca714b44f8372f29954ed5 Reviewed-on: https://review.whamcloud.com/32755 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Andreas Dilger Reviewed-by: Mike Pershin Reviewed-by: James Simmons --- lustre/llite/xattr.c | 26 +++++++++++++++++++++++--- lustre/llite/xattr26.c | 26 +++++++++++++++++++++++--- lustre/mdt/mdt_xattr.c | 4 ++++ lustre/tests/multiop.c | 19 +++++++++++++++++-- lustre/tests/sanity.sh | 24 ++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 8 deletions(-) diff --git a/lustre/llite/xattr.c b/lustre/llite/xattr.c index ac1dd91..7d98825 100644 --- 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 --- 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 --- 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 --- a/lustre/tests/multiop.c +++ b/lustre/tests/multiop.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -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 --- 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 -- 1.8.3.1