| Message ID | 1391440956-31924-1-git-send-email-wangshilong1991@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 03, 2014 at 11:22:36PM +0800, Wang Shilong wrote: > 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. > Couple of comments below. > +_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 Indentation. > +done > + > +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \ > + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 > + > +$BTRFS_UTIL_PROG send -f $SCRATCH_MNT/send_file \ > + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 & > + > +pid=$! > + > +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/snap_1 \ > + $SCRATCH_MNT/snap_2 >> $seqres.full 2>&1 > + > +wait $pid || echo "Failed to send, see dmesg" This seems kind of racy. It assumes that the send command doesn't complete before the wait $pid call is made. If $pid doesn't exist at this time because it has completed, wait will return 127 and the test will fail.... Also, why would a failure to send result in meaingful information in dmesg? Shouldn't the userspace command output information to tell you why there was a failure into $seqres.full? Cheers, Dave.
Hi Dave, > On Mon, Feb 03, 2014 at 11:22:36PM +0800, Wang Shilong wrote: >> 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. >> > Couple of comments below. > >> +_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 > > Indentation. > >> +done >> + >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \ >> + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 >> + >> +$BTRFS_UTIL_PROG send -f $SCRATCH_MNT/send_file \ >> + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 & >> + >> +pid=$! >> + >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/snap_1 \ >> + $SCRATCH_MNT/snap_2 >> $seqres.full 2>&1 >> + >> +wait $pid || echo "Failed to send, see dmesg" > > This seems kind of racy. It assumes that the send command > doesn't complete before the wait $pid call is made. If $pid doesn't > exist at this time because it has completed, wait will return 127 > and the test will fail…. Sorry for delay reply! Maybe a better idea for this will be: Opposite to previous way, we do snapshots background while at the same time we do sending. And btrfs-progs should output meaningful information on send failure, i will make a tiny patch to address this issue. but to make this test more friendly, i think we can still do something like: btrfs send <..> || echo "Failed to send" What do you think of this way? Feel free to tell me if there are better ideas^_^. Thanks, Wang > Also, why would a failure to send result in meaingful information in > dmesg? Shouldn't the userspace command output information to tell > you why there was a failure into $seqres.full? > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com -- 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
On Thu, Feb 06, 2014 at 09:12:51PM +0800, Wang Shilong wrote: > Hi Dave, > > > On Mon, Feb 03, 2014 at 11:22:36PM +0800, Wang Shilong wrote: > >> 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. > >> > > Couple of comments below. > > > >> +_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 > > > > Indentation. > > > >> +done > >> + > >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT \ > >> + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 > >> + > >> +$BTRFS_UTIL_PROG send -f $SCRATCH_MNT/send_file \ > >> + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 & > >> + > >> +pid=$! > >> + > >> +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/snap_1 \ > >> + $SCRATCH_MNT/snap_2 >> $seqres.full 2>&1 > >> + > >> +wait $pid || echo "Failed to send, see dmesg" > > > > This seems kind of racy. It assumes that the send command > > doesn't complete before the wait $pid call is made. If $pid doesn't > > exist at this time because it has completed, wait will return 127 > > and the test will fail…. > > Sorry for delay reply! > > Maybe a better idea for this will be: > > Opposite to previous way, we do snapshots background while at the > same time we do sending. > > And btrfs-progs should output meaningful information on send failure, i will > make a tiny patch to address this issue. but to make this test more friendly, i > think we can still do something like: > > btrfs send <..> || echo "Failed to send" If send is emitting error messages on failures, then the "|| echo..." is redundant and not necessary to cause a golden image mismatch. If send is not emitting error messages on failure, then it needs fixing because users are going to hate you for silently failing to send the data they asked to be sent. Either way, the echo command on error in the test is not needed. Cheers, Dave.
diff --git a/tests/btrfs/034 b/tests/btrfs/034 new file mode 100644 index 0000000..e27e3cf --- /dev/null +++ b/tests/btrfs/034 @@ -0,0 +1,75 @@ +#!/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.* +} + +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 + +$BTRFS_UTIL_PROG send -f $SCRATCH_MNT/send_file \ + $SCRATCH_MNT/snap_1 >> $seqres.full 2>&1 & + +pid=$! + +$BTRFS_UTIL_PROG subvolume snapshot -r $SCRATCH_MNT/snap_1 \ + $SCRATCH_MNT/snap_2 >> $seqres.full 2>&1 + +wait $pid || echo "Failed to send, see dmesg" + +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