Re: [PATCH 6/9] generic: test I/O error path by fully filling dm snapshot

[Dave Chinner <david@fromorbit.com>]

On Wed, Apr 01, 2015 at 01:57:10PM -0400, Brian Foster wrote:
> On Wed, Apr 01, 2015 at 09:54:15PM +0800, Eryu Guan wrote:
> > xfs used to panic in this test, this xfs commit fix the bug
> > 
> > 8d6c121 xfs: fix buffer use after free on IO error
> > 
> > ext4 and btrfs trigger WARNING on current 4.0-rc3 kernel
> > 
> > Signed-off-by: Eryu Guan <eguan@redhat.com>
> > ---
> > v2:
> > - add _require_dm_snapshot() function to require dm snapshot target
> > - make sure SCRATCH_DEV has enough space for the test
> > - fail the test directly when failures detected in setup phase
> > 
> 
> FYI, the mail subject header hasn't changed so Dave might not notice
> this is a new patch.

Saw it.

> > +_mount /dev/mapper/$vgname-$snapname $mnt
> > +
> > +# write 5M data to the snapshot
> > +$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
> > +
> 
> I noticed there were no errors in $seqres.full when running this test.
> E.g., the pwrite succeeds because nothing is written back to disk at
> that point. The fs does shutdown due to the flush on umount, but it's
> kind of hidden away up in the _cleanup() function.
> 
> Kind of a nit, but we could be a bit more explicit and do a '-c fsync'
> after the pwrite here? That way it's clear that writeback to disk is
> part of the core test and we have a little feedback in $seqres.full that
> I/O errors occurred, as expected.

Added the -c fsync as I pulled it in.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com