client: don't report any vxattrs to listxattr

The convention with kernel filesystems is to not report vxattrs when
listxattr is called. Doing this can throw a wrench to archiving tools
that will attempt to restore files with xattrs intact, only to find
that some of them can't be stored.

Remove the code that prints out vxattrs in listxattr. With this we
also don't need the "hidden" flag in the vxattr definitions.

Also fix up the existing testcases to account for the change in
behavior.

Fixes: https://tracker.ceph.com/issues/40965
Signed-off-by: Jeff Layton <jlayton@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 9801747fa0df601346e25fd00da38eae09b3ba8d..250d9257f8cd9664ab59dd975cc0a179b7c38702 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -11351,29 +11351,6 @@ int Client::_listxattr(Inode *in, char *name, size_t size,
     name += this_len;
     size -= this_len;
   }
-
-  const VXattr *vxattr;
-  for (vxattr = _get_vxattrs(in); vxattr && !vxattr->name.empty(); vxattr++) {
-    if (vxattr->hidden)
-      continue;
-    // call pointer-to-member function
-    if (vxattr->exists_cb && !(this->*(vxattr->exists_cb))(in))
-      continue;
-
-    size_t this_len = vxattr->name.length() + 1;
-    r += this_len;
-    if (len_only)
-      continue;
-
-    if (this_len > size) {
-      r = -ERANGE;
-      goto out;
-    }
-
-    memcpy(name, vxattr->name.c_str(), this_len);
-    name += this_len;
-    size -= this_len;
-  }
 out:
   ldout(cct, 8) << __func__ << "(" << in->ino << ", " << size << ") = " << r << dendl;
   return r;
@@ -11806,7 +11783,6 @@ size_t Client::_vxattrcb_snap_btime(Inode *in, char *val, size_t size)
   name: CEPH_XATTR_NAME(_type, _name),                         \
   getxattr_cb: &Client::_vxattrcb_ ## _type ## _ ## _name,     \
   readonly: true,                                              \
-  hidden: false,                                               \
   exists_cb: NULL,                                             \
   flags: 0,                                                     \
 }
@@ -11815,7 +11791,6 @@ size_t Client::_vxattrcb_snap_btime(Inode *in, char *val, size_t size)
   name: CEPH_XATTR_NAME(_type, _name),                         \
   getxattr_cb: &Client::_vxattrcb_ ## _type ## _ ## _name,     \
   readonly: true,                                              \
-  hidden: false,                                               \
   exists_cb: NULL,                                             \
   flags: _flags,                                               \
 }
@@ -11824,7 +11799,6 @@ size_t Client::_vxattrcb_snap_btime(Inode *in, char *val, size_t size)
   name: CEPH_XATTR_NAME2(_type, _name, _field),                        \
   getxattr_cb: &Client::_vxattrcb_ ## _name ## _ ## _field,    \
   readonly: false,                                             \
-  hidden: true,                                                        \
   exists_cb: &Client::_vxattrcb_layout_exists,                 \
   flags: 0,                                                     \
 }
@@ -11833,7 +11807,6 @@ size_t Client::_vxattrcb_snap_btime(Inode *in, char *val, size_t size)
   name: CEPH_XATTR_NAME(_type, _name),                         \
   getxattr_cb: &Client::_vxattrcb_ ## _type ## _ ## _name,     \
   readonly: false,                                             \
-  hidden: true,                                                        \
   exists_cb: &Client::_vxattrcb_quota_exists,                  \
   flags: 0,                                                     \
 }
@@ -11843,7 +11816,6 @@ const Client::VXattr Client::_dir_vxattrs[] = {
     name: "ceph.dir.layout",
     getxattr_cb: &Client::_vxattrcb_layout,
     readonly: false,
-    hidden: true,
     exists_cb: &Client::_vxattrcb_layout_exists,
     flags: 0,
   },
@@ -11864,7 +11836,6 @@ const Client::VXattr Client::_dir_vxattrs[] = {
     name: "ceph.quota",
     getxattr_cb: &Client::_vxattrcb_quota,
     readonly: false,
-    hidden: true,
     exists_cb: &Client::_vxattrcb_quota_exists,
     flags: 0,
   },
@@ -11874,7 +11845,6 @@ const Client::VXattr Client::_dir_vxattrs[] = {
     name: "ceph.dir.pin",
     getxattr_cb: &Client::_vxattrcb_dir_pin,
     readonly: false,
-    hidden: true,
     exists_cb: &Client::_vxattrcb_dir_pin_exists,
     flags: 0,
   },
@@ -11882,7 +11852,6 @@ const Client::VXattr Client::_dir_vxattrs[] = {
     name: "ceph.snap.btime",
     getxattr_cb: &Client::_vxattrcb_snap_btime,
     readonly: true,
-    hidden: false,
     exists_cb: &Client::_vxattrcb_snap_btime_exists,
     flags: 0,
   },
@@ -11894,7 +11863,6 @@ const Client::VXattr Client::_file_vxattrs[] = {
     name: "ceph.file.layout",
     getxattr_cb: &Client::_vxattrcb_layout,
     readonly: false,
-    hidden: true,
     exists_cb: &Client::_vxattrcb_layout_exists,
     flags: 0,
   },
@@ -11907,7 +11875,6 @@ const Client::VXattr Client::_file_vxattrs[] = {
     name: "ceph.snap.btime",
     getxattr_cb: &Client::_vxattrcb_snap_btime,
     readonly: true,
-    hidden: false,
     exists_cb: &Client::_vxattrcb_snap_btime_exists,
     flags: 0,
   },

diff --git a/src/client/Client.h b/src/client/Client.h
index f46008c82d8be09672a67ee21f5f89d5a1ca48e3..8cf4dceddaa4c0f6a584639f04e4b31997588d1e 100644 (file)
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -995,7 +995,7 @@ private:
   struct VXattr {
          const string name;
          size_t (Client::*getxattr_cb)(Inode *in, char *val, size_t size);
-         bool readonly, hidden;
+         bool readonly;
          bool (Client::*exists_cb)(Inode *in);
          unsigned int flags;
   };

diff --git a/src/test/libcephfs/test.cc b/src/test/libcephfs/test.cc
index 9dd0e1c69ebe25f3db5b84f4797606290766ac35..63eb5d91a704194a4d67a95f069edb9e4579523e 100644 (file)
--- a/src/test/libcephfs/test.cc
+++ b/src/test/libcephfs/test.cc
@@ -534,13 +534,8 @@ TEST(LibCephFS, Xattrs) {
   char *n;
   i = 'a';
   while (len > 0) {
-    // skip/ignore the dir layout
-    if (strcmp(p, "ceph.dir.layout") == 0 ||
-       strcmp(p, "ceph.file.layout") == 0) {
-      len -= strlen(p) + 1;
-      p += strlen(p) + 1;
-      continue;
-    }
+    // ceph.* xattrs should not be listed
+    ASSERT_NE(strncmp(p, "ceph.", 5), 0);
 
     sprintf(xattrk, "user.test_xattr_%c", i);
     ASSERT_STREQ(p, xattrk);
@@ -2257,10 +2252,9 @@ TEST(LibCephFS, SnapXattrs) {
   utime_t new_btime = utime_t(strtoull(gxattrv2, NULL, 10), strtoull(s + 1, NULL, 10));
   ASSERT_LT(btime, new_btime);
 
-  // check that the snap.btime vxattr appears in listxattr()
+  // listxattr() shouldn't return snap.btime vxattr
   char xattrlist[512];
-  int len = ceph_listxattr(cmount, c_temp, xattrlist, sizeof(xattrlist));
-  ASSERT_GT(len, 0);
+  int len = ceph_listxattr(cmount, test_snap_xattr_file, xattrlist, sizeof(xattrlist));
   ASSERT_GE(sizeof(xattrlist), (size_t)len);
   char *p = xattrlist;
   int found = 0;
@@ -2270,19 +2264,6 @@ TEST(LibCephFS, SnapXattrs) {
     len -= strlen(p) + 1;
     p += strlen(p) + 1;
   }
-  ASSERT_EQ(found, 1);
-
-  // listxattr() shouldn't return snap.btime vxattr for non-snaps
-  len = ceph_listxattr(cmount, test_snap_xattr_file, xattrlist, sizeof(xattrlist));
-  ASSERT_GE(sizeof(xattrlist), (size_t)len);
-  p = xattrlist;
-  found = 0;
-  while (len > 0) {
-    if (strcmp(p, "ceph.snap.btime") == 0)
-      found++;
-    len -= strlen(p) + 1;
-    p += strlen(p) + 1;
-  }
   ASSERT_EQ(found, 0);
 
   ceph_shutdown(cmount);