| Message ID | 1428054075-20780-1-git-send-email-eguan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 03, 2015 at 05:41:15PM +0800, Eryu Guan wrote: > Call 'udevadm settle' or 'udevsettle' or 'sleep 1' to make sure new lv > is ready for use before making filesystem on it, depends on which > command is available on the system. > > Also sleep 1 before removing the test vg, as the snapshot may block the > test vg from removal for a while. > > Signed-off-by: Eryu Guan <eguan@redhat.com> > --- > tests/generic/081 | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/tests/generic/081 b/tests/generic/081 > index e242c4c..8e1828b 100755 > --- a/tests/generic/081 > +++ b/tests/generic/081 > @@ -36,6 +36,9 @@ _cleanup() > 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 > + # fsync from xfs_io pins the snapshot in use for a while and blocks > + # vgremove, sleep 1 to avoid such failure > + sleep 1 Doesn't this indicate a bug in the LVM code? unmount flushes block device caches, so there should be no IO remaining on the device pinning it when unmount completes, right? > $LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1 > $LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1 > } > @@ -62,11 +65,30 @@ snapname=snap_$seq > mnt=$TEST_DIR/mnt_$seq > mkdir -p $mnt > > +# use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. > +# newer systems have udevadm command but older systems like RHEL5 don't. > +# But if neither one is available, just set it to "sleep 1" to wait for lv to > +# be settled > +UDEV_SETTLE_PROG="`set_prog_path udevadm`" > +if [ "$UDEV_SETTLE_PROG" == "" ]; then > + # try udevsettle command > + UDEV_SETTLE_PROG="`set_prog_path udevsettle`" > +else > + # udevadm is available, add 'settle' as subcommand > + UDEV_SETTLE_PROG="$UDEV_SETTLE_PROG settle" > +fi > +# neither command is available, use sleep 1 > +if [ "$UDEV_SETTLE_PROG" == "" ]; then > + UDEV_SETTLE_PROG="sleep 1" > +fi > + This should probably go into common/config.... > # 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 > +# wait for lv to be settled > +$UDEV_SETTLE_PROG >>$seqres.full 2>&1 "# wait for lvcreation to fully complete" Cheers, Dave.
On Mon, May 04, 2015 at 04:27:08PM +1000, Dave Chinner wrote: > On Fri, Apr 03, 2015 at 05:41:15PM +0800, Eryu Guan wrote: > > Call 'udevadm settle' or 'udevsettle' or 'sleep 1' to make sure new lv > > is ready for use before making filesystem on it, depends on which > > command is available on the system. > > > > Also sleep 1 before removing the test vg, as the snapshot may block the > > test vg from removal for a while. > > > > Signed-off-by: Eryu Guan <eguan@redhat.com> > > --- > > tests/generic/081 | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/tests/generic/081 b/tests/generic/081 > > index e242c4c..8e1828b 100755 > > --- a/tests/generic/081 > > +++ b/tests/generic/081 > > @@ -36,6 +36,9 @@ _cleanup() > > 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 > > + # fsync from xfs_io pins the snapshot in use for a while and blocks > > + # vgremove, sleep 1 to avoid such failure > > + sleep 1 > > Doesn't this indicate a bug in the LVM code? unmount flushes block > device caches, so there should be no IO remaining on the device > pinning it when unmount completes, right? Maybe, I'm not sure here.. But it does cause problems to fstests, I have two workarounds now, adding sleep 1 before removing vg, delete fsync from xfs_io command. I preper to delete fsync, as bug is still reproducible without it and no extra workaround(sleep 1) is needed. What do you think? > > > $LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1 > > $LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1 > > } > > @@ -62,11 +65,30 @@ snapname=snap_$seq > > mnt=$TEST_DIR/mnt_$seq > > mkdir -p $mnt > > > > +# use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. > > +# newer systems have udevadm command but older systems like RHEL5 don't. > > +# But if neither one is available, just set it to "sleep 1" to wait for lv to > > +# be settled > > +UDEV_SETTLE_PROG="`set_prog_path udevadm`" > > +if [ "$UDEV_SETTLE_PROG" == "" ]; then > > + # try udevsettle command > > + UDEV_SETTLE_PROG="`set_prog_path udevsettle`" > > +else > > + # udevadm is available, add 'settle' as subcommand > > + UDEV_SETTLE_PROG="$UDEV_SETTLE_PROG settle" > > +fi > > +# neither command is available, use sleep 1 > > +if [ "$UDEV_SETTLE_PROG" == "" ]; then > > + UDEV_SETTLE_PROG="sleep 1" > > +fi > > + > > This should probably go into common/config.... Makes sense. > > > # 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 > > +# wait for lv to be settled > > +$UDEV_SETTLE_PROG >>$seqres.full 2>&1 > > "# wait for lvcreation to fully complete" 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
On Wed, May 06, 2015 at 03:32:51PM +0800, Eryu Guan wrote: > On Mon, May 04, 2015 at 04:27:08PM +1000, Dave Chinner wrote: > > On Fri, Apr 03, 2015 at 05:41:15PM +0800, Eryu Guan wrote: > > > Call 'udevadm settle' or 'udevsettle' or 'sleep 1' to make sure new lv > > > is ready for use before making filesystem on it, depends on which > > > command is available on the system. > > > > > > Also sleep 1 before removing the test vg, as the snapshot may block the > > > test vg from removal for a while. > > > > > > Signed-off-by: Eryu Guan <eguan@redhat.com> > > > --- > > > tests/generic/081 | 22 ++++++++++++++++++++++ > > > 1 file changed, 22 insertions(+) > > > > > > diff --git a/tests/generic/081 b/tests/generic/081 > > > index e242c4c..8e1828b 100755 > > > --- a/tests/generic/081 > > > +++ b/tests/generic/081 > > > @@ -36,6 +36,9 @@ _cleanup() > > > 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 > > > + # fsync from xfs_io pins the snapshot in use for a while and blocks > > > + # vgremove, sleep 1 to avoid such failure > > > + sleep 1 > > > > Doesn't this indicate a bug in the LVM code? unmount flushes block > > device caches, so there should be no IO remaining on the device > > pinning it when unmount completes, right? > > Maybe, I'm not sure here.. But it does cause problems to fstests, I have > two workarounds now, adding sleep 1 before removing vg, delete fsync > from xfs_io command. I preper to delete fsync, as bug is still > reproducible without it and no extra workaround(sleep 1) is needed. > > What do you think? I think the bug should be triaged and fixed, not hidden by changing or removing something from the test. I'd suggest trying to find what the fsync is pinning in the filesyste/block device.... Cheers, Dave.
On Thu, May 07, 2015 at 09:13:18AM +1000, Dave Chinner wrote: > On Wed, May 06, 2015 at 03:32:51PM +0800, Eryu Guan wrote: > > On Mon, May 04, 2015 at 04:27:08PM +1000, Dave Chinner wrote: > > > On Fri, Apr 03, 2015 at 05:41:15PM +0800, Eryu Guan wrote: > > > > Call 'udevadm settle' or 'udevsettle' or 'sleep 1' to make sure new lv > > > > is ready for use before making filesystem on it, depends on which > > > > command is available on the system. > > > > > > > > Also sleep 1 before removing the test vg, as the snapshot may block the > > > > test vg from removal for a while. > > > > > > > > Signed-off-by: Eryu Guan <eguan@redhat.com> > > > > --- > > > > tests/generic/081 | 22 ++++++++++++++++++++++ > > > > 1 file changed, 22 insertions(+) > > > > > > > > diff --git a/tests/generic/081 b/tests/generic/081 > > > > index e242c4c..8e1828b 100755 > > > > --- a/tests/generic/081 > > > > +++ b/tests/generic/081 > > > > @@ -36,6 +36,9 @@ _cleanup() > > > > 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 > > > > + # fsync from xfs_io pins the snapshot in use for a while and blocks > > > > + # vgremove, sleep 1 to avoid such failure > > > > + sleep 1 > > > > > > Doesn't this indicate a bug in the LVM code? unmount flushes block > > > device caches, so there should be no IO remaining on the device > > > pinning it when unmount completes, right? > > > > Maybe, I'm not sure here.. But it does cause problems to fstests, I have > > two workarounds now, adding sleep 1 before removing vg, delete fsync > > from xfs_io command. I preper to delete fsync, as bug is still > > reproducible without it and no extra workaround(sleep 1) is needed. > > > > What do you think? > > I think the bug should be triaged and fixed, not hidden by changing > or removing something from the test. I'd suggest trying to find what > the fsync is pinning in the filesyste/block device.... OK, I'll send out a v3 to get the udevadm settle part in first and leave this part as it is for now. (I tried a self-built lvm binary to debug but I couldn't reproduce the issue with it, and I see the issue in only one of my kvm guests. I'll dig deeper.) 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/tests/generic/081 b/tests/generic/081 index e242c4c..8e1828b 100755 --- a/tests/generic/081 +++ b/tests/generic/081 @@ -36,6 +36,9 @@ _cleanup() 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 + # fsync from xfs_io pins the snapshot in use for a while and blocks + # vgremove, sleep 1 to avoid such failure + sleep 1 $LVM_PROG vgremove -f $vgname >>$seqres.full 2>&1 $LVM_PROG pvremove -f $SCRATCH_DEV >>$seqres.full 2>&1 } @@ -62,11 +65,30 @@ snapname=snap_$seq mnt=$TEST_DIR/mnt_$seq mkdir -p $mnt +# use 'udevadm settle' or 'udevsettle' to wait for lv to be settled. +# newer systems have udevadm command but older systems like RHEL5 don't. +# But if neither one is available, just set it to "sleep 1" to wait for lv to +# be settled +UDEV_SETTLE_PROG="`set_prog_path udevadm`" +if [ "$UDEV_SETTLE_PROG" == "" ]; then + # try udevsettle command + UDEV_SETTLE_PROG="`set_prog_path udevsettle`" +else + # udevadm is available, add 'settle' as subcommand + UDEV_SETTLE_PROG="$UDEV_SETTLE_PROG settle" +fi +# neither command is available, use sleep 1 +if [ "$UDEV_SETTLE_PROG" == "" ]; then + UDEV_SETTLE_PROG="sleep 1" +fi + # 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 +# wait for lv to be settled +$UDEV_SETTLE_PROG >>$seqres.full 2>&1 # _mkfs_dev exits the test on failure, this can make sure lv is created in # above vgcreate/lvcreate steps
Call 'udevadm settle' or 'udevsettle' or 'sleep 1' to make sure new lv is ready for use before making filesystem on it, depends on which command is available on the system. Also sleep 1 before removing the test vg, as the snapshot may block the test vg from removal for a while. Signed-off-by: Eryu Guan <eguan@redhat.com> --- tests/generic/081 | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)