[PATCH] btrfs: fix resending received snapshot with parent

[PATCH] btrfs: fix resending received snapshot with parent

[Robin Ruede]

This fixes a regression introduced by 37b8d27d between v4.1 and v4.2.

When a snapshot is received, its received_uuid is set to the original
uuid of the subvolume. When that snapshot is then resent to a third
filesystem, it's received_uuid is set to the second uuid
instead of the original one. The same was true for the parent_uuid.
This behaviour was partially changed in 37b8d27d, but in that patch
only the parent_uuid was taken from the real original,
not the uuid itself, causing the search for the parent to fail in
the case below.

This happens for example when trying to send a series of linked
snapshots (e.g. created by snapper) from the backup file system back to the original one.

The following commands reproduce the issue in v4.2.1
(no error in 4.1.6)

    # setup three test file systems
    for i in 1 2 3; do
	    truncate -s 50M fs$i
	    mkfs.btrfs fs$i
	    mkdir $i
	    mount fs$i $i
    done
    echo "content" > 1/testfile
    btrfs su snapshot -r 1/ 1/snap1
    echo "changed content" > 1/testfile
    btrfs su snapshot -r 1/ 1/snap2

    # works fine:
    btrfs send 1/snap1 | btrfs receive 2/
    btrfs send -p 1/snap1 1/snap2 | btrfs receive 2/

    # ERROR: could not find parent subvolume
    btrfs send 2/snap1 | btrfs receive 3/
    btrfs send -p 2/snap1 2/snap2 | btrfs receive 3/

Signed-off-by: Robin Ruede <rruede+git@gmail.com>
---
 fs/btrfs/send.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index aa72bfd..890933b 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -2351,8 +2351,14 @@ static int send_subvol_begin(struct send_ctx *sctx)
 	}
 
 	TLV_PUT_STRING(sctx, BTRFS_SEND_A_PATH, name, namelen);
-	TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
-			sctx->send_root->root_item.uuid);
+
+	if (!btrfs_is_empty_uuid(sctx->send_root->root_item.received_uuid))
+		TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
+			    sctx->send_root->root_item.received_uuid);
+	else
+		TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
+			    sctx->send_root->root_item.uuid);
+
 	TLV_PUT_U64(sctx, BTRFS_SEND_A_CTRANSID,
 		    le64_to_cpu(sctx->send_root->root_item.ctransid));
 	if (parent_root) {
-- 
2.6.0

Re: [PATCH] btrfs: fix resending received snapshot with parent

[Filipe Manana]

On Wed, Sep 30, 2015 at 8:23 PM, Robin Ruede <r.ruede@gmail.com> wrote:
> This fixes a regression introduced by 37b8d27d between v4.1 and v4.2.
>
> When a snapshot is received, its received_uuid is set to the original
> uuid of the subvolume. When that snapshot is then resent to a third
> filesystem, it's received_uuid is set to the second uuid
> instead of the original one. The same was true for the parent_uuid.
> This behaviour was partially changed in 37b8d27d, but in that patch
> only the parent_uuid was taken from the real original,
> not the uuid itself, causing the search for the parent to fail in
> the case below.
>
> This happens for example when trying to send a series of linked
> snapshots (e.g. created by snapper) from the backup file system back to the original one.
>
> The following commands reproduce the issue in v4.2.1
> (no error in 4.1.6)
>
>     # setup three test file systems
>     for i in 1 2 3; do
>             truncate -s 50M fs$i
>             mkfs.btrfs fs$i
>             mkdir $i
>             mount fs$i $i
>     done
>     echo "content" > 1/testfile
>     btrfs su snapshot -r 1/ 1/snap1
>     echo "changed content" > 1/testfile
>     btrfs su snapshot -r 1/ 1/snap2
>
>     # works fine:
>     btrfs send 1/snap1 | btrfs receive 2/
>     btrfs send -p 1/snap1 1/snap2 | btrfs receive 2/
>
>     # ERROR: could not find parent subvolume
>     btrfs send 2/snap1 | btrfs receive 3/
>     btrfs send -p 2/snap1 2/snap2 | btrfs receive 3/
>
> Signed-off-by: Robin Ruede <rruede+git@gmail.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks for fixing this.
I've added this to my integration branch [1] and will send soon a pull
request to Chris for 4.4, including this fix plus a few others for
send/receive, after some more testing.

I've also made an xfstest for it [1, 2]

[1] http://git.kernel.org/cgit/linux/kernel/git/fdmanana/linux.git/log/?h=integration-4.3
[2] https://patchwork.kernel.org/patch/7363681/
[3] https://patchwork.kernel.org/patch/7363661/

> ---
>  fs/btrfs/send.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index aa72bfd..890933b 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2351,8 +2351,14 @@ static int send_subvol_begin(struct send_ctx *sctx)
>         }
>
>         TLV_PUT_STRING(sctx, BTRFS_SEND_A_PATH, name, namelen);
> -       TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
> -                       sctx->send_root->root_item.uuid);
> +
> +       if (!btrfs_is_empty_uuid(sctx->send_root->root_item.received_uuid))
> +               TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
> +                           sctx->send_root->root_item.received_uuid);
> +       else
> +               TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
> +                           sctx->send_root->root_item.uuid);
> +
>         TLV_PUT_U64(sctx, BTRFS_SEND_A_CTRANSID,
>                     le64_to_cpu(sctx->send_root->root_item.ctransid));
>         if (parent_root) {
> --
> 2.6.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH] btrfs: fix resending received snapshot with parent

[Ed Tomlinson]

On Friday, October 9, 2015 4:24:10 PM EDT, Filipe Manana wrote:
> On Wed, Sep 30, 2015 at 8:23 PM, Robin Ruede <r.ruede@gmail.com> wrote:
>> This fixes a regression introduced by 37b8d27d between v4.1 and v4.2.
>> 
>> When a snapshot is received, its received_uuid is set to the original
>> uuid of the subvolume. When that snapshot is then resent to a third
>> filesystem, it's received_uuid is set to the second uuid
>> instead of the original one. The same was true for the parent_uuid. ...
> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
> Thanks for fixing this.
> I've added this to my integration branch [1] and will send soon a pull
> request to Chris for 4.4, including this fix plus a few others for
> send/receive, after some more testing.
>
> I've also made an xfstest for it [1, 2]

Another thanks for this fix.  It fixes things here.  I am runing 4.2.3 with 
the 4.3 btrfs tree pulled on top of it along with this fix.  Incremental 
sends
are now working again.  

Tested-by: Ed Tomlinson <edt@aei.ca>

This fixes a regression, can we please get into 4.3?

Thanks
Ed Tomlinson

Re: [PATCH] btrfs: fix resending received snapshot with parent

[Filipe Manana]

On Tue, Oct 13, 2015 at 1:31 AM, Ed Tomlinson <edt@aei.ca> wrote:
> On Friday, October 9, 2015 4:24:10 PM EDT, Filipe Manana wrote:
>>
>> On Wed, Sep 30, 2015 at 8:23 PM, Robin Ruede <r.ruede@gmail.com> wrote:
>>>
>>> This fixes a regression introduced by 37b8d27d between v4.1 and v4.2.
>>>
>>> When a snapshot is received, its received_uuid is set to the original
>>> uuid of the subvolume. When that snapshot is then resent to a third
>>> filesystem, it's received_uuid is set to the second uuid
>>> instead of the original one. The same was true for the parent_uuid. ...
>>
>> Reviewed-by: Filipe Manana <fdmanana@suse.com>
>>
>> Thanks for fixing this.
>> I've added this to my integration branch [1] and will send soon a pull
>> request to Chris for 4.4, including this fix plus a few others for
>> send/receive, after some more testing.
>>
>> I've also made an xfstest for it [1, 2]
>
>
> Another thanks for this fix.  It fixes things here.  I am runing 4.2.3 with
> the 4.3 btrfs tree pulled on top of it along with this fix.  Incremental
> sends
> are now working again.
> Tested-by: Ed Tomlinson <edt@aei.ca>
>
> This fixes a regression, can we please get into 4.3?

I've tagged it for stable backports in my 4.4 integration branch [1].
Thanks.

[1] http://git.kernel.org/cgit/linux/kernel/git/fdmanana/linux.git/log/?h=integration-4.4

thanks
>
> Thanks
> Ed Tomlinson
>



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH] btrfs: fix resending received snapshot with parent

[Dāvis Mosāns]

trešd., 2015. g. 30. sept., plkst. 22:25 — lietotājs Robin Ruede
(<r.ruede@gmail.com>) rakstīja:
>
> This fixes a regression introduced by 37b8d27d between v4.1 and v4.2.
>
> When a snapshot is received, its received_uuid is set to the original
> uuid of the subvolume. When that snapshot is then resent to a third
> filesystem, it's received_uuid is set to the second uuid
> instead of the original one. The same was true for the parent_uuid.
> This behaviour was partially changed in 37b8d27d, but in that patch
> only the parent_uuid was taken from the real original,
> not the uuid itself, causing the search for the parent to fail in
> the case below.
>
> This happens for example when trying to send a series of linked
> snapshots (e.g. created by snapper) from the backup file system back to the original one.
>
> The following commands reproduce the issue in v4.2.1
> (no error in 4.1.6)
>
>     # setup three test file systems
>     for i in 1 2 3; do
>             truncate -s 50M fs$i
>             mkfs.btrfs fs$i
>             mkdir $i
>             mount fs$i $i
>     done
>     echo "content" > 1/testfile
>     btrfs su snapshot -r 1/ 1/snap1
>     echo "changed content" > 1/testfile
>     btrfs su snapshot -r 1/ 1/snap2
>
>     # works fine:
>     btrfs send 1/snap1 | btrfs receive 2/
>     btrfs send -p 1/snap1 1/snap2 | btrfs receive 2/
>
>     # ERROR: could not find parent subvolume
>     btrfs send 2/snap1 | btrfs receive 3/
>     btrfs send -p 2/snap1 2/snap2 | btrfs receive 3/
>
> Signed-off-by: Robin Ruede <rruede+git@gmail.com>
> ---
>  fs/btrfs/send.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index aa72bfd..890933b 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -2351,8 +2351,14 @@ static int send_subvol_begin(struct send_ctx *sctx)
>         }
>
>         TLV_PUT_STRING(sctx, BTRFS_SEND_A_PATH, name, namelen);
> -       TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
> -                       sctx->send_root->root_item.uuid);
> +
> +       if (!btrfs_is_empty_uuid(sctx->send_root->root_item.received_uuid))
> +               TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
> +                           sctx->send_root->root_item.received_uuid);
> +       else
> +               TLV_PUT_UUID(sctx, BTRFS_SEND_A_UUID,
> +                           sctx->send_root->root_item.uuid);
> +
>         TLV_PUT_U64(sctx, BTRFS_SEND_A_CTRANSID,
>                     le64_to_cpu(sctx->send_root->root_item.ctransid));
>         if (parent_root) {
> --
> 2.6.0
>
> --

Hi,

This breaks use case when snapshot is sent from one system to another
where it's set to read-write, modified, set back to read-only to send
to 3rd system.
This will cause received_uuid to be present but it won't match
original first snapshot anymore so 2nd system's UUID should be used
instead of first.
Only very recently (Oct 2021) "btrfs property set" was changed to
clear received_uuid when switching ro -> rw thus it means there could
be a lot of rw filesystems with it set. And it can cause strange send
failure, see my email titled "btrfs send picks wrong subvolume UUID".
Also I'm not only one who encountered such issue [1].
Workaround is to clear received_uuid, but that might not be obvious or
not even possible for filesystems mounted as read-only (or corrupted
ones that can't be remounted rw).

[1] "Wrong received UUIDs after backup restore and onward" -
https://www.spinics.net/lists/linux-btrfs/msg65142.html

Best regards,
Dāvis