lkml.org 
[lkml]   [2022]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] btrfs: send: fix send failure of a subcase of orphan inodes
On Sun, Oct 16, 2022 at 4:34 PM bingjingc <bingjingc@synology.com> wrote:
>
> From: BingJing Chang <bingjingc@synology.com>
>
> Commit 9ed0a72e5b35 ("btrfs: send: fix failures when processing inodes with
> no links") tries to fix all incremental send cases of orphan inodes the
> send operation will meet. However, there's still a bug causing the corner
> subcase fails with a ENOENT error.
>
> Here's shortened steps of that subcase:
>
> $ btrfs subvolume create vol
> $ touch vol/foo
>
> $ btrfs subvolume snapshot -r vol snap1
> $ btrfs subvolume snapshot -r vol snap2
>
> # Turn the second snapshot to RW mode and delete the file while
> # holding an open file descriptor on it
> $ btrfs property set snap2 ro false
> $ exec 73<snap2/foo
> $ rm snap2/foo
>
> # Set the second snapshot back to RO mode and do an incremental send
> # with an unusal reverse order
> $ btrfs property set snap2 ro true
> $ btrfs send -p snap2 snap1 > /dev/null
> At subvol snap1
> ERROR: send ioctl failed with -2: No such file or directory
>
> It's subcase 3 of BTRFS_COMPARE_TREE_CHANGED in the commit 9ed0a72e5b35
> ("btrfs: send: fix failures when processing inodes with no links"). And
> it's not a common case. We still have not met it in the real world.
> Theoretically, this case can happen in a batch cascading snapshot backup.
> In cascading backups, the receive operation in the middle may cause orphan
> inodes to appear because of the open file descriptors on the snapshot files
> during receiving. And if we don't do the batch snapshot backups in their
> creation order, then we can have an inode, which is an orphan in the parent
> snapshot but refers to a file in the send snapshot. Since an orphan inode
> has no paths, the send operation will fail with a ENOENT error if it
> tries to generate a path for it.
>
> In that patch, this subcase will be treated as an inode with a new
> generation. However, when the routine tries to delete the old paths in
> the parent snapshot, the function process_all_refs() doesn't check whether
> there are paths recorded or not before it calls the function
> process_recorded_refs(). And the function process_recorded_refs() try
> to get the first path in the parent snapshot in the beginning. Since it has
> no paths in the parent snapshot, the send operation fails.
>
> To fix this, we can easily put a link count check to avoid entering the
> deletion routine like what we do a link count check to avoid creating a
> new one. Moreover, we can assume that the function process_all_refs()
> can always collect references to process because we know it has a
> positive link count.
>
> Signed-off-by: BingJing Chang <bingjingc@synology.com>

Looks good.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

Please add a test case for fstests later if you can.
Thanks!

> ---
> fs/btrfs/send.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 4ef4167072b8..1568fa977ca1 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -6665,17 +6665,19 @@ static int changed_inode(struct send_ctx *sctx,
> /*
> * First, process the inode as if it was deleted.
> */
> - sctx->cur_inode_gen = right_gen;
> - sctx->cur_inode_new = false;
> - sctx->cur_inode_deleted = true;
> - sctx->cur_inode_size = btrfs_inode_size(
> - sctx->right_path->nodes[0], right_ii);
> - sctx->cur_inode_mode = btrfs_inode_mode(
> - sctx->right_path->nodes[0], right_ii);
> - ret = process_all_refs(sctx,
> - BTRFS_COMPARE_TREE_DELETED);
> - if (ret < 0)
> - goto out;
> + if (old_nlinks > 0) {
> + sctx->cur_inode_gen = right_gen;
> + sctx->cur_inode_new = false;
> + sctx->cur_inode_deleted = true;
> + sctx->cur_inode_size = btrfs_inode_size(
> + sctx->right_path->nodes[0], right_ii);
> + sctx->cur_inode_mode = btrfs_inode_mode(
> + sctx->right_path->nodes[0], right_ii);
> + ret = process_all_refs(sctx,
> + BTRFS_COMPARE_TREE_DELETED);
> + if (ret < 0)
> + goto out;
> + }
>
> /*
> * Now process the inode as if it was new.
> --
> 2.37.1
>

\
 
 \ /
  Last update: 2022-10-17 13:49    [W:2.589 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site