LU-11107 mdt: handle nonexistent xattrs correctly

In mdt_getxattr_pack_reply() propagate -ENODATA returns from
mo_xattr_list() to mdt_getxattr(). Add sanity test_102s() to ensure
that getting a nonexistint xattr will fail.

Lustre-change: https://review.whamcloud.com/32753


Lustre-commit: 39c7ac4c

Signed-off-by: John L. Hammond <jhammond@whamcloud.com>
Change-Id: Ic7a01feb3fcac66d39f84b4ebdfc86025c3e2779
Reviewed-by: Emoly Liu <emoly@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Reviewed-by: James Simmons <uja.ornl@yahoo.com>
Signed-off-by: Minh Diep <mdiep@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/32910


Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>

lustre/mdt/mdt_xattr.c

@@ -51,29 +51,41 @@
 /* return EADATA length to the caller. negative value means error */
 static int mdt_getxattr_pack_reply(struct mdt_thread_info * info)
 {
-        struct req_capsule     *pill = info->mti_pill ;
-        struct ptlrpc_request  *req = mdt_info_req(info);
-        char                   *xattr_name;
-        __u64                   valid;
-        static const char       user_string[] = "user.";
-        int                     size, rc;
-        ENTRY;
+	struct req_capsule *pill = info->mti_pill;
+	struct ptlrpc_request *req = mdt_info_req(info);
+	const char *xattr_name;
+	u64 valid;
+	static const char user_string[] = "user.";
+	int size;
+	int rc = 0;
+	int rc2;
+	ENTRY;
 
 	valid = info->mti_body->mbo_valid & (OBD_MD_FLXATTR | OBD_MD_FLXATTRLS);
 
-        /* Determine how many bytes we need */
+	/* Determine how many bytes we need */
         if (valid == OBD_MD_FLXATTR) {
-                xattr_name = req_capsule_client_get(pill, &RMF_NAME);
-                if (!xattr_name)
-                        RETURN(-EFAULT);
+		xattr_name = req_capsule_client_get(pill, &RMF_NAME);
+		if (!xattr_name)
+			RETURN(-EFAULT);
 
 		if (!(exp_connect_flags(req->rq_export) & OBD_CONNECT_XATTR) &&
 		    !strncmp(xattr_name, user_string, sizeof(user_string) - 1))
 			RETURN(-EOPNOTSUPP);
 
-                size = mo_xattr_get(info->mti_env,
-                                    mdt_object_child(info->mti_object),
-                                    &LU_BUF_NULL, xattr_name);
+		size = mo_xattr_get(info->mti_env,
+				    mdt_object_child(info->mti_object),
+				    &LU_BUF_NULL, xattr_name);
+		if (size == -ENODATA) {
+			/* XXX: Some client code will not handle -ENODATA
+			 * for XATTR_NAME_LOV (trusted.lov) properly. */
+			if (strcmp(xattr_name, XATTR_NAME_LOV) == 0)
+				rc = 0;
+			else
+				rc = -ENODATA;
+
+			size = 0;
+		}
         } else if (valid == OBD_MD_FLXATTRLS) {
                 size = mo_xattr_list(info->mti_env,
                                      mdt_object_child(info->mti_object),
@@ -91,9 +103,7 @@ static int mdt_getxattr_pack_reply(struct mdt_thread_info * info)
 		RETURN(-EINVAL);
 	}
 
-	if (size == -ENODATA) {
-		size = 0;
-	} else if (size < 0) {
+	if (size < 0) {
 		if (size != -EOPNOTSUPP)
 			CERROR("Error geting EA size: %d\n", size);
 		RETURN(size);
@@ -103,18 +113,17 @@ static int mdt_getxattr_pack_reply(struct mdt_thread_info * info)
 		req_capsule_set_size(pill, &RMF_ACL, RCL_SERVER,
 				     LUSTRE_POSIX_ACL_MAX_SIZE_OLD);
 
-        req_capsule_set_size(pill, &RMF_EADATA, RCL_SERVER,
+	req_capsule_set_size(pill, &RMF_EADATA, RCL_SERVER,
 			     info->mti_body->mbo_eadatasize == 0 ? 0 : size);
-        rc = req_capsule_server_pack(pill);
-        if (rc) {
-                LASSERT(rc < 0);
-                RETURN(rc);
-        }
 
-        if (OBD_FAIL_CHECK(OBD_FAIL_MDS_GETXATTR_PACK))
-                RETURN(-ENOMEM);
+	rc2 = req_capsule_server_pack(pill);
+	if (rc2 < 0)
+		RETURN(rc2);
+
+	if (OBD_FAIL_CHECK(OBD_FAIL_MDS_GETXATTR_PACK))
+		RETURN(-ENOMEM);
 
-        RETURN(size);
+	RETURN(rc < 0 ? rc : size);
 }
 
 static int mdt_nodemap_map_acl(struct mdt_thread_info *info, void *buf,
@@ -239,8 +248,10 @@ int mdt_getxattr(struct mdt_thread_info *info)
 
         next = mdt_object_child(info->mti_object);
         easize = mdt_getxattr_pack_reply(info);
-        if (easize < 0)
-                GOTO(out, rc = err_serious(easize));
+	if (easize == -ENODATA)
+		GOTO(out, rc = easize);
+	else if (easize < 0)
+		GOTO(out, rc = err_serious(easize));
 
         repbody = req_capsule_server_get(info->mti_pill, &RMF_MDT_BODY);
         LASSERT(repbody != NULL);

lustre/tests/sanity.sh

@@ -7573,6 +7573,32 @@ test_102r() {
 }
 run_test 102r "set EAs with empty values"
+test_102s() {
+	[ $(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
+
+		rm -f $DIR/$tfile
+		touch $DIR/$tfile || error "touch"
+		for prefix in lustre security system trusted user; do
+			# Note getxattr() may fail with 'Operation not
+			# supported' or 'No such attribute' depending
+			# on prefix and cache.
+			getfattr -n $prefix.n102s $DIR/$tfile &&
+				error "getxattr '$prefix.n102s' should fail (cache = $cache)"
+		done
+	done
+
+	restore_lustre_params < $save
+}
+run_test 102s "getting nonexistent xattrs should fail"
+
 run_acl_subtest()
 {
     $LUSTRE/tests/acl/run $LUSTRE/tests/acl/$1.test