Re: [PATCH v2] fstests: generic/018: expand "write backwards sync but contiguous" to test regression in btrfs

[Dave Chinner <david@fromorbit.com>]

On Fri, Aug 14, 2015 at 10:19:16AM -0500, Eric Sandeen wrote:
> On 8/13/15 3:47 AM, Liu Bo wrote:
> > Btrfs has a problem when defraging a file which has a large fragment'ed range,
> > it'd leave the tail extent as a seperate extent instead of merging it with
> > previous extents.
> > 
> > This makes generic/018 recognize the above regression.
> 
> Sorry for the late review, but here it is ;)
....
> 3) You stop redirecting xfs_io to /dev/null, and save it to the golden output
> file instead.
> 
> Honestly, I find hundreds of extra xfs_io output lines to be rather unhelpful,
> because the old output file used to be quite easy to read, to see what's going on.

Yup, it should remain redirected to /dev/null. If writing a few
small IOs to an otherwise empty filesystem fails, then you've got
bigger problems. besides, we actually test that the writes worked by
the count of extents before defrag. If any of the writes fail, we'll
fail because the "before" extent count will be wrong.

Besides, if the pwrite() fails, then all the paths in xfs_io that
call do_pwrite() end up doing perror("pwrite64") and so it should
output errors to stderr just fine.

> Today it only redirects stdout:
> 
> $XFS_IO_PROG -f -c "pwrite -b $((4 * bsize)) 0 $((4 * bsize))" $fragfile \
> 					> /dev/null
> 
> so if a write fails, I *think* stderr will get output, and the test *should*
> fail as a result.[1]  You could add a || _fail "xfs_io failed" for good measure...

_fail calls do not belong here - catch the error message, and if
there isn't one on stderr then xfs_io need fixing...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com