ceph: 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.

And at the same time added a new flags CEPH_XATTR_REMOVE2 and in
MDS side it will return -ENODATA when the xattr doesn't exist.
While the CEPH_XATTR_REMOVE will be kept to be compatible with
old cephs.

Please note this commit also fixed a bug, which is that even when
the ACL xattrs don't exist the ctime/mode still will be updated.

URL: https://tracker.ceph.com/issues/64679
Signed-off-by: Xiubo Li <xiubli@redhat.com>

diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
index 85d3dd48b167a3a5028d9f5aac2180cedef91c23..04b2782ad7319e860f00c6402fe1160b82fcf7f7 100644 (file)
--- a/fs/ceph/acl.c
+++ b/fs/ceph/acl.c
@@ -144,6 +144,11 @@ int ceph_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
        }
 
        ret = __ceph_setxattr(inode, name, value, size, 0);
+       /*
+        * If the attribute didn't exist to start with that's fine.
+        */
+       if (!acl && ret == -ENODATA)
+               ret = 0;
        if (ret) {
                if (new_mode != old_mode) {
                        newattrs.ia_ctime = old_ctime;

diff --git a/fs/ceph/xattr.c b/fs/ceph/xattr.c
index 5f87f62091a144d67bda6f2fb4a4ebb59cdf2a68..81308167719505faa4808d9aebc9fb0c01fa9ee1 100644 (file)
--- a/fs/ceph/xattr.c
+++ b/fs/ceph/xattr.c
@@ -611,11 +611,10 @@ static int __set_xattr(struct ceph_inode_info *ci,
                        return err;
                }
                if (update_xattr < 0) {
-                       if (xattr)
-                               __remove_xattr(ci, xattr);
+                       err = __remove_xattr(ci, xattr);
                        kfree(name);
                        kfree(*newxattr);
-                       return 0;
+                       return err;
                }
        }
 
@@ -1129,7 +1128,7 @@ static int ceph_sync_setxattr(struct inode *inode, const char *name,
                if (flags & CEPH_XATTR_REPLACE)
                        op = CEPH_MDS_OP_RMXATTR;
                else
-                       flags |= CEPH_XATTR_REMOVE;
+                       flags |= CEPH_XATTR_REMOVE | CEPH_XATTR_REMOVE2;
        }
 
        doutc(cl, "name %s value size %zu\n", name, size);

diff --git a/include/linux/ceph/ceph_fs.h b/include/linux/ceph/ceph_fs.h
index 69ac3e55a3fe810e453b3fc88c83ce76caac869d..985fc7b79f5d0c8ea40a7c773a7b69432b76b422 100644 (file)
--- a/include/linux/ceph/ceph_fs.h
+++ b/include/linux/ceph/ceph_fs.h
@@ -390,6 +390,7 @@ extern const char *ceph_mds_op_name(int op);
  */
 #define CEPH_XATTR_CREATE  (1 << 0)
 #define CEPH_XATTR_REPLACE (1 << 1)
+#define CEPH_XATTR_REMOVE2 (1 << 30)
 #define CEPH_XATTR_REMOVE  (1 << 31)
 
 /*