diff mbox series

fstests: btrfs: use proper filter for subvolume deletion

Message ID 727fc0e695846a43830bdfc2d6757d6edc2d6878.1703213691.git.naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series fstests: btrfs: use proper filter for subvolume deletion | expand

Commit Message

Naohiro Aota Dec. 22, 2023, 2:56 a.m. UTC 
Test cases btrfs/208, 233, 276 does not use _filter_btrfs_subvol_delete()
to process "btrfs subvolume delete" command's output. So, the following

Comments

David Disseldorp Dec. 22, 2023, 5:07 a.m. UTC | #1 
On Fri, 22 Dec 2023 11:56:22 +0900, Naohiro Aota wrote:

> Test cases btrfs/208, 233, 276 does not use _filter_btrfs_subvol_delete()
> to process "btrfs subvolume delete" command's output. So, the following
> diff occurs even with a previous fix.
> 
> btrfs/208       - output mismatch (see /host/btrfs/208.out.bad)
>     --- tests/btrfs/208.out     2023-12-22 02:09:18.000000000 +0000
>     +++ /host/btrfs/208.out.bad 2023-12-22 02:21:40.697036486 +0000
>     @@ -6,12 +6,12 @@
>      subvol1
>      subvol2
>      subvol3
>     -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
>     +Delete subvolume 256 (no-commit): 'SCRATCH_MNT/subvol1'
>      After deleting one subvolume:
>      subvol2
>     ...
> 
> Let them use the filter and fix the output accordingly.
> 
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>

Looks fine.
Reviewed-by: David Disseldorp <ddiss@suse.de>
One minor nit...

> ---
>  tests/btrfs/208     | 2 +-
>  tests/btrfs/208.out | 6 +++---
>  tests/btrfs/233     | 3 ++-
>  tests/btrfs/233.out | 4 ++--
>  tests/btrfs/276     | 3 ++-
>  tests/btrfs/276.out | 2 +-
>  6 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/tests/btrfs/208 b/tests/btrfs/208
> index 909f9fa40803..d58803e2f801 100755
> --- a/tests/btrfs/208
> +++ b/tests/btrfs/208
> @@ -28,7 +28,7 @@ _delete_and_list()
>  	local msg="$2"
>  
>  	SUBVOLID=$(_btrfs_get_subvolid $SCRATCH_MNT "$subvol_name")
> -	$BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_scratch
> +	$BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_btrfs_subvol_delete
>  
>  	echo "$msg"
>  	$BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | $AWK_PROG '{ print $NF }'
> diff --git a/tests/btrfs/208.out b/tests/btrfs/208.out
> index 9b660699a4b2..dc5761ba1c87 100644
> --- a/tests/btrfs/208.out
> +++ b/tests/btrfs/208.out
> @@ -6,12 +6,12 @@ Current subvolume ids:
>  subvol1
>  subvol2
>  subvol3
> -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
> +Delete subvolume 'SCRATCH_MNT/subvol1'
>  After deleting one subvolume:
>  subvol2
>  subvol3
> -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol3'
> +Delete subvolume 'SCRATCH_MNT/subvol3'
>  Last remaining subvolume:
>  subvol2
> -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol2'
> +Delete subvolume 'SCRATCH_MNT/subvol2'
>  All subvolumes removed.
> diff --git a/tests/btrfs/233 b/tests/btrfs/233
> index 2b94a9c6befe..f2c1eba090be 100755
> --- a/tests/btrfs/233
> +++ b/tests/btrfs/233
> @@ -21,6 +21,7 @@ _cleanup()
>  
>  # Import common functions.
>  . ./common/filter
> +. ./common/filter.btrfs

common/filter.btrfs sources common/filter, so you can replace these.
Naohiro Aota Dec. 22, 2023, 6:02 a.m. UTC | #2 
On Fri, Dec 22, 2023 at 04:07:26PM +1100, David Disseldorp wrote:
> On Fri, 22 Dec 2023 11:56:22 +0900, Naohiro Aota wrote:
> 
> > Test cases btrfs/208, 233, 276 does not use _filter_btrfs_subvol_delete()
> > to process "btrfs subvolume delete" command's output. So, the following
> > diff occurs even with a previous fix.
> > 
> > btrfs/208       - output mismatch (see /host/btrfs/208.out.bad)
> >     --- tests/btrfs/208.out     2023-12-22 02:09:18.000000000 +0000
> >     +++ /host/btrfs/208.out.bad 2023-12-22 02:21:40.697036486 +0000
> >     @@ -6,12 +6,12 @@
> >      subvol1
> >      subvol2
> >      subvol3
> >     -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
> >     +Delete subvolume 256 (no-commit): 'SCRATCH_MNT/subvol1'
> >      After deleting one subvolume:
> >      subvol2
> >     ...
> > 
> > Let them use the filter and fix the output accordingly.
> > 
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> 
> Looks fine.
> Reviewed-by: David Disseldorp <ddiss@suse.de>
> One minor nit...
> 
> > ---
> >  tests/btrfs/208     | 2 +-
> >  tests/btrfs/208.out | 6 +++---
> >  tests/btrfs/233     | 3 ++-
> >  tests/btrfs/233.out | 4 ++--
> >  tests/btrfs/276     | 3 ++-
> >  tests/btrfs/276.out | 2 +-
> >  6 files changed, 11 insertions(+), 9 deletions(-)
> > 
> > diff --git a/tests/btrfs/208 b/tests/btrfs/208
> > index 909f9fa40803..d58803e2f801 100755
> > --- a/tests/btrfs/208
> > +++ b/tests/btrfs/208
> > @@ -28,7 +28,7 @@ _delete_and_list()
> >  	local msg="$2"
> >  
> >  	SUBVOLID=$(_btrfs_get_subvolid $SCRATCH_MNT "$subvol_name")
> > -	$BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_scratch
> > +	$BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_btrfs_subvol_delete
> >  
> >  	echo "$msg"
> >  	$BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | $AWK_PROG '{ print $NF }'
> > diff --git a/tests/btrfs/208.out b/tests/btrfs/208.out
> > index 9b660699a4b2..dc5761ba1c87 100644
> > --- a/tests/btrfs/208.out
> > +++ b/tests/btrfs/208.out
> > @@ -6,12 +6,12 @@ Current subvolume ids:
> >  subvol1
> >  subvol2
> >  subvol3
> > -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
> > +Delete subvolume 'SCRATCH_MNT/subvol1'
> >  After deleting one subvolume:
> >  subvol2
> >  subvol3
> > -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol3'
> > +Delete subvolume 'SCRATCH_MNT/subvol3'
> >  Last remaining subvolume:
> >  subvol2
> > -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol2'
> > +Delete subvolume 'SCRATCH_MNT/subvol2'
> >  All subvolumes removed.
> > diff --git a/tests/btrfs/233 b/tests/btrfs/233
> > index 2b94a9c6befe..f2c1eba090be 100755
> > --- a/tests/btrfs/233
> > +++ b/tests/btrfs/233
> > @@ -21,6 +21,7 @@ _cleanup()
> >  
> >  # Import common functions.
> >  . ./common/filter
> > +. ./common/filter.btrfs
> 
> common/filter.btrfs sources common/filter, so you can replace these.

Oh, I didn't notice that. But, there are many test cases importing
both. Maybe, it's good to express direct dependency (e.g, for
_filter_scratch) explicitly? Or, they should be fixed as well?

$ grep -E 'filter$' $(grep -lr filter.btrfs tests|sort)      
tests/btrfs/001:. ./common/filter
tests/btrfs/035:. ./common/filter
tests/btrfs/048:. ./common/filter
tests/btrfs/059:. ./common/filter
tests/btrfs/089:. ./common/filter
tests/btrfs/096:. ./common/filter
tests/btrfs/100:. ./common/filter
tests/btrfs/101:. ./common/filter
tests/btrfs/112:. ./common/filter
tests/btrfs/113:. ./common/filter
tests/btrfs/162:. ./common/filter
tests/btrfs/163:. ./common/filter
tests/btrfs/171:. ./common/filter
tests/btrfs/197:. ./common/filter
tests/btrfs/198:. ./common/filter
tests/btrfs/208:. ./common/filter
tests/btrfs/218:. ./common/filter
tests/btrfs/233:. ./common/filter
tests/btrfs/254:. ./common/filter
tests/btrfs/276:. ./common/filter
David Disseldorp Dec. 22, 2023, 7:30 a.m. UTC | #3 
On Fri, 22 Dec 2023 06:02:41 +0000, Naohiro Aota wrote:

> > >  # Import common functions.
> > >  . ./common/filter
> > > +. ./common/filter.btrfs  
> > 
> > common/filter.btrfs sources common/filter, so you can replace these.  
> 
> Oh, I didn't notice that. But, there are many test cases importing
> both. Maybe, it's good to express direct dependency (e.g, for
> _filter_scratch) explicitly? Or, they should be fixed as well?

I don't have a strong preference either way. I don't see any ordering
dependencies, so separate imports would also work.

Cheers, David
diff mbox series

Patch

diff occurs even with a previous fix.

btrfs/208       - output mismatch (see /host/btrfs/208.out.bad)
    --- tests/btrfs/208.out     2023-12-22 02:09:18.000000000 +0000
    +++ /host/btrfs/208.out.bad 2023-12-22 02:21:40.697036486 +0000
    @@ -6,12 +6,12 @@
     subvol1
     subvol2
     subvol3
    -Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
    +Delete subvolume 256 (no-commit): 'SCRATCH_MNT/subvol1'
     After deleting one subvolume:
     subvol2
    ...

Let them use the filter and fix the output accordingly.

Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 tests/btrfs/208     | 2 +-
 tests/btrfs/208.out | 6 +++---
 tests/btrfs/233     | 3 ++-
 tests/btrfs/233.out | 4 ++--
 tests/btrfs/276     | 3 ++-
 tests/btrfs/276.out | 2 +-
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/tests/btrfs/208 b/tests/btrfs/208
index 909f9fa40803..d58803e2f801 100755
--- a/tests/btrfs/208
+++ b/tests/btrfs/208
@@ -28,7 +28,7 @@  _delete_and_list()
 	local msg="$2"
 
 	SUBVOLID=$(_btrfs_get_subvolid $SCRATCH_MNT "$subvol_name")
-	$BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_scratch
+	$BTRFS_UTIL_PROG subvolume delete --subvolid $SUBVOLID $SCRATCH_MNT | _filter_btrfs_subvol_delete
 
 	echo "$msg"
 	$BTRFS_UTIL_PROG subvolume list $SCRATCH_MNT | $AWK_PROG '{ print $NF }'
diff --git a/tests/btrfs/208.out b/tests/btrfs/208.out
index 9b660699a4b2..dc5761ba1c87 100644
--- a/tests/btrfs/208.out
+++ b/tests/btrfs/208.out
@@ -6,12 +6,12 @@  Current subvolume ids:
 subvol1
 subvol2
 subvol3
-Delete subvolume (no-commit): 'SCRATCH_MNT/subvol1'
+Delete subvolume 'SCRATCH_MNT/subvol1'
 After deleting one subvolume:
 subvol2
 subvol3
-Delete subvolume (no-commit): 'SCRATCH_MNT/subvol3'
+Delete subvolume 'SCRATCH_MNT/subvol3'
 Last remaining subvolume:
 subvol2
-Delete subvolume (no-commit): 'SCRATCH_MNT/subvol2'
+Delete subvolume 'SCRATCH_MNT/subvol2'
 All subvolumes removed.
diff --git a/tests/btrfs/233 b/tests/btrfs/233
index 2b94a9c6befe..f2c1eba090be 100755
--- a/tests/btrfs/233
+++ b/tests/btrfs/233
@@ -21,6 +21,7 @@  _cleanup()
 
 # Import common functions.
 . ./common/filter
+. ./common/filter.btrfs
 . ./common/dmflakey
 
 # real QA test starts here
@@ -77,7 +78,7 @@  create_subvol_with_orphan()
 	# open, delete the subvolume, then 'sync' to durably persist the orphan
 	# item for the subvolume.
 	exec 73< $SCRATCH_MNT/testsv/foobar
-	$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/testsv | _filter_scratch
+	$BTRFS_UTIL_PROG subvolume delete $SCRATCH_MNT/testsv | _filter_btrfs_subvol_delete
 	sync
 
 	# Now silently drop writes on the device, close the file descriptor and
diff --git a/tests/btrfs/233.out b/tests/btrfs/233.out
index 492e959d895c..2754e900834e 100644
--- a/tests/btrfs/233.out
+++ b/tests/btrfs/233.out
@@ -1,5 +1,5 @@ 
 QA output created by 233
 Create subvolume 'SCRATCH_MNT/testsv'
-Delete subvolume (no-commit): 'SCRATCH_MNT/testsv'
+Delete subvolume 'SCRATCH_MNT/testsv'
 Create subvolume 'SCRATCH_MNT/testsv'
-Delete subvolume (no-commit): 'SCRATCH_MNT/testsv'
+Delete subvolume 'SCRATCH_MNT/testsv'
diff --git a/tests/btrfs/276 b/tests/btrfs/276
index 6470a2f6e62e..f15f20824350 100755
--- a/tests/btrfs/276
+++ b/tests/btrfs/276
@@ -12,6 +12,7 @@ 
 _begin_fstest auto snapshot fiemap remount
 
 . ./common/filter
+. ./common/filter.btrfs
 . ./common/attr
 
 _supported_fs btrfs
@@ -130,7 +131,7 @@  echo "Number of non-shared extents in range [512K, 512K + 64K): $(count_not_shar
 echo "Number of non-shared extents in range [249M, 249M + 64K): $(count_not_shared_extents 249M 64K)"
 
 # Now delete the snapshot.
-$BTRFS_UTIL_PROG subvolume delete -c $SCRATCH_MNT/snap | _filter_scratch
+$BTRFS_UTIL_PROG subvolume delete -c $SCRATCH_MNT/snap | _filter_btrfs_subvol_delete
 
 # We deleted the snapshot and committed the transaction used to delete it (-c),
 # but all its extents (both metadata and data) are actually only deleted in the
diff --git a/tests/btrfs/276.out b/tests/btrfs/276.out
index 197d8edc62ac..352e06b4d4b2 100644
--- a/tests/btrfs/276.out
+++ b/tests/btrfs/276.out
@@ -10,5 +10,5 @@  Number of non-shared extents in the whole file: 2
 Number of shared extents in the whole file: 1998
 Number of non-shared extents in range [512K, 512K + 64K): 1
 Number of non-shared extents in range [249M, 249M + 64K): 1
-Delete subvolume (commit): 'SCRATCH_MNT/snap'
+Delete subvolume 'SCRATCH_MNT/snap'
 Number of non-shared extents in the whole file: 2000