xfs
[Top] [All Lists]

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

To: Wang Shilong <wangshilong1991@xxxxxxxxx>
Subject: Re: [PATCH v3] Btrfs: add regression test for running snapshot and send concurrently
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 7 Feb 2014 15:41:53 +1100
Cc: xfs@xxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx, jbacik@xxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <D32F9438-DA80-41AF-9247-A81E6EC90497@xxxxxxxxx>
References: <1391703008-2322-1-git-send-email-wangshilong1991@xxxxxxxxx> <20140206224337.GB13647@dastard> <D32F9438-DA80-41AF-9247-A81E6EC90497@xxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
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@xxxxxxxxxxxxx

<Prev in Thread] Current Thread [Next in Thread>