| 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 |
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.
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
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 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