btrfs: fix transaction abort logic in btrfs_fileattr_set()

There's no need to abort the transaction if we failed to set or delete a
property, as we haven't done any change. However we need to abort if we
set a property or delete a property and then fail to update the inode
item, as that would leave the inode's state in subvolume tree
inconsistent.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9d47d16394fc569fa62a4769eeff0aaab16fe252..81e87bc398288d2f6fc031e129497003c4f08d91 100644 (file)
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -289,6 +289,7 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
        int ret;
        const char *comp = NULL;
        u32 inode_flags;
+       bool prop_set = false;
 
        if (btrfs_root_readonly(root))
                return -EROFS;
@@ -401,16 +402,15 @@ int btrfs_fileattr_set(struct mnt_idmap *idmap,
        if (comp) {
                ret = btrfs_set_prop(trans, inode, "btrfs.compression",
                                     comp, strlen(comp), 0);
-               if (unlikely(ret)) {
-                       btrfs_abort_transaction(trans, ret);
+               if (ret)
                        goto out_end_trans;
-               }
+               prop_set = true;
        } else {
                ret = btrfs_set_prop(trans, inode, "btrfs.compression", NULL, 0, 0);
-               if (unlikely(ret && ret != -ENODATA)) {
-                       btrfs_abort_transaction(trans, ret);
+               prop_set = (ret == 0);
+               /* If ret == -ENODATA ignore and proceed to update inode item. */
+               if (ret && ret != -ENODATA)
                        goto out_end_trans;
-               }
        }
 
 update_flags:
@@ -420,6 +420,12 @@ update_flags:
        inode_inc_iversion(&inode->vfs_inode);
        inode_set_ctime_current(&inode->vfs_inode);
        ret = btrfs_update_inode(trans, inode);
+       /*
+        * If we set a property or deleted one, we must abort if we fail to
+        * update the inode, to avoid persisting an inconsistent state.
+        */
+       if (unlikely(ret && prop_set))
+               btrfs_abort_transaction(trans, ret);
 
  out_end_trans:
        btrfs_end_transaction(trans);