btrfs/104: replace ugly sleep with subvol sync

PATCH In review Mark Fasheh 2 participants 3 messages Nov 10, 2015 – Nov 12, 2015

lore

Share & tools

lore

Mark Fasheh

author

2015-11-10 00:30 UTC #0 lore raw

PATCH 1/1 btrfs/104: replace ugly sleep with subvol sync

Btrfsprogs now has 'subvolume sync' which will wait until deleted subvolumes are cleaned from disk.

Signed-off-by Mark Fasheh <[email protected]>

---
 tests/btrfs/104 | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

tests/btrfs/104 +3 -5

index 80161a3..6848b87 100644
@@ -146,11 +146,9 @@ _scratch_remount
 # referenced above.
 _run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap1
 
-# There is no way from userspace to force btrfs_drop_snapshot to run
-# at a given time (even via mount/unmount). We must wait for it to
-# start and complete. This is the shortest time on my tests systems I
-# have found which always allows drop_snapshot to run to completion.
-sleep 45
+# Subvol delete is a delayed operation, wait for it to complete before
+# unmounting.
+_run_btrfs_util_prog subvolume sync $SCRATCH_MNT
 
 _scratch_unmount

Signature · 1 line

2.1.2

Dave Chinner

2015-11-12 00:49 UTC #1 lore raw

Hi Mark,

Quoted 14 lines · Mark Fasheh

Btrfsprogs now has 'subvolume sync' which will wait until deleted subvolumes are cleaned from disk.
Signed-off-by Mark Fasheh <[email protected]>
---
 tests/btrfs/104 | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
tests/btrfs/104 +0 -0
index 80161a3..6848b87 100644
@@ -146,11 +146,9 @@ _scratch_remount
 # referenced above.

tests/btrfs/104 _scratch_remount · line 151

Quoted 10 lines

tests/btrfs/104 +3 -5

 _run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap1
 
-# There is no way from userspace to force btrfs_drop_snapshot to run
-# at a given time (even via mount/unmount). We must wait for it to
-# start and complete. This is the shortest time on my tests systems I
-# have found which always allows drop_snapshot to run to completion.
-sleep 45
+# Subvol delete is a delayed operation, wait for it to complete before
+# unmounting.
+_run_btrfs_util_prog subvolume sync $SCRATCH_MNT

Won't that now fail on systems without the "subvolume sync" command because of the hidden use of _run_check in _run_btrfs_util_prog? i.e. doesn't it need to do something like:

$BTRFS_UTIL_PROG subvolume sync $SCRATCH_MNT
if [ $? -ne 0 ]; then
sleep 45
fi

So that it works on all versions of the btrfs utility?

Cheers,

Dave.

Signature · 2 lines

Dave Chinner
[email protected]

Mark Fasheh

author

2015-11-12 17:08 UTC #2 lore raw

Hi David,

Quoted 8 lines · Dave Chinner

tests/btrfs/104 _scratch_remount · line 151
Quoted 3 lines
tests/btrfs/104 +3 -0
+# Subvol delete is a delayed operation, wait for it to complete before
+# unmounting.
+_run_btrfs_util_prog subvolume sync $SCRATCH_MNT
Won't that now fail on systems without the "subvolume sync" command because of the hidden use of _run_check in _run_btrfs_util_prog? i.e. doesn't it need to do something like:

Quoted 6 lines

$BTRFS_UTIL_PROG subvolume sync $SCRATCH_MNT
if [ $? -ne 0 ]; then
sleep 45
fi
So that it works on all versions of the btrfs utility?

You know, you are probably right and I should have known because we already talked about _run_check hiding errors! I'll fix that up and send out a new patch once I've tested the change.

Thanks,

	--Mark

--
Mark Fasheh