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

[Mark Fasheh <mfasheh@suse.de>]

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