| Message ID | 20231016131103.966920-2-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] xfs/556: call _require_dm_target later | expand |
On Mon, Oct 16, 2023 at 03:11:03PM +0200, Christoph Hellwig wrote: > Add _require_scratch to a bunch of test that were using $SCRATCH_DEV > without that check. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > tests/xfs/184 | 1 + > tests/xfs/192 | 1 + > tests/xfs/200 | 1 + > tests/xfs/204 | 1 + > tests/xfs/232 | 1 + > 5 files changed, 5 insertions(+) > > diff --git a/tests/xfs/184 b/tests/xfs/184 > index 3bdd86ad..97f529f6 100755 > --- a/tests/xfs/184 > +++ b/tests/xfs/184 > @@ -19,6 +19,7 @@ _begin_fstest auto quick clone fiemap unshare > > # real QA test starts here > _supported_fs xfs > +_require_scratch > _require_scratch_delalloc > _require_scratch_reflink Oops. That _require_scratch_delalloc could've gone beneath the _require_scratch_reflink to take advantage of the _require_scratch call in _require_scratch_reflink. That said ... maybe _require_scratch_* calls should require the caller to have _require_scratch'd beforehand? And perhaps all _require_scratch_* functions should complain if ${RESULT_DIR}/require_scratch doesn't already exist. For this patch, though, everything looks fine: Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > _require_cp_reflink > diff --git a/tests/xfs/192 b/tests/xfs/192 > index eb577f15..ef7da55b 100755 > --- a/tests/xfs/192 > +++ b/tests/xfs/192 > @@ -19,6 +19,7 @@ _begin_fstest auto quick clone fiemap unshare > > # real QA test starts here > _supported_fs xfs > +_require_scratch > _require_scratch_delalloc > _require_scratch_reflink > _require_cp_reflink > diff --git a/tests/xfs/200 b/tests/xfs/200 > index b51b9a54..a9d6ce1b 100755 > --- a/tests/xfs/200 > +++ b/tests/xfs/200 > @@ -21,6 +21,7 @@ _begin_fstest auto quick clone fiemap unshare > > # real QA test starts here > _supported_fs xfs > +_require_scratch > _require_scratch_delalloc > _require_scratch_reflink > _require_cp_reflink > diff --git a/tests/xfs/204 b/tests/xfs/204 > index 7d6b79a8..7dacfa2d 100755 > --- a/tests/xfs/204 > +++ b/tests/xfs/204 > @@ -21,6 +21,7 @@ _begin_fstest auto quick clone fiemap unshare > > # real QA test starts here > _supported_fs xfs > +_require_scratch > _require_scratch_delalloc > _require_scratch_reflink > _require_cp_reflink > diff --git a/tests/xfs/232 b/tests/xfs/232 > index 59bbc436..0cb26fa1 100755 > --- a/tests/xfs/232 > +++ b/tests/xfs/232 > @@ -30,6 +30,7 @@ _cleanup() > > # real QA test starts here > _supported_fs xfs > +_require_scratch > _require_scratch_delalloc > _require_xfs_io_command "cowextsize" > _require_scratch_reflink > -- > 2.39.2 >
On Mon, Oct 16, 2023 at 11:41:24AM -0700, Darrick J. Wong wrote: > > +_require_scratch > > _require_scratch_delalloc > > _require_scratch_reflink > > Oops. That _require_scratch_delalloc could've gone beneath the > _require_scratch_reflink to take advantage of the _require_scratch call > in _require_scratch_reflink. > > That said ... maybe _require_scratch_* calls should require the caller > to have _require_scratch'd beforehand? And perhaps all > _require_scratch_* functions should complain if > ${RESULT_DIR}/require_scratch doesn't already exist. Either having a _require_scratch in all _require_scratch_* helpers or none sounds sensible to me, having it in some and nother others is rather confusing. Any preferences?
On Tue, Oct 17, 2023 at 06:25:07AM +0200, Christoph Hellwig wrote: > On Mon, Oct 16, 2023 at 11:41:24AM -0700, Darrick J. Wong wrote: > > > +_require_scratch > > > _require_scratch_delalloc > > > _require_scratch_reflink > > > > Oops. That _require_scratch_delalloc could've gone beneath the > > _require_scratch_reflink to take advantage of the _require_scratch call > > in _require_scratch_reflink. > > > > That said ... maybe _require_scratch_* calls should require the caller > > to have _require_scratch'd beforehand? And perhaps all > > _require_scratch_* functions should complain if > > ${RESULT_DIR}/require_scratch doesn't already exist. > > Either having a _require_scratch in all _require_scratch_* helpers or > none sounds sensible to me, having it in some and nother others is > rather confusing. Any preferences? IMIO, every test should be calling _require_scratch explicitly, and only once. The _require_scratch_XXX functions should complain if you haven't called _require_scratch. --D
diff --git a/tests/xfs/184 b/tests/xfs/184 index 3bdd86ad..97f529f6 100755 --- a/tests/xfs/184 +++ b/tests/xfs/184 @@ -19,6 +19,7 @@ _begin_fstest auto quick clone fiemap unshare # real QA test starts here _supported_fs xfs +_require_scratch _require_scratch_delalloc _require_scratch_reflink _require_cp_reflink diff --git a/tests/xfs/192 b/tests/xfs/192 index eb577f15..ef7da55b 100755 --- a/tests/xfs/192 +++ b/tests/xfs/192 @@ -19,6 +19,7 @@ _begin_fstest auto quick clone fiemap unshare # real QA test starts here _supported_fs xfs +_require_scratch _require_scratch_delalloc _require_scratch_reflink _require_cp_reflink diff --git a/tests/xfs/200 b/tests/xfs/200 index b51b9a54..a9d6ce1b 100755 --- a/tests/xfs/200 +++ b/tests/xfs/200 @@ -21,6 +21,7 @@ _begin_fstest auto quick clone fiemap unshare # real QA test starts here _supported_fs xfs +_require_scratch _require_scratch_delalloc _require_scratch_reflink _require_cp_reflink diff --git a/tests/xfs/204 b/tests/xfs/204 index 7d6b79a8..7dacfa2d 100755 --- a/tests/xfs/204 +++ b/tests/xfs/204 @@ -21,6 +21,7 @@ _begin_fstest auto quick clone fiemap unshare # real QA test starts here _supported_fs xfs +_require_scratch _require_scratch_delalloc _require_scratch_reflink _require_cp_reflink diff --git a/tests/xfs/232 b/tests/xfs/232 index 59bbc436..0cb26fa1 100755 --- a/tests/xfs/232 +++ b/tests/xfs/232 @@ -30,6 +30,7 @@ _cleanup() # real QA test starts here _supported_fs xfs +_require_scratch _require_scratch_delalloc _require_xfs_io_command "cowextsize" _require_scratch_reflink
Add _require_scratch to a bunch of test that were using $SCRATCH_DEV without that check. Signed-off-by: Christoph Hellwig <hch@lst.de> --- tests/xfs/184 | 1 + tests/xfs/192 | 1 + tests/xfs/200 | 1 + tests/xfs/204 | 1 + tests/xfs/232 | 1 + 5 files changed, 5 insertions(+)