diff mbox

[v2,1/4] xfstests: btrfs/131: add test for an incremental send with name collision

Message ID 1477619046-16375-2-git-send-email-robbieko@synology.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

robbieko Oct. 28, 2016, 1:44 a.m. UTC 
From: Robbie Ko <robbieko@synology.com>

Test that an incremental send operation doesn't work because
there's a name collision in the destination and it's not checked
corretly before the rename operation applies.

This test exercises scenarios used to fail in btrfs and are fixed by
the following patch for the linux kernel:

"Btrfs: incremental send, do not skip generation inconsistency check for inode 256."

Signed-off-by: Robbie Ko <robbieko@synology.com>
---
 tests/btrfs/131     | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/131.out |   2 +
 tests/btrfs/group   |   1 +
 3 files changed, 114 insertions(+)
 create mode 100755 tests/btrfs/131
 create mode 100644 tests/btrfs/131.out

Comments

Eryu Guan Nov. 1, 2016, 7:17 a.m. UTC | #1 
On Fri, Oct 28, 2016 at 09:44:03AM +0800, robbieko wrote:
> From: Robbie Ko <robbieko@synology.com>
> 
> Test that an incremental send operation doesn't work because
> there's a name collision in the destination and it's not checked
> corretly before the rename operation applies.
> 
> This test exercises scenarios used to fail in btrfs and are fixed by
> the following patch for the linux kernel:
> 
> "Btrfs: incremental send, do not skip generation inconsistency check for inode 256."
> 
> Signed-off-by: Robbie Ko <robbieko@synology.com>

Sorry for the late review!

> ---
>  tests/btrfs/131     | 111 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/btrfs/131.out |   2 +
>  tests/btrfs/group   |   1 +
>  3 files changed, 114 insertions(+)
>  create mode 100755 tests/btrfs/131
>  create mode 100644 tests/btrfs/131.out
> 
> diff --git a/tests/btrfs/131 b/tests/btrfs/131
> new file mode 100755
> index 0000000..2e2e0bc
> --- /dev/null
> +++ b/tests/btrfs/131
> @@ -0,0 +1,111 @@
> +#! /bin/bash
> +# FS QA Test No. btrfs/131
> +#
> +# Test that an incremental send operation doesn't work because
> +# there's a name collision in the destination and it's not checked
> +# corretly before the rename operation applies.
> +#
> +#-----------------------------------------------------------------------
> +# Copyright (C) 2016 Synology Inc. All Rights Reserved.
> +# Author: Robbie Ko <robbieko@synology.com>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License as
> +# published by the Free Software Foundation.
> +#
> +# This program is distributed in the hope that it would be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write the Free Software Foundation,
> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> +#-----------------------------------------------------------------------
> +#
> +
> +seq=`basename $0`
> +seqres=$RESULT_DIR/$seq
> +echo "QA output created by $seq"
> +
> +tmp=/tmp/$$
> +status=1	# failure is the default!
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +_cleanup()
> +{
> +	cd /
> +	rm -fr $send_files_dir
> +	rm -f $tmp.*
> +}
> +
> +# get standard environment, filters and checks
> +. ./common/rc
> +. ./common/filter
> +
> +# real QA test starts here
> +_supported_fs btrfs
> +_supported_os Linux
> +_require_test
> +_require_scratch
> +_require_fssum
> +
> +send_files_dir=$TEST_DIR/btrfs-test-$seq
> +
> +rm -f $seqres.full
> +rm -fr $send_files_dir
> +mkdir $send_files_dir
> +
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +mkdir $SCRATCH_MNT/a1
> +mkdir $SCRATCH_MNT/a2
> +
> +# Filesystem looks like:
> +#
> +# .                                                             (ino 256)
> +# |--- a1/                                                      (ino 257)
> +# |
> +# |--- a2/                                                      (ino 258)
> +#
> +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT $SCRATCH_MNT/mysnap1
> +
> +_run_btrfs_util_prog send $SCRATCH_MNT/mysnap1 -f $send_files_dir/1.snap

I found these "run_check" based helpers make the result harder to read
when test failed, I can't see immediate error messages from the diff
output but have to open $seqres.full file to see the details.

Can you please use $BTRFS_UTIL_PROG directly, in all these four tests?
One concern about using bare $BTRFS_UTIL_PROG is that output messages of
btrfs command are always changed. But I think, at least, we can ignore
the stdout and capture the stderr only. e.g.

$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
	$SCRATCH_MNT/mysnap1 >/dev/null

So if everything works fine no output can be seen, but if snapshot
creation is failed, we can see error messages from stderr, which could
fail the test.

> +
> +_scratch_unmount
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +touch $SCRATCH_MNT/a2
> +
> +# Filesystem now looks like:
> +#
> +# .                                                             (ino 256)
> +# |--- a2                                                       (ino 257)
> +#
> +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT $SCRATCH_MNT/mysnap2
> +
> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/1.snap
> +rm $send_files_dir/1.snap
> +
> +run_check $FSSUM_PROG -A -f -w $send_files_dir/1.fssum $SCRATCH_MNT/mysnap1
> +run_check $FSSUM_PROG -A -f -w $send_files_dir/2.fssum $SCRATCH_MNT/mysnap2

And this "run_check" is not necessary either. Just run $FSSUM_PROG
directly and let it print "OK", and match it in .out file.

Thanks,
Eryu

> +
> +_run_btrfs_util_prog send $SCRATCH_MNT/mysnap1 -f $send_files_dir/1.snap
> +_run_btrfs_util_prog send -p $SCRATCH_MNT/mysnap1 $SCRATCH_MNT/mysnap2 \
> +	-f $send_files_dir/2.snap
> +
> +# Now recreate the filesystem by receiving both send streams and verify we get
> +# the same content that the original filesystem had.
> +_scratch_unmount
> +_scratch_mkfs >>$seqres.full 2>&1
> +_scratch_mount
> +
> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/1.snap
> +run_check $FSSUM_PROG -r $send_files_dir/1.fssum $SCRATCH_MNT/mysnap1
> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/2.snap
> +run_check $FSSUM_PROG -r $send_files_dir/2.fssum $SCRATCH_MNT/mysnap2
> +
> +echo "Silence is golden"
> +status=0
> +exit
> diff --git a/tests/btrfs/131.out b/tests/btrfs/131.out
> new file mode 100644
> index 0000000..d118ca9
> --- /dev/null
> +++ b/tests/btrfs/131.out
> @@ -0,0 +1,2 @@
> +QA output created by 131
> +Silence is golden
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index f3a7a4f..a7a070a 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -133,3 +133,4 @@
>  128 auto quick send
>  129 auto quick send
>  130 auto clone send
> +131 auto quick send
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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
robbieko Nov. 2, 2016, 3:40 a.m. UTC | #2 
Hi Eryu Guan,

OK, I will modify it.

Thanks.
Robbie Ko

Eryu Guan 於 2016-11-01 15:17 寫到:
> On Fri, Oct 28, 2016 at 09:44:03AM +0800, robbieko wrote:
>> From: Robbie Ko <robbieko@synology.com>
>> 
>> Test that an incremental send operation doesn't work because
>> there's a name collision in the destination and it's not checked
>> corretly before the rename operation applies.
>> 
>> This test exercises scenarios used to fail in btrfs and are fixed by
>> the following patch for the linux kernel:
>> 
>> "Btrfs: incremental send, do not skip generation inconsistency check 
>> for inode 256."
>> 
>> Signed-off-by: Robbie Ko <robbieko@synology.com>
> 
> Sorry for the late review!
> 
>> ---
>>  tests/btrfs/131     | 111 
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/btrfs/131.out |   2 +
>>  tests/btrfs/group   |   1 +
>>  3 files changed, 114 insertions(+)
>>  create mode 100755 tests/btrfs/131
>>  create mode 100644 tests/btrfs/131.out
>> 
>> diff --git a/tests/btrfs/131 b/tests/btrfs/131
>> new file mode 100755
>> index 0000000..2e2e0bc
>> --- /dev/null
>> +++ b/tests/btrfs/131
>> @@ -0,0 +1,111 @@
>> +#! /bin/bash
>> +# FS QA Test No. btrfs/131
>> +#
>> +# Test that an incremental send operation doesn't work because
>> +# there's a name collision in the destination and it's not checked
>> +# corretly before the rename operation applies.
>> +#
>> +#-----------------------------------------------------------------------
>> +# Copyright (C) 2016 Synology Inc. All Rights Reserved.
>> +# Author: Robbie Ko <robbieko@synology.com>
>> +#
>> +# This program is free software; you can redistribute it and/or
>> +# modify it under the terms of the GNU General Public License as
>> +# published by the Free Software Foundation.
>> +#
>> +# This program is distributed in the hope that it would be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program; if not, write the Free Software 
>> Foundation,
>> +# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
>> +#-----------------------------------------------------------------------
>> +#
>> +
>> +seq=`basename $0`
>> +seqres=$RESULT_DIR/$seq
>> +echo "QA output created by $seq"
>> +
>> +tmp=/tmp/$$
>> +status=1	# failure is the default!
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>> +
>> +_cleanup()
>> +{
>> +	cd /
>> +	rm -fr $send_files_dir
>> +	rm -f $tmp.*
>> +}
>> +
>> +# get standard environment, filters and checks
>> +. ./common/rc
>> +. ./common/filter
>> +
>> +# real QA test starts here
>> +_supported_fs btrfs
>> +_supported_os Linux
>> +_require_test
>> +_require_scratch
>> +_require_fssum
>> +
>> +send_files_dir=$TEST_DIR/btrfs-test-$seq
>> +
>> +rm -f $seqres.full
>> +rm -fr $send_files_dir
>> +mkdir $send_files_dir
>> +
>> +_scratch_mkfs >>$seqres.full 2>&1
>> +_scratch_mount
>> +
>> +mkdir $SCRATCH_MNT/a1
>> +mkdir $SCRATCH_MNT/a2
>> +
>> +# Filesystem looks like:
>> +#
>> +# .                                                             (ino 
>> 256)
>> +# |--- a1/                                                      (ino 
>> 257)
>> +# |
>> +# |--- a2/                                                      (ino 
>> 258)
>> +#
>> +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT 
>> $SCRATCH_MNT/mysnap1
>> +
>> +_run_btrfs_util_prog send $SCRATCH_MNT/mysnap1 -f 
>> $send_files_dir/1.snap
> 
> I found these "run_check" based helpers make the result harder to read
> when test failed, I can't see immediate error messages from the diff
> output but have to open $seqres.full file to see the details.
> 
> Can you please use $BTRFS_UTIL_PROG directly, in all these four tests?
> One concern about using bare $BTRFS_UTIL_PROG is that output messages 
> of
> btrfs command are always changed. But I think, at least, we can ignore
> the stdout and capture the stderr only. e.g.
> 
> $BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
> 	$SCRATCH_MNT/mysnap1 >/dev/null
> 
> So if everything works fine no output can be seen, but if snapshot
> creation is failed, we can see error messages from stderr, which could
> fail the test.
> 
>> +
>> +_scratch_unmount
>> +_scratch_mkfs >>$seqres.full 2>&1
>> +_scratch_mount
>> +touch $SCRATCH_MNT/a2
>> +
>> +# Filesystem now looks like:
>> +#
>> +# .                                                             (ino 
>> 256)
>> +# |--- a2                                                       (ino 
>> 257)
>> +#
>> +_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT 
>> $SCRATCH_MNT/mysnap2
>> +
>> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/1.snap
>> +rm $send_files_dir/1.snap
>> +
>> +run_check $FSSUM_PROG -A -f -w $send_files_dir/1.fssum 
>> $SCRATCH_MNT/mysnap1
>> +run_check $FSSUM_PROG -A -f -w $send_files_dir/2.fssum 
>> $SCRATCH_MNT/mysnap2
> 
> And this "run_check" is not necessary either. Just run $FSSUM_PROG
> directly and let it print "OK", and match it in .out file.
> 
> Thanks,
> Eryu
> 
>> +
>> +_run_btrfs_util_prog send $SCRATCH_MNT/mysnap1 -f 
>> $send_files_dir/1.snap
>> +_run_btrfs_util_prog send -p $SCRATCH_MNT/mysnap1 
>> $SCRATCH_MNT/mysnap2 \
>> +	-f $send_files_dir/2.snap
>> +
>> +# Now recreate the filesystem by receiving both send streams and 
>> verify we get
>> +# the same content that the original filesystem had.
>> +_scratch_unmount
>> +_scratch_mkfs >>$seqres.full 2>&1
>> +_scratch_mount
>> +
>> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/1.snap
>> +run_check $FSSUM_PROG -r $send_files_dir/1.fssum $SCRATCH_MNT/mysnap1
>> +_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/2.snap
>> +run_check $FSSUM_PROG -r $send_files_dir/2.fssum $SCRATCH_MNT/mysnap2
>> +
>> +echo "Silence is golden"
>> +status=0
>> +exit
>> diff --git a/tests/btrfs/131.out b/tests/btrfs/131.out
>> new file mode 100644
>> index 0000000..d118ca9
>> --- /dev/null
>> +++ b/tests/btrfs/131.out
>> @@ -0,0 +1,2 @@
>> +QA output created by 131
>> +Silence is golden
>> diff --git a/tests/btrfs/group b/tests/btrfs/group
>> index f3a7a4f..a7a070a 100644
>> --- a/tests/btrfs/group
>> +++ b/tests/btrfs/group
>> @@ -133,3 +133,4 @@
>>  128 auto quick send
>>  129 auto quick send
>>  130 auto clone send
>> +131 auto quick send
>> --
>> 1.9.1
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe fstests" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



--
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
diff mbox

Patch

diff --git a/tests/btrfs/131 b/tests/btrfs/131
new file mode 100755
index 0000000..2e2e0bc
--- /dev/null
+++ b/tests/btrfs/131
@@ -0,0 +1,111 @@ 
+#! /bin/bash
+# FS QA Test No. btrfs/131
+#
+# Test that an incremental send operation doesn't work because
+# there's a name collision in the destination and it's not checked
+# corretly before the rename operation applies.
+#
+#-----------------------------------------------------------------------
+# Copyright (C) 2016 Synology Inc. All Rights Reserved.
+# Author: Robbie Ko <robbieko@synology.com>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it would be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write the Free Software Foundation,
+# Inc.,  51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+#-----------------------------------------------------------------------
+#
+
+seq=`basename $0`
+seqres=$RESULT_DIR/$seq
+echo "QA output created by $seq"
+
+tmp=/tmp/$$
+status=1	# failure is the default!
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	cd /
+	rm -fr $send_files_dir
+	rm -f $tmp.*
+}
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_test
+_require_scratch
+_require_fssum
+
+send_files_dir=$TEST_DIR/btrfs-test-$seq
+
+rm -f $seqres.full
+rm -fr $send_files_dir
+mkdir $send_files_dir
+
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
+mkdir $SCRATCH_MNT/a1
+mkdir $SCRATCH_MNT/a2
+
+# Filesystem looks like:
+#
+# .                                                             (ino 256)
+# |--- a1/                                                      (ino 257)
+# |
+# |--- a2/                                                      (ino 258)
+#
+_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT $SCRATCH_MNT/mysnap1
+
+_run_btrfs_util_prog send $SCRATCH_MNT/mysnap1 -f $send_files_dir/1.snap
+
+_scratch_unmount
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+touch $SCRATCH_MNT/a2
+
+# Filesystem now looks like:
+#
+# .                                                             (ino 256)
+# |--- a2                                                       (ino 257)
+#
+_run_btrfs_util_prog subvolume snapshot -r $SCRATCH_MNT $SCRATCH_MNT/mysnap2
+
+_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/1.snap
+rm $send_files_dir/1.snap
+
+run_check $FSSUM_PROG -A -f -w $send_files_dir/1.fssum $SCRATCH_MNT/mysnap1
+run_check $FSSUM_PROG -A -f -w $send_files_dir/2.fssum $SCRATCH_MNT/mysnap2
+
+_run_btrfs_util_prog send $SCRATCH_MNT/mysnap1 -f $send_files_dir/1.snap
+_run_btrfs_util_prog send -p $SCRATCH_MNT/mysnap1 $SCRATCH_MNT/mysnap2 \
+	-f $send_files_dir/2.snap
+
+# Now recreate the filesystem by receiving both send streams and verify we get
+# the same content that the original filesystem had.
+_scratch_unmount
+_scratch_mkfs >>$seqres.full 2>&1
+_scratch_mount
+
+_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/1.snap
+run_check $FSSUM_PROG -r $send_files_dir/1.fssum $SCRATCH_MNT/mysnap1
+_run_btrfs_util_prog receive $SCRATCH_MNT -f $send_files_dir/2.snap
+run_check $FSSUM_PROG -r $send_files_dir/2.fssum $SCRATCH_MNT/mysnap2
+
+echo "Silence is golden"
+status=0
+exit
diff --git a/tests/btrfs/131.out b/tests/btrfs/131.out
new file mode 100644
index 0000000..d118ca9
--- /dev/null
+++ b/tests/btrfs/131.out
@@ -0,0 +1,2 @@ 
+QA output created by 131
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index f3a7a4f..a7a070a 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -133,3 +133,4 @@ 
 128 auto quick send
 129 auto quick send
 130 auto clone send
+131 auto quick send