From: Jeff Layton Date: Thu, 25 Jul 2019 19:54:58 +0000 (-0400) Subject: client: don't report any vxattrs to listxattr X-Git-Tag: v15.1.0~2033^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bc0f7e18d58d364f03b55c0c980731bda8e7f540;p=ceph.git 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 --- diff --git a/src/client/Client.cc b/src/client/Client.cc index 9801747fa0df..250d9257f8cd 100644 --- 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 f46008c82d8b..8cf4dceddaa4 100644 --- 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 9dd0e1c69ebe..63eb5d91a704 100644 --- 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);