[PATCH v3] Btrfs: add regression test for running snapshot and send concurrently

[PATCH v3] Btrfs: add regression test for running snapshot and send concurrently

[Wang Shilong]

From: Wang Shilong <wangsl.fnst@cn.fujitsu.com>

Btrfs would fail to send if snapshot run concurrently, this test is to make
sure we have fixed the bug.

Signed-off-by: Wang Shilong <wangsl.fnst@cn.fujitsu.com>
---
Changelog
 v1->v2:
        Avoid race codes and a code style update(Thanks to Dave Chinner's comments)
 v2->v3:
        make sure we kill background snapshots on test failure
---
 tests/btrfs/034     | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/btrfs/034.out |  2 ++
 tests/btrfs/group   |  1 +
 3 files changed, 86 insertions(+)
 create mode 100644 tests/btrfs/034
 create mode 100644 tests/btrfs/034.out

diff --git a/tests/btrfs/034 b/tests/btrfs/034
new file mode 100644
index 0000000..5152969
--- /dev/null
+++ b/tests/btrfs/034
@@ -0,0 +1,83 @@
+#!/bin/bash
+# FS QA Test No. btrfs/034
+#
+# Regression test for running snapshots and send concurrently.
+#
+#-----------------------------------------------------------------------
+# Copyright (c) 2014 Fujitsu.  All Rights Reserved.
+#
+# 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"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+    rm -f $tmp.*
+}
+
+do_snapshots()
+{
+	i=2
+	while [ $i -lt 50 ]
+	do
+		$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/snap_1 \
+			$SCRATCH_MNT/snap_$i >> $seqres.full 2>&1
+		let i=$i+1
+	done
+}
+
+trap "_cleanup ; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common/rc
+. ./common/filter
+
+# real QA test starts here
+_supported_fs btrfs
+_supported_os Linux
+_require_scratch
+
+_scratch_mkfs > /dev/null 2>&1
+_scratch_mount
+
+
+touch $SCRATCH_MNT/foo
+
+# get file with fragments by using backwards writes.
+for i in `seq 10240 -1 1`; do
+	$XFS_IO_PROG -f -d -c "pwrite $(($i * 4096)) 4096" \
+		$SCRATCH_MNT/foo > /dev/null | _filter_xfs_io
+done
+
+$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
+	$SCRATCH_MNT/snap_1 >> $seqres.full 2>&1
+
+do_snapshots &
+snapshots_pid=$!
+
+$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 > /dev/null 2>&1 || echo "btrfs send failed"
+
+kill -TERM $snapshots_pid 2> /dev/null
+
+echo "Silence is golden"
+status=0 ; exit
diff --git a/tests/btrfs/034.out b/tests/btrfs/034.out
new file mode 100644
index 0000000..4c8873c
--- /dev/null
+++ b/tests/btrfs/034.out
@@ -0,0 +1,2 @@
+QA output created by 034
+Silence is golden
diff --git a/tests/btrfs/group b/tests/btrfs/group
index b29236c..f9f062f 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -36,3 +36,4 @@
 031 auto quick
 032 auto quick
 033 auto quick
+034 auto quick
-- 
1.8.4

Re: [PATCH v3] Btrfs: add regression test for running snapshot and send concurrently

[Dave Chinner]

On Fri, Feb 07, 2014 at 12:10:08AM +0800, Wang Shilong wrote:
> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
> +	$SCRATCH_MNT/snap_1 >> $seqres.full 2>&1
> +
> +do_snapshots &
> +snapshots_pid=$!
> +
> +$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 > /dev/null 2>&1 || echo "btrfs send failed"

Let's stop this anti-pattern before it takes hold.

If there's output from the send command it should be
filtered and captured in the golden image. Hence any deviation
caused by errors is automatically flagged as an error.

That's the whole point of using golden images for capturing errors -
you don't need to capture return values from binaries and it
guarantees that users are informed about failures through error
messages. IOWs:

$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 | _btrfs_send_filter

is what you should be doing here.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3] Btrfs: add regression test for running snapshot and send concurrently

[Wang Shilong]

Hi Dave,

> On Fri, Feb 07, 2014 at 12:10:08AM +0800, Wang Shilong wrote:
>> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
>> +	$SCRATCH_MNT/snap_1 >> $seqres.full 2>&1
>> +
>> +do_snapshots &
>> +snapshots_pid=$!
>> +
>> +$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 > /dev/null 2>&1 || echo "btrfs send failed"
> 
> Let's stop this anti-pattern before it takes hold.
> 
> If there's output from the send command it should be
> filtered and captured in the golden image. Hence any deviation
> caused by errors is automatically flagged as an error.
> 
> That's the whole point of using golden images for capturing errors -
> you don't need to capture return values from binaries and it
> guarantees that users are informed about failures through error
> messages. IOWs:
> 
> $BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 | _btrfs_send_filter
> 
> is what you should be doing here.

I knew what you mean here, in fact, i did this on purpose.
for this test failure, btrfs-prog did not output failure information from the beginning.
So to make older progs can also detect the test failure, i dropped into this way.

Anyway, if you don't like this and think  old btrfs-progs needn't consider this,  i will update
the patch.^_^

Thanks,
Wang

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

Re: [PATCH v3] Btrfs: add regression test for running snapshot and send concurrently

[Dave Chinner]

On Fri, Feb 07, 2014 at 12:18:31PM +0800, Wang Shilong wrote:
> 
> Hi Dave,
> 
> > On Fri, Feb 07, 2014 at 12:10:08AM +0800, Wang Shilong wrote:
> >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
> >> +	$SCRATCH_MNT/snap_1 >> $seqres.full 2>&1
> >> +
> >> +do_snapshots &
> >> +snapshots_pid=$!
> >> +
> >> +$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 > /dev/null 2>&1 || echo "btrfs send failed"
> > 
> > Let's stop this anti-pattern before it takes hold.
> > 
> > If there's output from the send command it should be
> > filtered and captured in the golden image. Hence any deviation
> > caused by errors is automatically flagged as an error.
> > 
> > That's the whole point of using golden images for capturing errors -
> > you don't need to capture return values from binaries and it
> > guarantees that users are informed about failures through error
> > messages. IOWs:
> > 
> > $BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 | _btrfs_send_filter
> > 
> > is what you should be doing here.
> 
> I knew what you mean here, in fact, i did this on purpose.

Ok, then you need to explain why you did it on purpose with a comment.
It's just as important to explain the reason for doing something in
test code as it is in the kernel code. i.e. so when we are looking
at the test in 5 years time we know the reason for it being that
way.

> for this test failure, btrfs-prog did not output failure
> information from the beginning. 

I have nothing good to say about that state of affairs, but...

> So to make older progs can also
> detect the test failure, i dropped into this way.

.. it's going to have to stay like it. Please insert an
appropriately sarcastic comment about the usefulness of a silent
send command here, because if I write it I'm going to offend lots of
people. :/

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH v3] Btrfs: add regression test for running snapshot and send concurrently

[Wang Shilong]

Hi Dave,

> On Fri, Feb 07, 2014 at 12:18:31PM +0800, Wang Shilong wrote:
>> 
>> Hi Dave,
>> 
>>> On Fri, Feb 07, 2014 at 12:10:08AM +0800, Wang Shilong wrote:
>>>> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \
>>>> +	$SCRATCH_MNT/snap_1 >> $seqres.full 2>&1
>>>> +
>>>> +do_snapshots &
>>>> +snapshots_pid=$!
>>>> +
>>>> +$BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 > /dev/null 2>&1 || echo "btrfs send failed"
>>> 
>>> Let's stop this anti-pattern before it takes hold.
>>> 
>>> If there's output from the send command it should be
>>> filtered and captured in the golden image. Hence any deviation
>>> caused by errors is automatically flagged as an error.
>>> 
>>> That's the whole point of using golden images for capturing errors -
>>> you don't need to capture return values from binaries and it
>>> guarantees that users are informed about failures through error
>>> messages. IOWs:
>>> 
>>> $BTRFS_UTIL_PROG send $SCRATCH_MNT/snap_1 | _btrfs_send_filter
>>> 
>>> is what you should be doing here.
>> 
>> I knew what you mean here, in fact, i did this on purpose.
> 
> Ok, then you need to explain why you did it on purpose with a comment.
> It's just as important to explain the reason for doing something in
> test code as it is in the kernel code. i.e. so when we are looking
> at the test in 5 years time we know the reason for it being that
> way.
> 
>> for this test failure, btrfs-prog did not output failure
>> information from the beginning. 
> 
> I have nothing good to say about that state of affairs, but...
> 
>> So to make older progs can also
>> detect the test failure, i dropped into this way.
> 
> .. it's going to have to stay like it. Please insert an
> appropriately sarcastic comment about the usefulness of a silent
> send command here, because if I write it I'm going to offend lots of
> people. :/

Sorry, my miss, when i was going to give a patch for btrfs-progs, i noticed
the issue has been fixed in latest btrfs-progs(3.12 which has released for a long time).

So let's drop the way you have said before.

Thanks,
Wang
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com