[PATCH] Revert "generic/730: add _require_scratch_shutdown"

[PATCH] Revert "generic/730: add _require_scratch_shutdown"

[Christoph Hellwig]

This reverts commit 68c9ac2ad74ba31c02275fcbb11d1cf90f0435b1.

The test does not actually use the xfs_io shutdown functionality,
and it does not use the scatch device.  Adding this checks causes
it to fail for setups that do not use SCRATCH_DEV at all.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 tests/generic/730 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/generic/730 b/tests/generic/730
index 6b5d319675f7..6251980e33e1 100755
--- a/tests/generic/730
+++ b/tests/generic/730
@@ -29,7 +29,6 @@ _require_scsi_debug
 size=$(_small_fs_size_mb 256)
 SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
 test -b "$SCSI_DEBUG_DEV" || _notrun "Failed to initialize scsi debug device"
-SCRATCH_DEV=$SCSI_DEBUG_DEV _require_scratch_shutdown
 echo "SCSI debug device $SCSI_DEBUG_DEV" >>$seqres.full
 
 run_check _mkfs_dev $SCSI_DEBUG_DEV
-- 
2.47.2

Re: [PATCH] Revert "generic/730: add _require_scratch_shutdown"

[Anand Jain]

On 24/7/25 15:25, Christoph Hellwig wrote:
> This reverts commit 68c9ac2ad74ba31c02275fcbb11d1cf90f0435b1.
> 
> The test does not actually use the xfs_io shutdown functionality,
> and it does not use the scatch device.  Adding this checks causes
> it to fail for setups that do not use SCRATCH_DEV at all.
> 

The test runs on the scsi_debug device but verifies whether the
file system type supports shutdown via the scratch device.

Since mainline Btrfs doesn't support shutdown yet, we see:

---------------------------------
generic/730 3s ... [not run] btrfs does not support shutdown
Ran: generic/730
---------------------------------

With the revert applied, the test case incorrectly runs
and fails on Btrfs.

---------------------------------
generic/730 3s ... - output mismatch (see 
/Volumes/work/ws/fstests/results//generic/730.out.bad)
     --- tests/generic/730.out	2025-07-01 17:41:30.991699629 +0800
     +++ /Volumes/work/ws/fstests/results//generic/730.out.bad 
2025-07-25 13:20:55.690043813 +0800
     @@ -1,2 +1 @@
      QA output created by 730
     -cat: -: Input/output error
     ...
     (Run 'diff -u /Volumes/work/ws/fstests/tests/generic/730.out 
/Volumes/work/ws/fstests/results//generic/730.out.bad'  to see the 
entire diff)
Ran: generic/730
---------------------------------

Thanks.


> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   tests/generic/730 | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/tests/generic/730 b/tests/generic/730
> index 6b5d319675f7..6251980e33e1 100755
> --- a/tests/generic/730
> +++ b/tests/generic/730
> @@ -29,7 +29,6 @@ _require_scsi_debug
>   size=$(_small_fs_size_mb 256)
>   SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
>   test -b "$SCSI_DEBUG_DEV" || _notrun "Failed to initialize scsi debug device"
> -SCRATCH_DEV=$SCSI_DEBUG_DEV _require_scratch_shutdown
>   echo "SCSI debug device $SCSI_DEBUG_DEV" >>$seqres.full
>   
>   run_check _mkfs_dev $SCSI_DEBUG_DEV

Re: [PATCH] Revert "generic/730: add _require_scratch_shutdown"

[Zorro Lang]

On Thu, Jul 24, 2025 at 09:25:14AM +0200, Christoph Hellwig wrote:
> This reverts commit 68c9ac2ad74ba31c02275fcbb11d1cf90f0435b1.
> 
> The test does not actually use the xfs_io shutdown functionality,
> and it does not use the scatch device.  Adding this checks causes
> it to fail for setups that do not use SCRATCH_DEV at all.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  tests/generic/730 | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/generic/730 b/tests/generic/730
> index 6b5d319675f7..6251980e33e1 100755
> --- a/tests/generic/730
> +++ b/tests/generic/730
> @@ -29,7 +29,6 @@ _require_scsi_debug
>  size=$(_small_fs_size_mb 256)
>  SCSI_DEBUG_DEV=`_get_scsi_debug_dev 512 512 0 $size`
>  test -b "$SCSI_DEBUG_DEV" || _notrun "Failed to initialize scsi debug device"
> -SCRATCH_DEV=$SCSI_DEBUG_DEV _require_scratch_shutdown

About why this case need "shutdown":
  https://lore.kernel.org/fstests/PUZPR04MB63169A8C1008035BB2D104568166A@PUZPR04MB6316.apcprd04.prod.outlook.com/

So looks like we'd better to notrun if shutdown isn't supported by fs on
SCSI_DEBUG_DEV. But if think "_require_scratch_shutdown" is a problem, maybe
we can avoid using _scratch_ things, use a local function to check shutdown
on SCSI_DEBUG_DEV manually, and notrun if it's not supported?


BTW, can I ask why do you need to test with TEST_DEV only :) Although
SCRATCH_DEV is optional (README says), I think nearly all testers test
with SCRATCH_DEV.

Thanks,
Zorro


>  echo "SCSI debug device $SCSI_DEBUG_DEV" >>$seqres.full
>  
>  run_check _mkfs_dev $SCSI_DEBUG_DEV
> -- 
> 2.47.2
> 
> 

Re: [PATCH] Revert "generic/730: add _require_scratch_shutdown"

[Christoph Hellwig]

On Fri, Jul 25, 2025 at 09:37:53PM +0800, Zorro Lang wrote:
> About why this case need "shutdown":
>   https://lore.kernel.org/fstests/PUZPR04MB63169A8C1008035BB2D104568166A@PUZPR04MB6316.apcprd04.prod.outlook.com/
> 
> So looks like we'd better to notrun if shutdown isn't supported by fs on
> SCSI_DEBUG_DEV. But if think "_require_scratch_shutdown" is a problem, maybe
> we can avoid using _scratch_ things, use a local function to check shutdown
> on SCSI_DEBUG_DEV manually, and notrun if it's not supported?

These are two almost entirely unrelated, except usually sharing some code
for the implementation:  _require_scratch_shutdown checks for the 
IOC_SHUDOWN ioctl, while this test coverd behavior of the file system
when the underlying block device is removed.

I can't really think of a feature test for shutting down the file system
on device removal, because it really should not be an optional feature..

> BTW, can I ask why do you need to test with TEST_DEV only :) Although
> SCRATCH_DEV is optional (README says), I think nearly all testers test
> with SCRATCH_DEV.

I'm working on an experimental xfs change that will need a lot of
tooling changes to handle the various scratch mkfs use cases.  So
while it's still bleeding edge I'd rather get all the TEST_DEV testing
than debuggіng/fixing xfstests.