generic/081: wait for lv to be settled before creating fs on it

PATCH Superseded Eryu Guan 2 participants 7 messages Apr 3, 2015 – May 7, 2015

Share & tools

Eryu Guan

author

2015-04-03 09:42 UTC #0 lore raw

PATCH 1/1 generic/081: wait for lv to be settled before creating fs on it

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 <[email protected]>

---
 tests/generic/081 | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

tests/generic/081 +22 -0

index e242c4c..8e1828b 100755
@@ -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

Signature · 1 line

1.8.3.1

Dave Chinner

2015-05-04 06:27 UTC #1 lore raw

Quoted 17 lines · Eryu Guan

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 <[email protected]>
---
 tests/generic/081 | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
tests/generic/081 +0 -0
index e242c4c..8e1828b 100755

tests/generic/081 _cleanup() · line 41

Quoted 6 lines

tests/generic/081 +3 -0

@@ -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?

Quoted 14 lines

tests/generic/081 +7 -0

 	$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

tests/generic/081 snapname=snap_$seq · line 84

Quoted 10 lines

tests/generic/081 +10 -0

+	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....

tests/generic/081 snapname=snap_$seq · line 91

Quoted 7 lines

tests/generic/081 +2 -0

 # 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.

Signature · 2 lines

Dave Chinner
[email protected]

Eryu Guan

author

2015-05-06 07:32 UTC #2 lore raw

Quoted 13 lines · Dave Chinner

Quoted 12 lines · Eryu Guan
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 <[email protected]>
---
 tests/generic/081 | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Quoted 15 lines

tests/generic/081 _cleanup() · line 41
Quoted 11 lines
tests/generic/081 +3 -0
index e242c4c..8e1828b 100755
@@ -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?

Quoted 19 lines

tests/generic/081 +11 -0

 	$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"

Quoted 8 lines

tests/generic/081 snapname=snap_$seq · line 84
Quoted 6 lines
tests/generic/081 +6 -0
+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.

Quoted 10 lines

tests/generic/081 snapname=snap_$seq · line 91

Quoted 8 lines

tests/generic/081 +2 -0

 # 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

Eryu Guan

author

2015-05-06 07:58 UTC #3 lore raw

PATCH v2 1/1 generic/081: wait for lv to be settled before creating fs on it

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 remove fsync from xfs_io, as it may block snapshot from removal for a while and leaves SCRATCH_DEV used by lvm after test.

Signed-off-by Eryu Guan <[email protected]>

---
v2:
- move UDEV_SETTLE_PROG setup code to common/config
- remove fsync from xfs_io command
- update comments a bit about waiting for lv creation

 common/config     | 18 ++++++++++++++++++
 tests/generic/081 |  4 +++-
 2 files changed, 21 insertions(+), 1 deletion(-)

common/config +18 -0

index 3732287..b7991ed 100644
@@ -192,6 +192,24 @@ export DUMP_PROG="`set_prog_path dump`"
 export RESTORE_PROG="`set_prog_path restore`"
 export LVM_PROG="`set_prog_path lvm`"
 
+# 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
+export UDEV_SETTLE_PROG
+
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
 #

tests/generic/081 +3 -1

index 5d38c11..70029d4 100755
@@ -66,6 +66,8 @@ mkdir -p $mnt
 _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 lvcreation to fully complete
+$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

@@ -78,7 +80,7 @@ $LVM_PROG lvcreate -s -L 4M -n $snapname $vgname/$lvname >>$seqres.full 2>&1 ||
 _mount /dev/mapper/$vgname-$snapname $mnt
 
 # write 5M data to the snapshot
-$XFS_IO_PROG -fc "pwrite 0 5m" -c fsync $mnt/testfile >>$seqres.full 2>&1
+$XFS_IO_PROG -fc "pwrite 0 5m" $mnt/testfile >>$seqres.full 2>&1
 
 # _check_dmesg will check for WARNINGs/BUGs in dmesg
 status=0

Signature · 1 line

2.1.0

Dave Chinner

2015-05-06 23:13 UTC #4 lore raw

In reply to Eryu Guan

Quoted 30 lines · Eryu Guan

Quoted 13 lines · Dave Chinner
Quoted 12 lines · Eryu Guan
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 <[email protected]>
---
 tests/generic/081 | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
Quoted 15 lines
tests/generic/081 _cleanup() · line 41
Quoted 11 lines
tests/generic/081 +3 -0
index e242c4c..8e1828b 100755
@@ -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?

Quoted 6 lines

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.

Signature · 2 lines

Dave Chinner
[email protected]

Eryu Guan

author

2015-05-07 03:11 UTC #5 lore raw

Quoted 36 lines · Dave Chinner

Quoted 35 lines · Eryu Guan
Quoted 13 lines · Dave Chinner
Quoted 12 lines · Eryu Guan
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 <[email protected]>
---
 tests/generic/081 | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
Quoted 15 lines
tests/generic/081 _cleanup() · line 41
Quoted 11 lines
tests/generic/081 +3 -0
index e242c4c..8e1828b 100755
@@ -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.

Quoted 5 lines

Quoted 1 line
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

Eryu Guan

author

2015-05-07 03:34 UTC #6 lore raw

PATCH v3 1/1 generic/081: wait for lv to be settled before creating fs on it

In reply to Eryu Guan

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.

Signed-off-by Eryu Guan <[email protected]>

---
v3:
- add fsync command back and no 'sleep 1' in cleanup either

v2:
- move UDEV_SETTLE_PROG setup code to common/config
- remove fsync from xfs_io command
- update comments a bit about waiting for lv creation

 common/config     | 18 ++++++++++++++++++
 tests/generic/081 |  2 ++
 2 files changed, 20 insertions(+)

common/config +18 -0

index 3732287..b7991ed 100644
@@ -192,6 +192,24 @@ export DUMP_PROG="`set_prog_path dump`"
 export RESTORE_PROG="`set_prog_path restore`"
 export LVM_PROG="`set_prog_path lvm`"
 
+# 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
+export UDEV_SETTLE_PROG
+
 # Generate a comparable xfsprogs version number in the form of
 # major * 10000 + minor * 100 + release
 #

tests/generic/081 +2 -0

index 5d38c11..34da1ac 100755
@@ -66,6 +66,8 @@ mkdir -p $mnt
 _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 lvcreation to fully complete
+$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

Signature · 1 line

2.1.0