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.f...@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. -- Dave Chinner da...@fromorbit.com -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
