| Message ID | 1427896455-12048-1-git-send-email-eguan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > common/config | 1 + > common/rc | 10 ++++++ > tests/generic/081 | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++ > tests/generic/081.out | 2 ++ > tests/generic/group | 1 + > 5 files changed, 100 insertions(+) > create mode 100755 tests/generic/081 > create mode 100644 tests/generic/081.out > > diff --git a/common/config b/common/config > index e5c3579..3732287 100644 > --- a/common/config > +++ b/common/config > @@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`" > export WIPEFS_PROG="`set_prog_path wipefs`" > export DUMP_PROG="`set_prog_path dump`" > export RESTORE_PROG="`set_prog_path restore`" > +export LVM_PROG="`set_prog_path lvm`" > > # Generate a comparable xfsprogs version number in the form of > # major * 10000 + minor * 100 + release > diff --git a/common/rc b/common/rc > index 857308a..74df379 100644 > --- a/common/rc > +++ b/common/rc > @@ -1311,6 +1311,16 @@ _require_dm_flakey() > fi > } > > +_require_dm_snapshot() > +{ > + _require_command "$DMSETUP_PROG" dmsetup > + modprobe dm-snapshot >/dev/null 2>&1 > + $DMSETUP_PROG targets | grep -q snapshot > + if [ $? -ne 0 ]; then > + _notrun "This test requires dm snapshot support" > + fi > +} > + > # this test requires the projid32bit feature to be available in mkfs.xfs. > # > _require_projid32bit() > diff --git a/tests/generic/081 b/tests/generic/081 > new file mode 100755 > index 0000000..3e17d34 > --- /dev/null > +++ b/tests/generic/081 > @@ -0,0 +1,86 @@ > +#! /bin/bash > +# FS QA Test No. 081 > +# > +# Test I/O error path by fully filling an dm snapshot. > +# > +#----------------------------------------------------------------------- > +# Copyright (c) 2015 Red Hat Inc. 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! > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +_cleanup() > +{ > + cd / > + rm -f $tmp.* > + # lvm may have umounted it on I/O error, but in case it does not > + $UMOUNT_PROG $mnt >/dev/null 2>&1 > + $LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1 > + $LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1 > +} > + > +# get standard environment, filters and checks > +. ./common/rc > +. ./common/filter > + > +# real QA test starts here > +_supported_fs generic > +_supported_os Linux > +_require_test > +_require_scratch_nocheck > +_require_dm_snapshot > +_require_block_device $SCRATCH_DEV > +_require_command $LVM_PROG lvm > + > +echo "Silence is golden" > +rm -f $seqres.full > + > +vgname=vg_$seq > +lvname=base_$seq > +snapname=snap_$seq > +mnt=$TEST_DIR/mnt_$seq > +mkdir -p $mnt > + > +# make sure there's enough disk space for 256M lv, test for 300M here in case > +# lvm uses some space for metadata > +_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > + > +# _mkfs_dev exits the test on failure, this can make sure lv is created in > +# above vgcreate/lvcreate steps > +_mkfs_dev /dev/mapper/$vgname-$lvname > + > +# create a 4M snapshot > +$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1 || \ > + _fail "Failed to create snapshot" > + > +_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. Otherwise, the rest looks Ok to me. Brian > +# _check_dmesg will check for WARNINGs/BUGs in dmesg > +status=0 > +exit > diff --git a/tests/generic/081.out b/tests/generic/081.out > new file mode 100644 > index 0000000..663a886 > --- /dev/null > +++ b/tests/generic/081.out > @@ -0,0 +1,2 @@ > +QA output created by 081 > +Silence is golden > diff --git a/tests/generic/group b/tests/generic/group > index cf7408c..f5ebe48 100644 > --- a/tests/generic/group > +++ b/tests/generic/group > @@ -83,6 +83,7 @@ > 078 auto quick metadata > 079 acl attr ioctl metadata auto quick > 080 auto freeze mount > +081 auto quick > 083 rw auto enospc stress > 088 perms auto quick > 089 metadata auto > -- > 2.1.0 > > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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> .... > +# lvm uses some space for metadata > +_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > + > +# _mkfs_dev exits the test on failure, this can make sure lv is created in > +# above vgcreate/lvcreate steps > +_mkfs_dev /dev/mapper/$vgname-$lvname So on my 1p test VM, this fails with +mkfs.xfs: cannot open /dev/mapper/vg_081-base_081: Device or resource busy The problem is that udev has not finished setting up the device before mkfs is run. Hence we need a "udevadm settle" call after the lvcreate call. This results in mkfs succeeding on this machine. Eryu, I'm going to commit the test as it stands as it works on all my other test systems - can you write a followup patch that does the udev settle call in a portable manner? i.e. older systems used to have a 'udev-settle' command, do we still care about that? Cheers, Dave.
On Thu, Apr 02, 2015 at 09:41:01AM +1100, Dave Chinner 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> > .... > > +# lvm uses some space for metadata > > +_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 > > +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 > > +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 > > + > > +# _mkfs_dev exits the test on failure, this can make sure lv is created in > > +# above vgcreate/lvcreate steps > > +_mkfs_dev /dev/mapper/$vgname-$lvname > > So on my 1p test VM, this fails with > > +mkfs.xfs: cannot open /dev/mapper/vg_081-base_081: Device or resource busy > > The problem is that udev has not finished setting up the device > before mkfs is run. Hence we need a "udevadm settle" call after the > lvcreate call. This results in mkfs succeeding on this machine. > > Eryu, I'm going to commit the test as it stands as it works on all > my other test systems - can you write a followup patch that does the > udev settle call in a portable manner? i.e. older systems used to > have a 'udev-settle' command, do we still care about that? Sure, I'll do that, along with the fix of the cleanup error, as Brian suggested. As a distribution tester of RHEL, I don't care much about older systems now like RHEL5, but I'll try to make it portable if possible. Thanks, Eryu -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/common/config b/common/config index e5c3579..3732287 100644 --- a/common/config +++ b/common/config @@ -190,6 +190,7 @@ export DMSETUP_PROG="`set_prog_path dmsetup`" export WIPEFS_PROG="`set_prog_path wipefs`" export DUMP_PROG="`set_prog_path dump`" export RESTORE_PROG="`set_prog_path restore`" +export LVM_PROG="`set_prog_path lvm`" # Generate a comparable xfsprogs version number in the form of # major * 10000 + minor * 100 + release diff --git a/common/rc b/common/rc index 857308a..74df379 100644 --- a/common/rc +++ b/common/rc @@ -1311,6 +1311,16 @@ _require_dm_flakey() fi } +_require_dm_snapshot() +{ + _require_command "$DMSETUP_PROG" dmsetup + modprobe dm-snapshot >/dev/null 2>&1 + $DMSETUP_PROG targets | grep -q snapshot + if [ $? -ne 0 ]; then + _notrun "This test requires dm snapshot support" + fi +} + # this test requires the projid32bit feature to be available in mkfs.xfs. # _require_projid32bit() diff --git a/tests/generic/081 b/tests/generic/081 new file mode 100755 index 0000000..3e17d34 --- /dev/null +++ b/tests/generic/081 @@ -0,0 +1,86 @@ +#! /bin/bash +# FS QA Test No. 081 +# +# Test I/O error path by fully filling an dm snapshot. +# +#----------------------------------------------------------------------- +# Copyright (c) 2015 Red Hat Inc. 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! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_cleanup() +{ + cd / + rm -f $tmp.* + # lvm may have umounted it on I/O error, but in case it does not + $UMOUNT_PROG $mnt >/dev/null 2>&1 + $LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1 + $LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1 +} + +# get standard environment, filters and checks +. ./common/rc +. ./common/filter + +# real QA test starts here +_supported_fs generic +_supported_os Linux +_require_test +_require_scratch_nocheck +_require_dm_snapshot +_require_block_device $SCRATCH_DEV +_require_command $LVM_PROG lvm + +echo "Silence is golden" +rm -f $seqres.full + +vgname=vg_$seq +lvname=base_$seq +snapname=snap_$seq +mnt=$TEST_DIR/mnt_$seq +mkdir -p $mnt + +# make sure there's enough disk space for 256M lv, test for 300M here in case +# lvm uses some space for metadata +_scratch_mkfs_sized $((300 * 1024 * 1024)) >>$seqres.full 2>&1 +$LVM_PROG vgcreate -f $vgname $SCRATCH_DEV >>$seqres.full 2>&1 +$LVM_PROG lvcreate --yes -L 256M -n $lvname $vgname >>$seqres.full 2>&1 + +# _mkfs_dev exits the test on failure, this can make sure lv is created in +# above vgcreate/lvcreate steps +_mkfs_dev /dev/mapper/$vgname-$lvname + +# create a 4M snapshot +$LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1 || \ + _fail "Failed to create snapshot" + +_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 + +# _check_dmesg will check for WARNINGs/BUGs in dmesg +status=0 +exit diff --git a/tests/generic/081.out b/tests/generic/081.out new file mode 100644 index 0000000..663a886 --- /dev/null +++ b/tests/generic/081.out @@ -0,0 +1,2 @@ +QA output created by 081 +Silence is golden diff --git a/tests/generic/group b/tests/generic/group index cf7408c..f5ebe48 100644 --- a/tests/generic/group +++ b/tests/generic/group @@ -83,6 +83,7 @@ 078 auto quick metadata 079 acl attr ioctl metadata auto quick 080 auto freeze mount +081 auto quick 083 rw auto enospc stress 088 perms auto quick 089 metadata auto
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 common/config | 1 + common/rc | 10 ++++++ tests/generic/081 | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++ tests/generic/081.out | 2 ++ tests/generic/group | 1 + 5 files changed, 100 insertions(+) create mode 100755 tests/generic/081 create mode 100644 tests/generic/081.out