tests/scsi: Add tests for SCSI devices with gap zones

[bvanassche]

trafficstars

Verify that I/O on top of BTRFS or F2FS on top of zoned scsi_debug works fine if gap zones are present.

[bvanassche]

Hi @bvanassche thank you for moving the test cases to the zbd group. The PR title and commit message title still notes "tests/scsi". Then should be "tests/zbd" or simply "zbd".

This has been fixed.

Other than the comments I made in line, let me make two nit comments (I do not stick to them):

* Local variable declaration is not gathered at function start. A bit looks messy.

Isn't that a matter of personal preference? Every C++ style guide I know recommends to declare variables as close as possible to their first use.

* Double brackets [[ ]] are preferred to single bracket [ ]

Does this really matter for a simple -z test?

[kawasaki]

Hi @bvanassche thank you for moving the test cases to the zbd group. The PR title and commit message title still notes "tests/scsi". Then should be "tests/zbd" or simply "zbd".

This has been fixed.

Other than the comments I made in line, let me make two nit comments (I do not stick to them):

* Local variable declaration is not gathered at function start. A bit looks messy.

Isn't that a matter of personal preference? Every C++ style guide I know recommends to declare variables as close as possible to their first use.

Thanks for this feedback. To be honest, I'm struggling to strike a balance between tight coding style consistency for readability and code style freedom of the contributors. Probably I've been too picky.

Regarding the local variable declarations at the function start, I thought that Linux kernel guidelines would recommend it. However, it looks that Linux guidelines does not recommend it. So there is no reasoning to recommend it to blktests either.

* Double brackets [[ ]] are preferred to single bracket [ ]

Does this really matter for a simple -z test?

No. Actually, in the blktests' "new" script, it is guided as follows:

Use the bash [[ ]] form of tests instead of [ ].

I guess original author's intent was to just have consistency among blktests, and make the blktests code more bash oriented.

However, already there are many single brackets in blktests at this moment. On top of that, many of key contributors prefer single brackets. Now I'm thinking to remove the double brackets recommendation from the "new" script. I'm getting tired of requesting double brackets :)

Let's go with "local variable declarations close to the first use" and "single bracket".

I'll do another round of review tomorrow.

[kawasaki]

@bvanassche Thank you for updating the commit. Still I'm holding merge of this PR since zbd/009 fails with the mkfs.btrfs in latest btrfs-progs release. Let me wait a week or so to see how the next btrfs-progs version will be.

Also, I think zbd/009 should check btrfs-progs version to avoid the failure. Now I'm thinking to add the following patch on top of this PR. It will skip the test case when mkfs.btrfs has version 5.17 or 5.18.x.

diff --git a/tests/zbd/009 b/tests/zbd/009
index af430e7..483cbf6 100755
--- a/tests/zbd/009
+++ b/tests/zbd/009
@@ -8,12 +8,36 @@
 DESCRIPTION="test gap zone support with BTRFS"
 QUICK=1
 
+# compare 3 digits versions (e.g., 5.18.1)
+ver_a_is_before_b() {
+       local a1 a2 a3 b1 b2 b3
+
+       IFS='.' read -r a1 a2 a3 <<< "$1"
+       IFS='.' read -r b1 b2 b3 <<< "$2"
+       (((a1 * 65536 + a2 * 256 + a3) < (b1 * 65536 + b2 * 256 + b3)))
+}
+
+# mkfs.btrfs version from 5.17 to 5.18.x fails to format zoned block devices
+have_good_mkfs_btrfs() {
+       local ver
+
+       ver=$(mkfs.btrfs -V | sed 's/[^[:digit:]]*//')
+       if ver_a_is_before_b "$ver" 5.17; then
+               return 0
+       elif ver_a_is_before_b "$ver" 5.19; then
+               SKIP_REASONS+=("mkfs.btrfs ver $ver fails to format zoned block devices")
+               return 1
+       fi
+       return 0
+}
+
 requires() {
        _have_fio
        _have_driver btrfs
        _have_module_param scsi_debug zone_cap_mb
        _have_program mkfs.btrfs
        _have_scsi_debug
+       have_good_mkfs_btrfs
 }
 
 test() {

[kawasaki]

The new btrfs-progs is not yet released. Not sure when it will be, and it looks just losing time to wait for it. I've merged the changes, and added one more commit to check the mkfs.btrfs version. It took long, but this PR is done anyway. Thanks!