mds/client: return -ENODATA when xattr doesn't exist for removexattr

The POSIX says we should return -ENODATA when the corresponding
attribute doesn't exist when removing it. While there is one
exception for the acl ones in the local filesystems, for exmaple
for xfs, which will treat it as success.

While in the MDS side there have two ways to remove the xattr:
sending a CEPH_MDS_OP_SETXATTR request by setting the 'flags' with
CEPH_XATTR_REMOVE and just issued a CEPH_MDS_OP_RMXATTR request
directly.

For the first one it will always return 0 when the corresponding
xattr doesn't exist, while for the later one it will return
-ENODATA instead, this should be fixed in MDS to make them to be
consistent.

Added a CEPH_XATTR_REMOVE2 new flags and will return -ENODATA errno
directly when the crresponding xattr doesn't exist. Just keeps the
old CEPH_XATTR_REMOVE flags to make it to be compatible with old
clients.

Fixes: https://tracker.ceph.com/issues/64679
Signed-off-by: Xiubo Li <xiubli@redhat.com>
(cherry picked from commit 797e92692196122fb9dc26617904379cc777bd2b)

diff --git a/src/client/Client.cc b/src/client/Client.cc
index a84e69f38f2fdcca7691a0c805b6d27a0398a924..5ba71426c1b22c7a9104344d1db2f9a112fea266 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -13215,7 +13215,7 @@ int Client::_do_setxattr(Inode *in, const char *name, const void *value,
 
   int xattr_flags = 0;
   if (!value)
-    xattr_flags |= CEPH_XATTR_REMOVE;
+    xattr_flags |= CEPH_XATTR_REMOVE | CEPH_XATTR_REMOVE2;
   if (flags & XATTR_CREATE)
     xattr_flags |= CEPH_XATTR_CREATE;
   if (flags & XATTR_REPLACE)
@@ -13273,6 +13273,7 @@ int Client::_setxattr(Inode *in, const char *name, const void *value,
       mode_t new_mode = in->mode;
       if (value) {
        int ret = posix_acl_equiv_mode(value, size, &new_mode);
+       ldout(cct, 3) << __func__ << "(" << in->ino << ", \"" << name << "\") = " << ret << dendl;
        if (ret < 0)
          return ret;
        if (ret == 0) {
@@ -13322,6 +13323,11 @@ int Client::_setxattr(Inode *in, const char *name, const void *value,
       ret = -CEPHFS_EOPNOTSUPP;
   }
 
+  if ((!strcmp(name, ACL_EA_ACCESS) ||
+      !strcmp(name, ACL_EA_DEFAULT)) &&
+      ret == -CEPHFS_ENODATA)
+    ret = 0;
+
   return ret;
 }
 
@@ -13410,7 +13416,7 @@ int Client::ll_setxattr(Inode *in, const char *name, const void *value,
 
   vinodeno_t vino = _get_vino(in);
 
-  ldout(cct, 3) << __func__ << " " << vino << " " << name << " size " << size << dendl;
+  ldout(cct, 3) << __func__ << " " << vino << " " << name << " size " << size << " value " << !!value << dendl;
   tout(cct) << __func__ << std::endl;
   tout(cct) << vino.ino.val << std::endl;
   tout(cct) << name << std::endl;

diff --git a/src/include/ceph_fs.h b/src/include/ceph_fs.h
index 2454216802651b133682809aaea604174219f7f6..4c56d7b7f54e4ca663d6d8cf938d9d4b6c788b2e 100644 (file)
--- a/src/include/ceph_fs.h
+++ b/src/include/ceph_fs.h
@@ -479,6 +479,7 @@ int ceph_flags_sys2wire(int flags);
  */
 #define CEPH_XATTR_CREATE  (1 << 0)
 #define CEPH_XATTR_REPLACE (1 << 1)
+#define CEPH_XATTR_REMOVE2 (1 << 30)
 #define CEPH_XATTR_REMOVE  (1 << 31)
 
 /*

diff --git a/src/mds/Server.cc b/src/mds/Server.cc
index 54b8d27f3a74633c970cd0aad620e66fa5f5abdd..859ff70690d1c61c4b063ed377b8a435d50c908f 100644 (file)
--- a/src/mds/Server.cc
+++ b/src/mds/Server.cc
@@ -6474,6 +6474,11 @@ int Server::xattr_validate(CInode *cur, const InodeStoreBase::xattr_map_const_pt
       return -CEPHFS_ENODATA;
     }
 
+    if ((flags & CEPH_XATTR_REMOVE2) && !(xattrs && xattrs->count(mempool::mds_co::string(xattr_name)))) {
+      dout(10) << "setxattr '" << xattr_name << "' XATTR_REMOVE2 and CEPHFS_ENODATA on " << *cur << dendl;
+      return -CEPHFS_ENODATA;
+    }
+
     return 0;
   }
 
@@ -6683,7 +6688,7 @@ void Server::handle_client_setxattr(MDRequestRef& mdr)
   pi.inode->change_attr++;
   pi.inode->xattr_version++;
 
-  if ((flags & CEPH_XATTR_REMOVE)) {
+  if ((flags & (CEPH_XATTR_REMOVE | CEPH_XATTR_REMOVE2))) {
     std::invoke(handler->removexattr, this, cur, pi.xattrs, xattr_op);
   } else {
     std::invoke(handler->setxattr, this, cur, pi.xattrs, xattr_op);