Re: [PATCH 2/2] btrfs: send: fix invalid commands for inodes with changed type but same gen

[Andrei Borzenkov <arvidjaar@gmail.com>]

11.01.2021 22:02, Roman Anasal пишет:
> When doing a send, if a new inode has the same number as an inode in the
> parent subvolume it will only be detected as to be recreated when the
> genrations differ. But with inodes in the same generation the emitted
> commands will cause the receiver to fail.
> 
> This case does not happen when doing incremental sends with snapshots
> that are kept read-only by the user all the time, but it may happen if
> - a snapshot was modified after it was created
> - the subvol used as parent was created independently from the sent subvol
> 
> Example reproducers:
> 
>   # case 1: same ino at same path
>   btrfs subvolume create subvol1
>   btrfs subvolume create subvol2
>   mkdir subvol1/a
>   touch subvol2/a

What happens in case of

echo foo > subvol1/a
echo bar > subvol2/a

?

>   btrfs property set subvol1 ro true
>   btrfs property set subvol2 ro true
>   btrfs send -p subvol1 subvol2 | btrfs receive --dump
> 
> The produced tree state here is:
>   |-- subvol1
>   |   `-- a/        (ino 257)
>   |
>   `-- subvol2
>       `-- a         (ino 257)
> 
>   Where subvol1/a/ is a directory and subvol2/a is a file with the same
>   inode number and same generation.
> 
> Example output of the receive command:
>   At subvol subvol2
>   snapshot        ./subvol2                       uuid=19d2be0a-5af1-fa44-9b3f-f21815178d00 transid=9 parent_uuid=1bac8b12-ddb2-6441-8551-700456991785 parent_transid=9
>   utimes          ./subvol2/                      atime=2021-01-11T13:41:36+0000 mtime=2021-01-11T13:41:36+0000 ctime=2021-01-11T13:41:36+0000
>   link            ./subvol2/a                     dest=a
>   unlink          ./subvol2/a
>   utimes          ./subvol2/                      atime=2021-01-11T13:41:36+0000 mtime=2021-01-11T13:41:36+0000 ctime=2021-01-11T13:41:36+0000
>   chmod           ./subvol2/a                     mode=644
>   utimes          ./subvol2/a                     atime=2021-01-11T13:41:36+0000 mtime=2021-01-11T13:41:36+0000 ctime=2021-01-11T13:41:36+0000
> 
> => the `link` command causes the receiver to fail with:
>    ERROR: link a -> a failed: File exists
> 
> Second example:
>   # case 2: same ino at different path
>   btrfs subvolume create subvol1
>   btrfs subvolume create subvol2
>   mkdir subvol1/a
>   touch subvol2/b
>   btrfs property set subvol1 ro true
>   btrfs property set subvol2 ro true
>   btrfs send -p subvol1 subvol2 | btrfs receive --dump
> 
> The produced tree state here is:
>   |-- subvol1
>   |   `-- a/        (ino 257)
>   |
>   `-- subvol2
>       `-- b         (ino 257)
> 
>   Where subvol1/a/ is a directory and subvol2/b is a file with the same
>   inode number and same generation.
> 
> Example output of the receive command:
>   At subvol subvol2
>   snapshot        ./subvol2                       uuid=ea93c47a-5f47-724f-8a43-e15ce745aef0 transid=20 parent_uuid=f03578ef-5bca-1445-a480-3df63677fddf parent_transid=20
>   utimes          ./subvol2/                      atime=2021-01-11T13:58:00+0000 mtime=2021-01-11T13:58:00+0000 ctime=2021-01-11T13:58:00+0000
>   link            ./subvol2/b                     dest=a
>   unlink          ./subvol2/a
>   utimes          ./subvol2/                      atime=2021-01-11T13:58:00+0000 mtime=2021-01-11T13:58:00+0000 ctime=2021-01-11T13:58:00+0000
>   chmod           ./subvol2/b                     mode=644
>   utimes          ./subvol2/b                     atime=2021-01-11T13:58:00+0000 mtime=2021-01-11T13:58:00+0000 ctime=2021-01-11T13:58:00+0000
> 
> => the `link` command causes the receiver to fail with:
>    ERROR: link b -> a failed: Operation not permitted
> 
> Signed-off-by: Roman Anasal <roman.anasal@bdsu.de>
> ---
>  fs/btrfs/send.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 420371c1d..33ae48442 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -6299,12 +6299,18 @@ static int changed_inode(struct send_ctx *sctx,
>  		right_gen = btrfs_inode_generation(sctx->right_path->nodes[0],
>  				right_ii);
>  
> +		u64 left_type = S_IFMT & btrfs_inode_mode(
> +				sctx->left_path->nodes[0], left_ii);
> +		u64 right_type = S_IFMT & btrfs_inode_mode(
> +				sctx->right_path->nodes[0], right_ii);
> +
> +
>  		/*
>  		 * The cur_ino = root dir case is special here. We can't treat
>  		 * the inode as deleted+reused because it would generate a
>  		 * stream that tries to delete/mkdir the root dir.
>  		 */
> -		if (left_gen != right_gen &&
> +		if ((left_gen != right_gen || left_type != right_type) &&
>  		    sctx->cur_ino != BTRFS_FIRST_FREE_OBJECTID)
>  			sctx->cur_inode_recreated = 1;
>  	}
> @@ -6359,10 +6365,10 @@ static int changed_inode(struct send_ctx *sctx,
>  	} else if (result == BTRFS_COMPARE_TREE_CHANGED) {
>  		/*
>  		 * We need to do some special handling in case the inode was
> -		 * reported as changed with a changed generation number. This
> -		 * means that the original inode was deleted and new inode
> -		 * reused the same inum. So we have to treat the old inode as
> -		 * deleted and the new one as new.
> +		 * reported as changed with a changed generation number or
> +		 * changed inode type. This means that the original inode was
> +		 * deleted and new inode reused the same inum. So we have to
> +		 * treat the old inode as deleted and the new one as new.
>  		 */
>  		if (sctx->cur_inode_recreated) {
>  			/*
>