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@xxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html