[PATCH 2/3] btrfs-progs: send: fix handling of multiple snapshots (-p option)

[PATCH 2/3] btrfs-progs: send: fix handling of multiple snapshots (-p option)

[Tsutomu Itoh]

We cannot send multiple snapshots at once by -p option.

[before]
# btrfs send -f /tmp/data0 -p Snap0 Snap[12]
At subvol Snap1
At subvol Snap2
ERROR: parent determination failed for 0
# 

[after]
# btrfs send -f /tmp/data0 -p Snap0 Snap[12]
At subvol Snap1
At subvol Snap2
# 

Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
---
 cmds-send.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/cmds-send.c b/cmds-send.c
index dfdfe01..2a8a697 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -650,7 +650,7 @@ int cmd_send(int argc, char **argv)
 			goto out;
 		}
 
-		if (!full_send && !parent_root_id) {
+		if (!full_send && root_id) {
 			ret = find_good_parent(&send, root_id, &parent_root_id);
 			if (ret < 0) {
 				error("parent determination failed for %lld",
@@ -673,7 +673,7 @@ int cmd_send(int argc, char **argv)
 		if (ret < 0)
 			goto out;
 
-		if (!full_send) {
+		if (!full_send && root_id) {
 			/* done with this subvol, so add it to the clone sources */
 			ret = add_clone_source(&send, root_id);
 			if (ret < 0) {
@@ -681,8 +681,6 @@ int cmd_send(int argc, char **argv)
 				goto out;
 			}
 		}
-
-		parent_root_id = 0;
 	}
 
 	ret = 0;
-- 
2.9.3

Re: [PATCH 2/3] btrfs-progs: send: fix handling of multiple snapshots (-p option)

[David Sterba]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Wed, Oct 19, 2016 at 11:35:03AM +0900, Tsutomu Itoh wrote:
> We cannot send multiple snapshots at once by -p option.

We cannot like that it's broken, or we cannot because it's not supposed
to work that way. I guess it's the former, but the changelog text is a
bit confusing.
> 
> [before]
> # btrfs send -f /tmp/data0 -p Snap0 Snap[12]
> At subvol Snap1
> At subvol Snap2
> ERROR: parent determination failed for 0
> # 
> 
> [after]
> # btrfs send -f /tmp/data0 -p Snap0 Snap[12]
> At subvol Snap1
> At subvol Snap2
> # 

I'm not sure it's fixed, I wrote a simple test, attached, that triggers
the bug even with the patch applied.

[-- Attachment #2: test.sh --]
[-- Type: text/plain, Size: 1101 bytes --]

#!/bin/bash
#
# minimal test for the following syntax: btrfs send -p parent subvol1 subvol2

source $TOP/tests/common

check_prereq mkfs.btrfs
check_prereq btrfs

setup_root_helper
prepare_test_dev 2g

run_check $TOP/mkfs.btrfs -f $IMAGE
run_check_mount_test_dev

here=`pwd`
cd "$TEST_MNT" || _fail "cannot chdir to TEST_MNT"

run_check $SUDO_HELPER btrfs subvolume create subv-parent
run_check $SUDO_HELPER dd if=/dev/urandom of=subv-parent/file bs=1M count=10
run_check $SUDO_HELPER btrfs subvolume snapshot -r subv-parent subv-snap1
run_check $SUDO_HELPER dd if=/dev/urandom of=subv-parent/file bs=1M count=10
run_check $SUDO_HELPER btrfs subvolume snapshot -r subv-parent subv-snap2
run_check $SUDO_HELPER dd if=/dev/urandom of=subv-parent/file bs=1M count=10
run_check $SUDO_HELPER btrfs subvolume snapshot -r subv-parent subv-snap3

run_check truncate -s0 "$here"/send.stream
run_check chmod a+w "$here"/send.stream
run_check $SUDO_HELPER btrfs send -f "$here"/send.stream -p subv-snap1 subv-snap2 subv-snap3

cd "$here" || _fail "cannot chdir back to test directory"

run_check_umount_test_dev

Re: [PATCH 2/3] btrfs-progs: send: fix handling of multiple snapshots (-p option)

[Tsutomu Itoh]

Sorry for the late reply.

On 2016/10/29 0:10, David Sterba wrote:
> On Wed, Oct 19, 2016 at 11:35:03AM +0900, Tsutomu Itoh wrote:
>> We cannot send multiple snapshots at once by -p option.
> 
> We cannot like that it's broken, or we cannot because it's not supposed
> to work that way. I guess it's the former, but the changelog text is a
> bit confusing.

Sorry, I'm not good at English.

>>
>> [before]
>> # btrfs send -f /tmp/data0 -p Snap0 Snap[12]
>> At subvol Snap1
>> At subvol Snap2
>> ERROR: parent determination failed for 0
>> # 
>>
>> [after]
>> # btrfs send -f /tmp/data0 -p Snap0 Snap[12]
>> At subvol Snap1
>> At subvol Snap2
>> # 
> 
> I'm not sure it's fixed, I wrote a simple test, attached, that triggers
> the bug even with the patch applied.
> 

In the attached script,

   run_check $SUDO_HELPER btrfs send -f "$here"/send.stream -p subv-snap1 subv-snap2 subv-snap3

I think that 'btrfs' is a mistake of '$TOP/btrfs'.

Thanks,
Tsutomu

Re: [PATCH 2/3] btrfs-progs: send: fix handling of multiple snapshots (-p option)

[David Sterba]

On Wed, Nov 02, 2016 at 10:11:55AM +0900, Tsutomu Itoh wrote:
> > I'm not sure it's fixed, I wrote a simple test, attached, that triggers
> > the bug even with the patch applied.
> 
> In the attached script,
> 
>    run_check $SUDO_HELPER btrfs send -f "$here"/send.stream -p subv-snap1 subv-snap2 subv-snap3
> 
> I think that 'btrfs' is a mistake of '$TOP/btrfs'.

Oh right, with the fix the test passes.

Re: [PATCH 2/3] btrfs-progs: send: fix handling of multiple snapshots (-p option)

[Tsutomu Itoh]

xfstests btrfs/038 fails when this patch is applied. Sorry for my fault.
I posted the patch that corrected this problem.

  [PATCH] btrfs-progs: send: fix failure of xfstests btrfs/038

Thanks,
Tsutomu

On 2016/10/19 11:35, Tsutomu Itoh wrote:
> We cannot send multiple snapshots at once by -p option.
> 
> [before]
> # btrfs send -f /tmp/data0 -p Snap0 Snap[12]
> At subvol Snap1
> At subvol Snap2
> ERROR: parent determination failed for 0
> # 
> 
> [after]
> # btrfs send -f /tmp/data0 -p Snap0 Snap[12]
> At subvol Snap1
> At subvol Snap2
> # 
> 
> Signed-off-by: Tsutomu Itoh <t-itoh@jp.fujitsu.com>
> ---
>  cmds-send.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/cmds-send.c b/cmds-send.c
> index dfdfe01..2a8a697 100644
> --- a/cmds-send.c
> +++ b/cmds-send.c
> @@ -650,7 +650,7 @@ int cmd_send(int argc, char **argv)
>  			goto out;
>  		}
>  
> -		if (!full_send && !parent_root_id) {
> +		if (!full_send && root_id) {
>  			ret = find_good_parent(&send, root_id, &parent_root_id);
>  			if (ret < 0) {
>  				error("parent determination failed for %lld",
> @@ -673,7 +673,7 @@ int cmd_send(int argc, char **argv)
>  		if (ret < 0)
>  			goto out;
>  
> -		if (!full_send) {
> +		if (!full_send && root_id) {
>  			/* done with this subvol, so add it to the clone sources */
>  			ret = add_clone_source(&send, root_id);
>  			if (ret < 0) {
> @@ -681,8 +681,6 @@ int cmd_send(int argc, char **argv)
>  				goto out;
>  			}
>  		}
> -
> -		parent_root_id = 0;
>  	}
>  
>  	ret = 0;
>