[PATCH] btrfs/104: remove ugly sleep

[PATCH] btrfs/104: remove ugly sleep

[Mark Fasheh]

Latest btrfsprogs can now do 'subvolume sync' to wait on snapshots to be
dropped. Replace the sleep call in btrfs/104 with this. We can also add this
test to the 'quick' group now.

Signed-off-by: Mark Fasheh <mfasheh@suse.de>
---
 tests/btrfs/104   | 8 +++-----
 tests/btrfs/group | 2 +-
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/btrfs/104 b/tests/btrfs/104
index 6afaa02..627e77b 100755
--- a/tests/btrfs/104
+++ b/tests/btrfs/104
@@ -145,11 +145,9 @@ _scratch_cycle_mount
 # 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
+# Removing snapshots on btrfs is a delayed operation, so we must wait for
+# btrfs_drop_snapshot to complete.
+_run_btrfs_util_prog subvolume sync $SCRATCH_MNT
 
 _scratch_unmount
 
diff --git a/tests/btrfs/group b/tests/btrfs/group
index ef6c260..9403daa 100644
--- a/tests/btrfs/group
+++ b/tests/btrfs/group
@@ -104,7 +104,7 @@
 101 auto quick replace
 102 auto quick metadata enospc
 103 auto quick clone compress
-104 auto qgroup
+104 auto quick qgroup
 105 auto quick send
 106 auto quick clone compress
 107 auto quick prealloc
-- 
2.1.4

Re: [PATCH] btrfs/104: remove ugly sleep

[Filipe Manana]

On Fri, Apr 15, 2016 at 12:04 AM, Mark Fasheh <mfasheh@suse.de> wrote:
> Latest btrfsprogs can now do 'subvolume sync' to wait on snapshots to be
> dropped. Replace the sleep call in btrfs/104 with this. We can also add this
> test to the 'quick' group now.
>
> Signed-off-by: Mark Fasheh <mfasheh@suse.de>
> ---
>  tests/btrfs/104   | 8 +++-----
>  tests/btrfs/group | 2 +-
>  2 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/tests/btrfs/104 b/tests/btrfs/104
> index 6afaa02..627e77b 100755
> --- a/tests/btrfs/104
> +++ b/tests/btrfs/104
> @@ -145,11 +145,9 @@ _scratch_cycle_mount
>  # 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
> +# Removing snapshots on btrfs is a delayed operation, so we must wait for
> +# btrfs_drop_snapshot to complete.
> +_run_btrfs_util_prog subvolume sync $SCRATCH_MNT

Looks good, except that this will cause a test failure when running
with older versions of btrfs-progs that don't have this command.

We should skip the test with such old versions (it's usually what we
do everywhere) or make it fallback to the sleep.
We have "_require_btrfs <command>" to check if btrfs-progs supports a
specific command, so we could extend it to check for a subcommand too
as an optional argument so we could call
"_require_btrfs subvolume sync" in the test.

>
>  _scratch_unmount
>
> diff --git a/tests/btrfs/group b/tests/btrfs/group
> index ef6c260..9403daa 100644
> --- a/tests/btrfs/group
> +++ b/tests/btrfs/group
> @@ -104,7 +104,7 @@
>  101 auto quick replace
>  102 auto quick metadata enospc
>  103 auto quick clone compress
> -104 auto qgroup
> +104 auto quick qgroup
>  105 auto quick send
>  106 auto quick clone compress
>  107 auto quick prealloc
> --
> 2.1.4
>
> --
> 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



-- 
Filipe David Manana,

"Reasonable men adapt themselves to the world.
 Unreasonable men adapt the world to themselves.
 That's why all progress depends on unreasonable men."

Re: [PATCH] btrfs/104: remove ugly sleep

[Mark Fasheh]

On Fri, Apr 15, 2016 at 12:04:53PM +0100, Filipe Manana wrote:
> On Fri, Apr 15, 2016 at 12:04 AM, Mark Fasheh <mfasheh@suse.de> wrote:
> > diff --git a/tests/btrfs/104 b/tests/btrfs/104
> > index 6afaa02..627e77b 100755
> > --- a/tests/btrfs/104
> > +++ b/tests/btrfs/104
> > @@ -145,11 +145,9 @@ _scratch_cycle_mount
> >  # 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
> > +# Removing snapshots on btrfs is a delayed operation, so we must wait for
> > +# btrfs_drop_snapshot to complete.
> > +_run_btrfs_util_prog subvolume sync $SCRATCH_MNT
> 
> Looks good, except that this will cause a test failure when running
> with older versions of btrfs-progs that don't have this command.

Right, also there was at least one version which had a broken subvol sync
(see commit d3be5b6).


> We should skip the test with such old versions (it's usually what we
> do everywhere) or make it fallback to the sleep.
> We have "_require_btrfs <command>" to check if btrfs-progs supports a
> specific command, so we could extend it to check for a subcommand too
> as an optional argument so we could call
> "_require_btrfs subvolume sync" in the test.

Hmm, so _require_btrfs() can do this by running --help on the subcommand and
checking the return value. I don't think there's a similar trick we can do
for 'btrfs subvol sync'. Anything other than a subvolume as an argument to
subvol sync returns 1.

I suppose we could try grepping the output for 'too few arguments' though.

Also, thanks for the review!
	--Mark

--
Mark Fasheh