Btrfs: fix xattr loss after power failure
PATCH 1/1 Btrfs: fix xattr loss after power failure
From: Filipe Manana <[email protected]>
If a file has xattrs, we fsync it, to ensure we clear the flags BTRFS_INODE_NEEDS_FULL_SYNC and BTRFS_INODE_COPY_EVERYTHING from its inode, the current transaction commits and then we fsync it (without either of those bits being set in its inode), we end up not logging all its xattrs. This results in deleting all xattrs when replying the log after a power failure.
Trivial reproducer
$ mkfs.btrfs -f /dev/sdb $ mount /dev/sdb /mnt
$ touch /mnt/foobar $ setfattr -n user.xa -v qwerty /mnt/foobar $ xfs_io -c "fsync" /mnt/foobar
$ sync
$ xfs_io -c "pwrite -S 0xab 0 64K" /mnt/foobar $ xfs_io -c "fsync" /mnt/foobar <power failure>
$ mount /dev/sdb /mnt $ getfattr --absolute-names --dump /mnt/foobar <empty output> $
So fix this by making sure all xattrs are logged if we log a file's inode item and neither the flags BTRFS_INODE_NEEDS_FULL_SYNC nor BTRFS_INODE_COPY_EVERYTHING were set in the inode.
--- fs/btrfs/tree-log.c | 7 +++++++ 1 file changed, 7 insertions(+)
fs/btrfs/tree-log.c +7 -0
@@ -4916,6 +4916,7 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,struct extent_map_tree *em_tree = &inode->extent_tree;u64 logged_isize =0 ;bool need_log_inode_item = true; +bool xattrs_logged = false; path =btrfs_alloc_path ();if (!path)
@@ -5217,6 +5218,7 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans, err =btrfs_log_all_xattrs (trans, root, inode, path, dst_path);if (err) goto out_unlock; + xattrs_logged = true;if (max_key.type >=BTRFS_EXTENT_DATA_KEY && !fast_search) {btrfs_release_path (path);btrfs_release_path (dst_path);
@@ -5229,6 +5231,11 @@ static int btrfs_log_inode(struct btrfs_trans_handle *trans,btrfs_release_path (dst_path);if (need_log_inode_item) { err =log_inode_item (trans, log, dst_path, inode); +if (!err && !xattrs_logged) { + err =btrfs_log_all_xattrs (trans, root, inode, path, + dst_path); +btrfs_release_path (path); + }if (err) goto out_unlock; }
Signature · 1 line
2.11.0
Quoted 29 lines · On Fri, May 11, 2018 at 04:42:42PM +0100, [email protected] wrote:
From: Filipe Manana <[email protected]>
If a file has xattrs, we fsync it, to ensure we clear the flags
BTRFS_INODE_NEEDS_FULL_SYNCandBTRFS_INODE_COPY_EVERYTHINGfrom its inode, the current transaction commits and then we fsync it (without either of those bits being set in its inode), we end up not logging all its xattrs. This results in deleting all xattrs when replying the log after a power failure.Trivial reproducer
$ mkfs.btrfs -f /dev/sdb $ mount /dev/sdb /mnt$ touch /mnt/foobar $ setfattr -n user.xa -v qwerty /mnt/foobar $ xfs_io -c "fsync" /mnt/foobar$ sync$ xfs_io -c "pwrite -S 0xab 0 64K" /mnt/foobar $ xfs_io -c "fsync" /mnt/foobar <power failure>$ mount /dev/sdb /mnt $ getfattr --absolute-names --dump /mnt/foobar <empty output> $
Quoted 7 lines
So fix this by making sure all xattrs are logged if we log a file's inode item and neither the flags
BTRFS_INODE_NEEDS_FULL_SYNCnorBTRFS_INODE_COPY_EVERYTHINGwere set in the inode.Fixes 36283bf777d9 ("Btrfs: fix fsync xattr loss in the fast fsync path")Cc <[email protected]>Signed-off-by Filipe Manana <[email protected]>
Added to next, thanks, and I'll probably add that to 4.17-rc too.