public inbox for fio@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 00/21] verify fixes and a new test suite
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
@ 2025-02-27  7:28   ` fiotestbot
  2025-02-27 10:47   ` [PATCH 01/21] filesetup: remove unnecessary check Ankit Kumar
                     ` (21 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: fiotestbot @ 2025-02-27  7:28 UTC (permalink / raw)
  To: fio

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]


The result of fio's continuous integration tests was: None

For more details see https://github.com/fiotestbot/fio/actions/runs/13560919839

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 00/21] verify fixes and a new test suite
       [not found] <CGME20250227053350epcas5p31b3b5b47590d70804dade59842d87b27@epcas5p3.samsung.com>
@ 2025-02-27 10:47 ` Ankit Kumar
  2025-02-27  7:28   ` fiotestbot
                     ` (22 more replies)
  0 siblings, 23 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

This series fixes few issues with verify, and introduces a new test
suite.

Inital few patches add the missing client/server support for
verify_write_sequence. This also changes its behavior such that if
verify_write_sequence is explicitly enabled fio will not disable it
under any circumstance.

Numerous header seed mismatch issues have been reported. This series
introduces a new option verify_header_seed which is similar to
verify_write_sequence, which allow users to disable any header seed
verification. For certain workloads which used to overwrite header seed
before verification, we simply disable the header seed checks now. This
now includes a few more scenarios such as verify_only mode, read only
workloads and workloads with norandommap, where the header seed match is
not guaranteed.

Few more fixes related to verify_offset, workloads that have offset
modifiers, verification issue when multiple files are specified, are part
of this series.

Lastly this includes robust test suite for verify.
CI run result: https://github.com/vincentkfu/fio/actions/runs/13552248490

Note: The fixes in this series doesn't cover experimental_verify and any
workload with variable block sizes.

Ankit Kumar (14):
  filesetup: remove unnecessary check
  verify: add missing client/server support for verify_write_sequence
  init: write sequence behavior change for verify_only mode
  fio: add verify_header_seed option
  verify: disable header seed checking instead of overwriting it
  verify: enable header seed check for 100% write jobs
  verify: disable header seed check for verify_only jobs
  verify: header seed check for read only workloads
  verify: fix verify issues with norandommap
  verify: disable write sequence checks with norandommap and iodepth > 1
  backend: fix verify issue during readwrite
  init: fixup verify_offset option
  verify: fix verify issue with offest modifiers
  verify: adjust fio_offset_overlap_risk to include randommap

Vincent Fu (7):
  t/fiotestcommon: do not require nvmecdev argument for Requirements
  t/fiotestlib: improve JSON decoding
  t/fiotestlib: display stderr size when it is not empty but should be
  t/verify.py: Add verify test script
  t/fiotestcommon: add a success pattern for long tests
  t/run-fio-test: add t/verify.py
  ci: add nightly test for verify

 .github/workflows/ci.yml |   2 +
 HOWTO.rst                |  52 ++-
 backend.c                |  18 +-
 cconv.c                  |   4 +
 ci/actions-full-test.sh  |  13 +
 filesetup.c              |  14 +-
 fio.1                    |  46 ++-
 fio.h                    |   9 +
 init.c                   |  43 ++-
 iolog.c                  |   9 +-
 options.c                |  11 +
 server.h                 |   2 +-
 t/fiotestcommon.py       |   7 +-
 t/fiotestlib.py          |  20 +-
 t/run-fio-tests.py       |   8 +
 t/verify.py              | 799 +++++++++++++++++++++++++++++++++++++++
 thread_options.h         |   3 +
 verify.c                 |  10 +-
 18 files changed, 1005 insertions(+), 65 deletions(-)
 create mode 100755 t/verify.py

-- 
2.25.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 01/21] filesetup: remove unnecessary check
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
  2025-02-27  7:28   ` fiotestbot
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 02/21] verify: add missing client/server support for verify_write_sequence Ankit Kumar
                     ` (20 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

If read_iolog_file is set, the goto statement moves it beyond this
point. So remove this redundant check.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 filesetup.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/filesetup.c b/filesetup.c
index cb42a852..50406c69 100644
--- a/filesetup.c
+++ b/filesetup.c
@@ -1388,16 +1388,10 @@ int setup_files(struct thread_data *td)
 	if (err)
 		goto err_out;
 
-	/*
-	 * iolog already set the total io size, if we read back
-	 * stored entries.
-	 */
-	if (!o->read_iolog_file) {
-		if (o->io_size)
-			td->total_io_size = o->io_size * o->loops;
-		else
-			td->total_io_size = o->size * o->loops;
-	}
+	if (o->io_size)
+		td->total_io_size = o->io_size * o->loops;
+	else
+		td->total_io_size = o->size * o->loops;
 
 done:
 	if (td->o.zone_mode == ZONE_MODE_ZBD) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 02/21] verify: add missing client/server support for verify_write_sequence
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
  2025-02-27  7:28   ` fiotestbot
  2025-02-27 10:47   ` [PATCH 01/21] filesetup: remove unnecessary check Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 03/21] init: write sequence behavior change for verify_only mode Ankit Kumar
                     ` (19 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

Ensure that we convert verify_write_sequence option for client/server.

Fixes: 2dd80ee4 ("fio: Support verify_write_sequence")

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 cconv.c          | 2 ++
 server.h         | 2 +-
 thread_options.h | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/cconv.c b/cconv.c
index 9571f1a8..ef3fbaa8 100644
--- a/cconv.c
+++ b/cconv.c
@@ -182,6 +182,7 @@ int convert_thread_options_to_cpu(struct thread_options *o,
 	o->verify_state = le32_to_cpu(top->verify_state);
 	o->verify_interval = le32_to_cpu(top->verify_interval);
 	o->verify_offset = le32_to_cpu(top->verify_offset);
+	o->verify_write_sequence = le32_to_cpu(top->verify_write_sequence);
 
 	o->verify_pattern_bytes = le32_to_cpu(top->verify_pattern_bytes);
 	o->buffer_pattern_bytes = le32_to_cpu(top->buffer_pattern_bytes);
@@ -442,6 +443,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
 	top->verify_state = cpu_to_le32(o->verify_state);
 	top->verify_interval = cpu_to_le32(o->verify_interval);
 	top->verify_offset = cpu_to_le32(o->verify_offset);
+	top->verify_write_sequence = cpu_to_le32(o->verify_write_sequence);
 	top->verify_pattern_bytes = cpu_to_le32(o->verify_pattern_bytes);
 	top->verify_fatal = cpu_to_le32(o->verify_fatal);
 	top->verify_dump = cpu_to_le32(o->verify_dump);
diff --git a/server.h b/server.h
index 449c18cf..d31cd688 100644
--- a/server.h
+++ b/server.h
@@ -51,7 +51,7 @@ struct fio_net_cmd_reply {
 };
 
 enum {
-	FIO_SERVER_VER			= 107,
+	FIO_SERVER_VER			= 108,
 
 	FIO_SERVER_MAX_FRAGMENT_PDU	= 1024,
 	FIO_SERVER_MAX_CMD_MB		= 2048,
diff --git a/thread_options.h b/thread_options.h
index d0e0a4ae..4d8addc4 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -484,6 +484,8 @@ struct thread_options_pack {
 	uint32_t experimental_verify;
 	uint32_t verify_state;
 	uint32_t verify_state_save;
+	uint32_t verify_write_sequence;
+	uint32_t pad2;
 	uint32_t use_thread;
 	uint32_t unlink;
 	uint32_t unlink_each_loop;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 03/21] init: write sequence behavior change for verify_only mode
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (2 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 02/21] verify: add missing client/server support for verify_write_sequence Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 04/21] fio: add verify_header_seed option Ankit Kumar
                     ` (18 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

Change the behavior for verify_only mode to not disable
verify_write_sequence unless its explicitly enabled.

Update the fio doc. accordingly.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst | 3 ++-
 fio.1     | 3 ++-
 init.c    | 6 ++++--
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 3f2bde18..c7c131e7 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -3816,7 +3816,8 @@ Verification
 	invocation of this workload. This option allows one to check data multiple
 	times at a later date without overwriting it. This option makes sense only
 	for workloads that write data, and does not support workloads with the
-	:option:`time_based` option set.
+	:option:`time_based` option set. :option:`verify_write_sequence` will be
+	disabled in this mode, unless its explicitly enabled.
 
 .. option:: do_verify=bool
 
diff --git a/fio.1 b/fio.1
index 3c5f02b3..88d3be81 100644
--- a/fio.1
+++ b/fio.1
@@ -3542,7 +3542,8 @@ Do not perform specified workload, only verify data still matches previous
 invocation of this workload. This option allows one to check data multiple
 times at a later date without overwriting it. This option makes sense only
 for workloads that write data, and does not support workloads with the
-\fBtime_based\fR option set.
+\fBtime_based\fR option set. \fBverify_write_sequence\fR option will be
+disabled in this mode, unless its explicitly enabled.
 .TP
 .BI do_verify \fR=\fPbool
 Run the verify phase after a write phase. Only valid if \fBverify\fR is
diff --git a/init.c b/init.c
index 96a03d98..dc3e7ed1 100644
--- a/init.c
+++ b/init.c
@@ -854,8 +854,10 @@ static int fixup_options(struct thread_data *td)
 			o->verify_interval = gcd(o->min_bs[DDIR_WRITE],
 							o->max_bs[DDIR_WRITE]);
 
-		if (td->o.verify_only)
-			o->verify_write_sequence = 0;
+		if (o->verify_only) {
+			if (!fio_option_is_set(o, verify_write_sequence))
+				o->verify_write_sequence = 0;
+		}
 	}
 
 	if (td->o.oatomic) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 04/21] fio: add verify_header_seed option
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (3 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 03/21] init: write sequence behavior change for verify_only mode Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 05/21] verify: disable header seed checking instead of overwriting it Ankit Kumar
                     ` (17 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

Add a new option to disable the verify header seed check. The header
seed check is enabled by default.
There have been numerous issues observed with header seed mismatch. Hence
this allows end user to disable this check, and proceed with the checksum
verification. This is similar to option verify_write_sequence, which
allows the capability to disable write sequence number check.

Update the documentation accordingly.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst        | 14 ++++++++++++--
 cconv.c          |  2 ++
 fio.1            | 12 ++++++++++--
 options.c        | 11 +++++++++++
 server.h         |  2 +-
 thread_options.h |  3 ++-
 verify.c         |  2 +-
 7 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index c7c131e7..a221b9d8 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -3830,8 +3830,9 @@ Verification
 	of the job. Each verification method also implies verification of special
 	header, which is written to the beginning of each block. This header also
 	includes meta information, like offset of the block, block number, timestamp
-	when block was written, etc.  :option:`verify` can be combined with
-	:option:`verify_pattern` option.  The allowed values are:
+	when block was written, initial seed value used to generate the buffer
+	contents etc. :option:`verify` can be combined with :option:`verify_pattern`
+	option.  The allowed values are:
 
 		**md5**
 			Use an md5 sum of the data area and store it in the header of
@@ -4029,6 +4030,15 @@ Verification
         fail).
         Defaults to true.
 
+.. option:: verify_header_seed=bool
+
+	Verify the header seed value which was used to generate the buffer contents.
+	In certain scenarios with read / verify only workloads, when
+	:option:`norandommap` is enabled, with offset modifiers
+	(refer :option:`readwrite` and :option:`rw_sequencer`) etc verification of
+	header seed may fail. Disabling this option will mean that header seed
+	checking is skipped. Defaults to true.
+
 .. option:: trim_percentage=int
 
 	Number of verify blocks to discard/trim.
diff --git a/cconv.c b/cconv.c
index ef3fbaa8..df841703 100644
--- a/cconv.c
+++ b/cconv.c
@@ -183,6 +183,7 @@ int convert_thread_options_to_cpu(struct thread_options *o,
 	o->verify_interval = le32_to_cpu(top->verify_interval);
 	o->verify_offset = le32_to_cpu(top->verify_offset);
 	o->verify_write_sequence = le32_to_cpu(top->verify_write_sequence);
+	o->verify_header_seed = le32_to_cpu(top->verify_header_seed);
 
 	o->verify_pattern_bytes = le32_to_cpu(top->verify_pattern_bytes);
 	o->buffer_pattern_bytes = le32_to_cpu(top->buffer_pattern_bytes);
@@ -444,6 +445,7 @@ void convert_thread_options_to_net(struct thread_options_pack *top,
 	top->verify_interval = cpu_to_le32(o->verify_interval);
 	top->verify_offset = cpu_to_le32(o->verify_offset);
 	top->verify_write_sequence = cpu_to_le32(o->verify_write_sequence);
+	top->verify_header_seed = cpu_to_le32(o->verify_header_seed);
 	top->verify_pattern_bytes = cpu_to_le32(o->verify_pattern_bytes);
 	top->verify_fatal = cpu_to_le32(o->verify_fatal);
 	top->verify_dump = cpu_to_le32(o->verify_dump);
diff --git a/fio.1 b/fio.1
index 88d3be81..dd6d9546 100644
--- a/fio.1
+++ b/fio.1
@@ -3554,8 +3554,9 @@ If writing to a file, fio can verify the file contents after each iteration
 of the job. Each verification method also implies verification of special
 header, which is written to the beginning of each block. This header also
 includes meta information, like offset of the block, block number, timestamp
-when block was written, etc. \fBverify\fR can be combined with
-\fBverify_pattern\fR option. The allowed values are:
+when block was written, initial seed value used to generate the buffer
+contents, etc. \fBverify\fR can be combined with \fBverify_pattern\fR option.
+The allowed values are:
 .RS
 .RS
 .TP
@@ -3751,6 +3752,13 @@ useful for testing atomic writes, as it means that checksum verification can
 still be attempted. For when \fBatomic\fR is enabled, checksum verification
 is expected to succeed (while write sequence checking can still fail).
 .TP
+.BI verify_header_seed \fR=\fPbool
+Verify the header seed value which was used to generate the buffer contents.
+In certain scenarios with read / verify only workloads, when \fBnorandommap\fR
+is enabled, with offset modifiers (refer options \fBreadwrite\fR and
+\fBrw_sequencer\fR), etc verification of header seed may fail. Disabling this
+option will mean that header seed checking is skipped. Defaults to true.
+.TP
 .BI trim_percentage \fR=\fPint
 Number of verify blocks to discard/trim.
 .TP
diff --git a/options.c b/options.c
index c35878f7..416bc91c 100644
--- a/options.c
+++ b/options.c
@@ -3408,6 +3408,17 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.category = FIO_OPT_C_IO,
 		.group	= FIO_OPT_G_VERIFY,
 	},
+	{
+		.name	= "verify_header_seed",
+		.lname	= "Verify header seed",
+		.off1	= offsetof(struct thread_options, verify_header_seed),
+		.type	= FIO_OPT_BOOL,
+		.def	= "1",
+		.help	= "Verify the header seed used to generate the buffer contents",
+		.parent	= "verify",
+		.category = FIO_OPT_C_IO,
+		.group	= FIO_OPT_G_VERIFY,
+	},
 #ifdef FIO_HAVE_TRIM
 	{
 		.name	= "trim_percentage",
diff --git a/server.h b/server.h
index d31cd688..e5968112 100644
--- a/server.h
+++ b/server.h
@@ -51,7 +51,7 @@ struct fio_net_cmd_reply {
 };
 
 enum {
-	FIO_SERVER_VER			= 108,
+	FIO_SERVER_VER			= 109,
 
 	FIO_SERVER_MAX_FRAGMENT_PDU	= 1024,
 	FIO_SERVER_MAX_CMD_MB		= 2048,
diff --git a/thread_options.h b/thread_options.h
index 4d8addc4..d25ba891 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -157,6 +157,7 @@ struct thread_options {
 	unsigned int verify_state;
 	unsigned int verify_state_save;
 	unsigned int verify_write_sequence;
+	unsigned int verify_header_seed;
 	unsigned int use_thread;
 	unsigned int unlink;
 	unsigned int unlink_each_loop;
@@ -485,7 +486,7 @@ struct thread_options_pack {
 	uint32_t verify_state;
 	uint32_t verify_state_save;
 	uint32_t verify_write_sequence;
-	uint32_t pad2;
+	uint32_t verify_header_seed;
 	uint32_t use_thread;
 	uint32_t unlink;
 	uint32_t unlink_each_loop;
diff --git a/verify.c b/verify.c
index 570c888f..49d44982 100644
--- a/verify.c
+++ b/verify.c
@@ -833,7 +833,7 @@ static int verify_header(struct io_u *io_u, struct thread_data *td,
 			hdr->len, hdr_len);
 		goto err;
 	}
-	if (hdr->rand_seed != io_u->rand_seed) {
+	if (td->o.verify_header_seed && (hdr->rand_seed != io_u->rand_seed)) {
 		log_err("verify: bad header rand_seed %"PRIu64
 			", wanted %"PRIu64,
 			hdr->rand_seed, io_u->rand_seed);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 05/21] verify: disable header seed checking instead of overwriting it
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (4 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 04/21] fio: add verify_header_seed option Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 06/21] verify: enable header seed check for 100% write jobs Ankit Kumar
                     ` (16 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

The existing header seed is overwritten if zone reset frequency is set or
if verify backlog is enabled. Disable verify header seed check for these
scenarios, unless there is an explicit request to enable it.

Note: There is no fio behavior change intended by this patch.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 init.c   | 11 +++++++++++
 verify.c |  8 --------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/init.c b/init.c
index dc3e7ed1..99661ca8 100644
--- a/init.c
+++ b/init.c
@@ -858,6 +858,17 @@ static int fixup_options(struct thread_data *td)
 			if (!fio_option_is_set(o, verify_write_sequence))
 				o->verify_write_sequence = 0;
 		}
+
+		/*
+		 * Disable rand_seed check when we have verify_backlog, or
+		 * zone reset frequency for zonemode=zbd.
+		 * Unless we were explicitly asked to enable it.
+		 */
+		if (!td_rw(td) || (td->flags & TD_F_VER_BACKLOG) ||
+		    o->zrf.u.f) {
+			if (!fio_option_is_set(o, verify_header_seed))
+				o->verify_header_seed = 0;
+		}
 	}
 
 	if (td->o.oatomic) {
diff --git a/verify.c b/verify.c
index 49d44982..928bdd54 100644
--- a/verify.c
+++ b/verify.c
@@ -934,14 +934,6 @@ int verify_io_u(struct thread_data *td, struct io_u **io_u_ptr)
 			memswp(p, p + td->o.verify_offset, header_size);
 		hdr = p;
 
-		/*
-		 * Make rand_seed check pass when have verify_backlog or
-		 * zone reset frequency for zonemode=zbd.
-		 */
-		if (!td_rw(td) || (td->flags & TD_F_VER_BACKLOG) ||
-		    td->o.zrf.u.f)
-			io_u->rand_seed = hdr->rand_seed;
-
 		if (td->o.verify != VERIFY_PATTERN_NO_HDR) {
 			ret = verify_header(io_u, td, hdr, hdr_num, hdr_inc);
 			if (ret)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 06/21] verify: enable header seed check for 100% write jobs
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (5 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 05/21] verify: disable header seed checking instead of overwriting it Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 07/21] verify: disable header seed check for verify_only jobs Ankit Kumar
                     ` (15 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

There are 3 modes where verify can be performed. write, read and
readwrite. The existing readwrite condition prohibits header seed check
for write or read workloads. For write workloads, there shouldn't be any
extra limitation that triggers header seed mismatch which cannot be
triggered with readwrite workloads. Hence modify this condition to only
disable verify header seed checks for read workload.

The subsequent patches fixes header seed mismatch issues.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init.c b/init.c
index 99661ca8..7a1a6840 100644
--- a/init.c
+++ b/init.c
@@ -864,7 +864,7 @@ static int fixup_options(struct thread_data *td)
 		 * zone reset frequency for zonemode=zbd.
 		 * Unless we were explicitly asked to enable it.
 		 */
-		if (!td_rw(td) || (td->flags & TD_F_VER_BACKLOG) ||
+		if (!td_write(td) || (td->flags & TD_F_VER_BACKLOG) ||
 		    o->zrf.u.f) {
 			if (!fio_option_is_set(o, verify_header_seed))
 				o->verify_header_seed = 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 07/21] verify: disable header seed check for verify_only jobs
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (6 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 06/21] verify: enable header seed check for 100% write jobs Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 08/21] verify: header seed check for read only workloads Ankit Kumar
                     ` (14 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

For the invoked verify_only job, header seed can match only if it
exactly matches the original write job. This means either randrepeat
should be true, or we must use the same randseed which was used with the
original write job. After write the verify_only workload shouldn't be
changed from sequential to random or vice versa.

Considering these constraints disable verify_header_seed for verify_only
jobs. Users will still be able to enable header seed checking if they
explicitly set the verify_header_sequence option.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst | 5 +++--
 fio.1     | 5 +++--
 init.c    | 3 +++
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index a221b9d8..05bdeebb 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -3816,8 +3816,9 @@ Verification
 	invocation of this workload. This option allows one to check data multiple
 	times at a later date without overwriting it. This option makes sense only
 	for workloads that write data, and does not support workloads with the
-	:option:`time_based` option set. :option:`verify_write_sequence` will be
-	disabled in this mode, unless its explicitly enabled.
+	:option:`time_based` option set. :option:`verify_write_sequence` and
+	:option:`verify_header_seed` will be disabled in this mode, unless they are
+	explicitly enabled.
 
 .. option:: do_verify=bool
 
diff --git a/fio.1 b/fio.1
index dd6d9546..1342a23a 100644
--- a/fio.1
+++ b/fio.1
@@ -3542,8 +3542,9 @@ Do not perform specified workload, only verify data still matches previous
 invocation of this workload. This option allows one to check data multiple
 times at a later date without overwriting it. This option makes sense only
 for workloads that write data, and does not support workloads with the
-\fBtime_based\fR option set. \fBverify_write_sequence\fR option will be
-disabled in this mode, unless its explicitly enabled.
+\fBtime_based\fR option set. Options \fBverify_write_sequence\fR and
+\fBverify_header_seed\fR will be disabled in this mode, unless they are
+explicitly enabled.
 .TP
 .BI do_verify \fR=\fPbool
 Run the verify phase after a write phase. Only valid if \fBverify\fR is
diff --git a/init.c b/init.c
index 7a1a6840..3d6230cb 100644
--- a/init.c
+++ b/init.c
@@ -857,6 +857,9 @@ static int fixup_options(struct thread_data *td)
 		if (o->verify_only) {
 			if (!fio_option_is_set(o, verify_write_sequence))
 				o->verify_write_sequence = 0;
+
+			if (!fio_option_is_set(o, verify_header_seed))
+				o->verify_header_seed = 0;
 		}
 
 		/*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 08/21] verify: header seed check for read only workloads
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (7 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 07/21] verify: disable header seed check for verify_only jobs Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 09/21] verify: fix verify issues with norandommap Ankit Kumar
                     ` (13 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

For read jobs, users should have the option to verify header seeds at a
later point of time. Currently for read jobs header seeds are not
generated

Consider the below mentioned write followed by read workloads. Here fio
should allow header seed verification.

fio --name=test --filesize=16k --rw=randwrite --verify=md5
fio --name=test --filesize=16k --rw=randread --verify=md5 --verify_header_seed=1

However there are other scenarios where header seed verification will
fail. These include:
 * randrepeat is set to false, leading to different seed across runs.
 * randseed is different across write and read workloads.
 * Read workload is changed from sequential to random or vice versa
   across runs.
 * Read workloads run in the same invocation as write, i.e. a write job
   followed by a stonewall read job. Header seed verification will fail
   because random seeds vary between jobs. Refer t/jobs/t0029.fio

If verify_header_seed is explicitly enabled, fio will verify header seed
for the workload.

This reverts part of commit mentioned below
Fixes: def41e55 ("verify: decouple seed generation from buffer fill")

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst | 16 ++++++++++++----
 backend.c | 11 +++++++++++
 fio.1     | 15 +++++++++++----
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 05bdeebb..6d462af3 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -3907,10 +3907,18 @@ Verification
 			:option:`ioengine`\=null, not for much else.
 
 	This option can be used for repeated burn-in tests of a system to make sure
-	that the written data is also correctly read back. If the data direction
-	given is a read or random read, fio will assume that it should verify a
-	previously written file. If the data direction includes any form of write,
-	the verify will be of the newly written data.
+	that the written data is also correctly read back.
+
+	If the data direction given is a read or random read, fio will assume that
+	it should verify a previously written file. In this scenario fio will not
+	verify the block number written in the header. The header seed won't be
+	verified, unless its explicitly requested by setting
+	:option:`verify_header_seed`. Note in this scenario the header seed check
+	will only work if the read invocation exactly matches the original write
+	invocation.
+
+	If the data direction includes any form of write, the verify will be of the
+	newly written data.
 
 	To avoid false verification errors, do not use the norandommap option when
 	verifying data with async I/O engines and I/O depths > 1.  Or use the
diff --git a/backend.c b/backend.c
index f3e5b56a..f5cfffdb 100644
--- a/backend.c
+++ b/backend.c
@@ -1069,6 +1069,17 @@ static void do_io(struct thread_data *td, uint64_t *bytes_done)
 		if (td->o.verify != VERIFY_NONE && io_u->ddir == DDIR_READ &&
 		    ((io_u->flags & IO_U_F_VER_LIST) || !td_rw(td))) {
 
+			/*
+			 * For read only workloads generate the seed. This way
+			 * we can still verify header seed at any later
+			 * invocation.
+			 */
+			if (!td_write(td) && !td->o.verify_pattern_bytes) {
+				io_u->rand_seed = __rand(&td->verify_state);
+				if (sizeof(int) != sizeof(long *))
+					io_u->rand_seed *= __rand(&td->verify_state);
+			}
+
 			if (verify_state_should_stop(td, io_u)) {
 				put_io_u(td, io_u);
 				break;
diff --git a/fio.1 b/fio.1
index 1342a23a..3fbc1657 100644
--- a/fio.1
+++ b/fio.1
@@ -3634,10 +3634,17 @@ Only pretend to verify. Useful for testing internals with
 .RE
 .P
 This option can be used for repeated burn\-in tests of a system to make sure
-that the written data is also correctly read back. If the data direction
-given is a read or random read, fio will assume that it should verify a
-previously written file. If the data direction includes any form of write,
-the verify will be of the newly written data.
+that the written data is also correctly read back.
+.P
+If the data direction given is a read or random read, fio will assume that it
+should verify a previously written file. In this scenario fio will not verify
+the block number written in the header. The header seed won't be verified,
+unless its explicitly requested by setting \fBverify_header_seed\fR option.
+Note in this scenario the header seed check will only work if the read
+invocation exactly matches the original write invocation.
+.P
+If the data direction includes any form of write, the verify will be of the
+newly written data.
 .P
 To avoid false verification errors, do not use the norandommap option when
 verifying data with async I/O engines and I/O depths > 1.  Or use the
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 09/21] verify: fix verify issues with norandommap
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (8 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 08/21] verify: header seed check for read only workloads Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 10/21] verify: disable write sequence checks with norandommap and iodepth > 1 Ankit Kumar
                     ` (12 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

When norandommap is enabled, fio logs the I/O entries in a RB tree. This
is to account for offset overlaps and overwrites. Then during verify
phase, the I/O entries are picked from the top and in this case the
smallest offset is verified first and so on. This creates a mismatch
during the header verification as the seed generated at the time of read
differs from what was logged during write. Skip seed verification in
this scenario.

fixes #1756

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst | 12 +++++++-----
 fio.1     |  3 ++-
 init.c    |  6 +++---
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 6d462af3..fa6b44a5 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -1564,11 +1564,13 @@ I/O type
 	this option is given, fio will just get a new random offset without looking
 	at past I/O history. This means that some blocks may not be read or written,
 	and that some blocks may be read/written more than once. If this option is
-	used with :option:`verify` and multiple blocksizes (via :option:`bsrange`),
-	only intact blocks are verified, i.e., partially-overwritten blocks are
-	ignored.  With an async I/O engine and an I/O depth > 1, it is possible for
-	the same block to be overwritten, which can cause verification errors.  Either
-	do not use norandommap in this case, or also use the lfsr random generator.
+	used with :option:`verify` then :option:`verify_header_seed` will be
+	disabled. If this option is used with :option:`verify` and multiple blocksizes
+	(via :option:`bsrange`), only intact blocks are verified, i.e.,
+	partially-overwritten blocks are ignored. With an async I/O engine and an I/O
+	depth > 1, it is possible for the same block to be overwritten, which can
+	cause verification errors. Either do not use norandommap in this case, or also
+	use the lfsr random generator.
 
 .. option:: softrandommap=bool
 
diff --git a/fio.1 b/fio.1
index 3fbc1657..07f09f07 100644
--- a/fio.1
+++ b/fio.1
@@ -1368,7 +1368,8 @@ Normally fio will cover every block of the file when doing random I/O. If
 this option is given, fio will just get a new random offset without looking
 at past I/O history. This means that some blocks may not be read or written,
 and that some blocks may be read/written more than once. If this option is
-used with \fBverify\fR and multiple blocksizes (via \fBbsrange\fR),
+used with \fBverify\fR then \fBverify_header_seed\fR will be disabled. If this
+option is used with \fBverify\fR and multiple blocksizes (via \fBbsrange\fR),
 only intact blocks are verified, i.e., partially-overwritten blocks are
 ignored.  With an async I/O engine and an I/O depth > 1, it is possible for
 the same block to be overwritten, which can cause verification errors.  Either
diff --git a/init.c b/init.c
index 3d6230cb..91f056e3 100644
--- a/init.c
+++ b/init.c
@@ -863,12 +863,12 @@ static int fixup_options(struct thread_data *td)
 		}
 
 		/*
-		 * Disable rand_seed check when we have verify_backlog, or
-		 * zone reset frequency for zonemode=zbd.
+		 * Disable rand_seed check when we have verify_backlog,
+		 * zone reset frequency for zonemode=zbd, or norandommap.
 		 * Unless we were explicitly asked to enable it.
 		 */
 		if (!td_write(td) || (td->flags & TD_F_VER_BACKLOG) ||
-		    o->zrf.u.f) {
+		    o->zrf.u.f || o->norandommap) {
 			if (!fio_option_is_set(o, verify_header_seed))
 				o->verify_header_seed = 0;
 		}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 10/21] verify: disable write sequence checks with norandommap and iodepth > 1
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (9 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 09/21] verify: fix verify issues with norandommap Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 11/21] backend: fix verify issue during readwrite Ankit Kumar
                     ` (11 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

With norandommap for async I/O engines specifying I/O depth > 1, it is
possible that two or more writes with the same offset are queued at once.
When fio tries to verify the block, it may find a numberio mismatch
because the writes did not land on the media in the order that they were
queued. Avoid these spurious failures by disabling sequence number
checking. Users will still be able to enable sequence number checking
if they explicitly set the verify_header_sequence option.

fio -name=verify -ioengine=libaio -rw=randwrite -verify=sha512 -direct=1 \
-iodepth=32 -filesize=16M -bs=512 -norandommap=1 -debug=io,verify

Below is the truncated log for the above command demonstrating the issue.
This includes extra log entries when write sequence number is saved and
retrieved.

set: io_u->numberio=28489, off=0x5f2400
queue: io_u 0x5b8039e30d40: off=0x5f2400,len=0x200,ddir=1,file=verify.0.0
set: io_u->numberio=28574, off=0x5f2400
iolog: overlap 6235136/512, 6235136/512
queue: io_u 0x5b8039e75500: off=0x5f2400,len=0x200,ddir=1,file=verify.0.0
complete: io_u 0x5b8039e75500: off=0x5f2400,len=0x200,ddir=1,file=verify.0.0
complete: io_u 0x5b8039e30d40: off=0x5f2400,len=0x200,ddir=1,file=verify.0.0

retrieve: io_u->numberio=28574, off=0x5f2400
queue: io_u 0x5b8039e1db40: off=0x5f2400,len=0x200,ddir=0,file=verify.0.0

bad header numberio 28489, wanted 28574 at file verify.0.0 offset 6235136, length 512 (requested block: offset=6235136, length=512)

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst |  5 ++---
 fio.1     |  5 ++---
 init.c    | 11 +++++++++++
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index fa6b44a5..ca0044ce 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -1568,9 +1568,8 @@ I/O type
 	disabled. If this option is used with :option:`verify` and multiple blocksizes
 	(via :option:`bsrange`), only intact blocks are verified, i.e.,
 	partially-overwritten blocks are ignored. With an async I/O engine and an I/O
-	depth > 1, it is possible for the same block to be overwritten, which can
-	cause verification errors. Either do not use norandommap in this case, or also
-	use the lfsr random generator.
+	depth > 1, header write sequence number verification will be disabled. See
+	:option:`verify_write_sequence`.
 
 .. option:: softrandommap=bool
 
diff --git a/fio.1 b/fio.1
index 07f09f07..fd580258 100644
--- a/fio.1
+++ b/fio.1
@@ -1371,9 +1371,8 @@ and that some blocks may be read/written more than once. If this option is
 used with \fBverify\fR then \fBverify_header_seed\fR will be disabled. If this
 option is used with \fBverify\fR and multiple blocksizes (via \fBbsrange\fR),
 only intact blocks are verified, i.e., partially-overwritten blocks are
-ignored.  With an async I/O engine and an I/O depth > 1, it is possible for
-the same block to be overwritten, which can cause verification errors.  Either
-do not use norandommap in this case, or also use the lfsr random generator.
+ignored. With an async I/O engine and an I/O depth > 1, header write sequence
+number verification will be disabled. See \fBverify_write_sequence\fR.
 .TP
 .BI softrandommap \fR=\fPbool
 See \fBnorandommap\fR. If fio runs with the random block map enabled and
diff --git a/init.c b/init.c
index 91f056e3..00a3a8c7 100644
--- a/init.c
+++ b/init.c
@@ -862,6 +862,17 @@ static int fixup_options(struct thread_data *td)
 				o->verify_header_seed = 0;
 		}
 
+		if (o->norandommap && !td_ioengine_flagged(td, FIO_SYNCIO) &&
+		    o->iodepth > 1) {
+			/*
+			 * Disable write sequence checks with norandommap and
+			 * iodepth > 1.
+			 * Unless we were explicitly asked to enable it.
+			 */
+			if (!fio_option_is_set(o, verify_write_sequence))
+				o->verify_write_sequence = 0;
+		}
+
 		/*
 		 * Disable rand_seed check when we have verify_backlog,
 		 * zone reset frequency for zonemode=zbd, or norandommap.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 11/21] backend: fix verify issue during readwrite
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (10 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 10/21] verify: disable write sequence checks with norandommap and iodepth > 1 Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 12/21] init: fixup verify_offset option Ankit Kumar
                     ` (10 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

In readwrite mode if specified io_size > size, offsets can overlap.
This will result in verify errors. Add check to handle this case.

Fixes: d782b76f ("Don break too early in readwrite mode")

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 backend.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/backend.c b/backend.c
index f5cfffdb..b75eea80 100644
--- a/backend.c
+++ b/backend.c
@@ -978,8 +978,11 @@ static void do_io(struct thread_data *td, uint64_t *bytes_done)
 	if (td_write(td) && td_random(td) && td->o.norandommap)
 		total_bytes = max(total_bytes, (uint64_t) td->o.io_size);
 
-	/* Don't break too early if io_size > size */
-	if (td_rw(td) && !td_random(td))
+	/*
+	 * Don't break too early if io_size > size. The exception is when
+	 * verify is enabled.
+	 */
+	if (td_rw(td) && !td_random(td) && td->o.verify == VERIFY_NONE)
 		total_bytes = max(total_bytes, (uint64_t)td->o.io_size);
 
 	/*
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 12/21] init: fixup verify_offset option
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (11 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 11/21] backend: fix verify issue during readwrite Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 13/21] verify: fix verify issue with offest modifiers Ankit Kumar
                     ` (9 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

Verify offset should swap verification header within the verify interval.
If this is not the case return error. Update the doc. accordingly.

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst |  3 ++-
 fio.1     |  3 ++-
 init.c    | 11 +++++++++++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index ca0044ce..4293c03c 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -3929,7 +3929,8 @@ Verification
 .. option:: verify_offset=int
 
 	Swap the verification header with data somewhere else in the block before
-	writing. It is swapped back before verifying.
+	writing. It is swapped back before verifying. This should be within the
+	range of :option:`verify_interval`.
 
 .. option:: verify_interval=int
 
diff --git a/fio.1 b/fio.1
index fd580258..a0f204c0 100644
--- a/fio.1
+++ b/fio.1
@@ -3654,7 +3654,8 @@ same offset with multiple outstanding I/Os.
 .TP
 .BI verify_offset \fR=\fPint
 Swap the verification header with data somewhere else in the block before
-writing. It is swapped back before verifying.
+writing. It is swapped back before verifying. This should be within the range
+of \fBverify_interval\fR.
 .TP
 .BI verify_interval \fR=\fPint
 Write the verification header at a finer granularity than the
diff --git a/init.c b/init.c
index 00a3a8c7..bf257ea1 100644
--- a/init.c
+++ b/init.c
@@ -873,6 +873,17 @@ static int fixup_options(struct thread_data *td)
 				o->verify_write_sequence = 0;
 		}
 
+		/*
+		 * Verify header should not be offset beyond the verify
+		 * interval.
+		 */
+		if (o->verify_offset + sizeof(struct verify_header) >
+		    o->verify_interval) {
+			log_err("fio: cannot offset verify header beyond the "
+				"verify interval.\n");
+			ret |= 1;
+		}
+
 		/*
 		 * Disable rand_seed check when we have verify_backlog,
 		 * zone reset frequency for zonemode=zbd, or norandommap.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 13/21] verify: fix verify issue with offest modifiers
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (12 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 12/21] init: fixup verify_offset option Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 14/21] verify: adjust fio_offset_overlap_risk to include randommap Ankit Kumar
                     ` (8 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar

Offset modifiers such as rw=readwrite:8 or rw=write:4K can create
overlaps. For these cases use RB tree instead of list to log the I/O
entries.

Add a helper function fio_offset_overlap_risk() to decide whether to log
the I/O entry in an RB tree or a list.

Disable header seed verification if there are offset modifiers, unless
its explicitly enabled.

fixes #1503

Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 HOWTO.rst | 4 +++-
 fio.1     | 4 +++-
 fio.h     | 8 ++++++++
 init.c    | 6 ++++--
 iolog.c   | 9 +++++----
 5 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/HOWTO.rst b/HOWTO.rst
index 4293c03c..bb5da1cc 100644
--- a/HOWTO.rst
+++ b/HOWTO.rst
@@ -1185,7 +1185,9 @@ I/O type
 	pattern, then the *<nr>* value specified will be **added** to the generated
 	offset for each I/O turning sequential I/O into sequential I/O with holes.
 	For instance, using ``rw=write:4k`` will skip 4k for every write.  Also see
-	the :option:`rw_sequencer` option.
+	the :option:`rw_sequencer` option. If this is used with :option:`verify`
+	then :option:`verify_header_seed` will be disabled, unless its explicitly
+	enabled.
 
 .. option:: rw_sequencer=str
 
diff --git a/fio.1 b/fio.1
index a0f204c0..4755ca19 100644
--- a/fio.1
+++ b/fio.1
@@ -955,7 +955,9 @@ modifier with a value of 8. If the suffix is used with a sequential I/O
 pattern, then the `<nr>' value specified will be added to the generated
 offset for each I/O turning sequential I/O into sequential I/O with holes.
 For instance, using `rw=write:4k' will skip 4k for every write. Also see
-the \fBrw_sequencer\fR option.
+the \fBrw_sequencer\fR option. If this is used with \fBverify\fR then
+\fBverify_header_seed\fR option will be disabled, unless its explicitly
+enabled.
 .RE
 .TP
 .BI rw_sequencer \fR=\fPstr
diff --git a/fio.h b/fio.h
index b8cf3229..c7c0d147 100644
--- a/fio.h
+++ b/fio.h
@@ -800,6 +800,14 @@ extern void lat_target_reset(struct thread_data *);
 	    	 (i) < (td)->o.nr_files && ((f) = (td)->files[i]) != NULL; \
 		 (i)++)
 
+static inline bool fio_offset_overlap_risk(struct thread_data *td)
+{
+	if (td->o.ddir_seq_add || (td->o.ddir_seq_nr > 1))
+		return true;
+
+	return false;
+}
+
 static inline bool fio_fill_issue_time(struct thread_data *td)
 {
 	if (td->o.read_iolog_file ||
diff --git a/init.c b/init.c
index bf257ea1..feaa3f28 100644
--- a/init.c
+++ b/init.c
@@ -886,11 +886,13 @@ static int fixup_options(struct thread_data *td)
 
 		/*
 		 * Disable rand_seed check when we have verify_backlog,
-		 * zone reset frequency for zonemode=zbd, or norandommap.
+		 * zone reset frequency for zonemode=zbd, norandommap, or
+		 * offset modifiers.
 		 * Unless we were explicitly asked to enable it.
 		 */
 		if (!td_write(td) || (td->flags & TD_F_VER_BACKLOG) ||
-		    o->zrf.u.f || o->norandommap) {
+		    o->zrf.u.f || o->norandommap ||
+		    fio_offset_overlap_risk(td)) {
 			if (!fio_option_is_set(o, verify_header_seed))
 				o->verify_header_seed = 0;
 		}
diff --git a/iolog.c b/iolog.c
index ef173b09..6b5dc9ab 100644
--- a/iolog.c
+++ b/iolog.c
@@ -301,11 +301,12 @@ void log_io_piece(struct thread_data *td, struct io_u *io_u)
 	}
 
 	/*
-	 * Only sort writes if we don't have a random map in which case we need
-	 * to check for duplicate blocks and drop the old one, which we rely on
-	 * the rb insert/lookup for handling.
+	 * Sort writes if we don't have a random map in which case we need to
+	 * check for duplicate blocks and drop the old one, which we rely on
+	 * the rb insert/lookup for handling. Sort writes if we have offset
+	 * modifier which can also create duplicate blocks.
 	 */
-	if (file_randommap(td, ipo->file)) {
+	if (file_randommap(td, ipo->file) && !fio_offset_overlap_risk(td)) {
 		INIT_FLIST_HEAD(&ipo->list);
 		flist_add_tail(&ipo->list, &td->io_hist_list);
 		ipo->flags |= IP_F_ONLIST;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 14/21] verify: adjust fio_offset_overlap_risk to include randommap
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (13 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 13/21] verify: fix verify issue with offest modifiers Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 15/21] t/fiotestcommon: do not require nvmecdev argument for Requirements Ankit Kumar
                     ` (7 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Ankit Kumar, Vincent Fu

Currently we are using a list to log I/O history if:
 * randommap is enabled and fio manages to allocate memory for it.
 * there are no offset modfiers with any jobs.
For any different scenario we use an RB tree to handle offset overlaps,
which disables header seed checks for them.

This commit expands fio_offset_overlap_risk() such that it covers
file_randommap() cases.
For random workload with this change these are the possible scenarios

 -----------------------------------------------------------------------
|                 |         norandommap=0              |  norandommap=1 |
|-----------------------------------------------------------------------|
| softrandommap=0 |        list (No change)            |    RB tree     |
|                 | (fio was able to allocate memory)  |  (No change)   |
|-----------------|------------------------------------|----------------|
|                 |      RB tree (Now always)          |    RB tree     |
| softrandommap=1 |Even if fio was able to allocate mem|  (No Change)   |
 -----------------------------------------------------------------------

With randommap enabled and softrandommap=1 we now always use an RB tree,
even when fio is able to allocate memory for random map. In this case
verify header seed check will be disabled. If users want to check header
seed they can either disable softrandommap or explicilty enable
verify_header_seed.

Effectively this changes randommap from being a per-file property to
per-job property.

This also fixes rand seed mismatch isues, that have been observed when
multiple files are used, such as for the below mentioned configuration.

[global]
do_verify=1
verify=md5
direct=1
[multi_file]
rw=readwrite
directory=.
nrfiles=2
size=32K

Here is the truncated log with debug=verify flag, and an extra log when
the seed gets generated as well as the mismatch.

verify   368109 file ./multi_file.0.1 seed 46386204153304124 offset=0, length=4096
verify   368109 file ./multi_file.0.0 seed 9852480210356360750 offset=0, length=4096
verify   368109 file ./multi_file.0.1 seed 4726550845720924880 offset=4096, length=4096
verify: bad header rand_seed 9852480210356360750, wanted 46386204153304124 at file ./multi_file.0.0 offset 0, length 4096 (requested block: offset=0, length=4096)

Earlier the I/O entries were getting logged in an RB tree, as we were
relying on file_randommap(), which was false for sequential workloads.
In RB tree, files are prioritized first and then the offset. Thus during
the verify phase the I/O entries are removed from tree in order of file
and then offset which is not how it was originally written. With the new
checks, for sequential workload we now store the entries in the list
instead of RB tree.
Even for sequential workload if the user fortuitously specified
norandommap or softrandommap, then I/Os will be stored in an RB tree.
However in this case header seed checks will be disabled.

fixes #740
fixes #746
fixes #844
fixes #1538

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 fio.h   | 3 ++-
 init.c  | 7 +++----
 iolog.c | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fio.h b/fio.h
index c7c0d147..d6423258 100644
--- a/fio.h
+++ b/fio.h
@@ -802,7 +802,8 @@ extern void lat_target_reset(struct thread_data *);
 
 static inline bool fio_offset_overlap_risk(struct thread_data *td)
 {
-	if (td->o.ddir_seq_add || (td->o.ddir_seq_nr > 1))
+	if (td->o.norandommap || td->o.softrandommap ||
+	    td->o.ddir_seq_add || (td->o.ddir_seq_nr > 1))
 		return true;
 
 	return false;
diff --git a/init.c b/init.c
index feaa3f28..95f2179d 100644
--- a/init.c
+++ b/init.c
@@ -886,13 +886,12 @@ static int fixup_options(struct thread_data *td)
 
 		/*
 		 * Disable rand_seed check when we have verify_backlog,
-		 * zone reset frequency for zonemode=zbd, norandommap, or
-		 * offset modifiers.
+		 * zone reset frequency for zonemode=zbd, or if we are using
+		 * an RB tree for IO history logs.
 		 * Unless we were explicitly asked to enable it.
 		 */
 		if (!td_write(td) || (td->flags & TD_F_VER_BACKLOG) ||
-		    o->zrf.u.f || o->norandommap ||
-		    fio_offset_overlap_risk(td)) {
+		    o->zrf.u.f || fio_offset_overlap_risk(td)) {
 			if (!fio_option_is_set(o, verify_header_seed))
 				o->verify_header_seed = 0;
 		}
diff --git a/iolog.c b/iolog.c
index 6b5dc9ab..dcf6083c 100644
--- a/iolog.c
+++ b/iolog.c
@@ -306,7 +306,7 @@ void log_io_piece(struct thread_data *td, struct io_u *io_u)
 	 * the rb insert/lookup for handling. Sort writes if we have offset
 	 * modifier which can also create duplicate blocks.
 	 */
-	if (file_randommap(td, ipo->file) && !fio_offset_overlap_risk(td)) {
+	if (!fio_offset_overlap_risk(td)) {
 		INIT_FLIST_HEAD(&ipo->list);
 		flist_add_tail(&ipo->list, &td->io_hist_list);
 		ipo->flags |= IP_F_ONLIST;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 15/21] t/fiotestcommon: do not require nvmecdev argument for Requirements
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (14 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 14/21] verify: adjust fio_offset_overlap_risk to include randommap Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 16/21] t/fiotestlib: improve JSON decoding Ankit Kumar
                     ` (6 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Vincent Fu

From: Vincent Fu <vincent.fu@samsung.com>

Enable Requirements checking for test suites that do not have an
nvmecdev argument. macOS does not support NUMA placement so we need to
skip some tests on that platform when the test suite does not have an
nvmecdev argument. This will be used in an upcoming patch for a set of
verify tests.

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
 t/fiotestcommon.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/fiotestcommon.py b/t/fiotestcommon.py
index f5012c82..6c146b66 100644
--- a/t/fiotestcommon.py
+++ b/t/fiotestcommon.py
@@ -101,7 +101,7 @@ class Requirements():
         Requirements._unittests = os.path.exists(unittest_path)
 
         Requirements._cpucount4 = multiprocessing.cpu_count() >= 4
-        Requirements._nvmecdev = args.nvmecdev
+        Requirements._nvmecdev = args.nvmecdev if hasattr(args, 'nvmecdev') else False
 
         req_list = [
                 Requirements.linux,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 16/21] t/fiotestlib: improve JSON decoding
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (15 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 15/21] t/fiotestcommon: do not require nvmecdev argument for Requirements Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 17/21] t/fiotestlib: display stderr size when it is not empty but should be Ankit Kumar
                     ` (5 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Vincent Fu

From: Vincent Fu <vincent.fu@samsung.com>

Sometimes error/informational messages appear at the end of the JSON
data. Try to parse as JSON only the text between the first { and the
last }.

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
 t/fiotestlib.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/t/fiotestlib.py b/t/fiotestlib.py
index 61adca14..916dc17f 100755
--- a/t/fiotestlib.py
+++ b/t/fiotestlib.py
@@ -260,12 +260,13 @@ class FioJobFileTest(FioExeTest):
             return
 
         #
-        # Sometimes fio informational messages are included at the top of the
-        # JSON output, especially under Windows. Try to decode output as JSON
-        # data, skipping everything until the first {
+        # Sometimes fio informational messages are included outside the JSON
+        # output, especially under Windows. Try to decode output as JSON data,
+        # skipping outside the first { and last }
         #
         lines = file_data.splitlines()
-        file_data = '\n'.join(lines[lines.index("{"):])
+        last = len(lines) - lines[::-1].index("}")
+        file_data = '\n'.join(lines[lines.index("{"):last])
         try:
             self.json_data = json.loads(file_data)
         except json.JSONDecodeError:
@@ -320,12 +321,13 @@ class FioJobCmdTest(FioExeTest):
             file_data = file.read()
 
         #
-        # Sometimes fio informational messages are included at the top of the
-        # JSON output, especially under Windows. Try to decode output as JSON
-        # data, skipping everything until the first {
+        # Sometimes fio informational messages are included outside the JSON
+        # output, especially under Windows. Try to decode output as JSON data,
+        # skipping outside the first { and last }
         #
         lines = file_data.splitlines()
-        file_data = '\n'.join(lines[lines.index("{"):])
+        last = len(lines) - lines[::-1].index("}")
+        file_data = '\n'.join(lines[lines.index("{"):last])
         try:
             self.json_data = json.loads(file_data)
         except json.JSONDecodeError:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 17/21] t/fiotestlib: display stderr size when it is not empty but should be
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (16 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 16/21] t/fiotestlib: improve JSON decoding Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 18/21] t/verify.py: Add verify test script Ankit Kumar
                     ` (4 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Vincent Fu

From: Vincent Fu <vincent.fu@samsung.com>

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
 t/fiotestlib.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/fiotestlib.py b/t/fiotestlib.py
index 916dc17f..913cb605 100755
--- a/t/fiotestlib.py
+++ b/t/fiotestlib.py
@@ -139,7 +139,7 @@ class FioExeTest(FioTest):
         if 'stderr_empty' in self.success:
             if self.success['stderr_empty']:
                 if stderr_size != 0:
-                    self.failure_reason = f"{self.failure_reason} stderr not empty,"
+                    self.failure_reason = f"{self.failure_reason} stderr not empty size {stderr_size},"
                     self.passed = False
             else:
                 if stderr_size == 0:
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 18/21] t/verify.py: Add verify test script
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (17 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 17/21] t/fiotestlib: display stderr size when it is not empty but should be Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 19/21] t/fiotestcommon: add a success pattern for long tests Ankit Kumar
                     ` (3 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Vincent Fu, Ankit Kumar

From: Vincent Fu <vincent.fu@samsung.com>

The script contains three sets of tests. The first set of tests
exercises fio's decision making about checking the verify header's
sequence number and random seed. The second set of tests are aimed at
making sure that the checksum functions can detect data mismatches. The
final set of tests exercise fio's verify-related options such as
verify_backlog and verify_inteval.

This test script includes two checksum lists. The first list (default)
contains a subset of the checksum methods offered by fio, whereas the
second list contains the full set of checksum methods. The second, full
set can be run by specifying -c or --complete. Testing all of the
checksum methods can take a long time.

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
Signed-off-by: Ankit Kumar <ankit.kumar@samsung.com>
---
 t/verify.py | 799 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 799 insertions(+)
 create mode 100755 t/verify.py

diff --git a/t/verify.py b/t/verify.py
new file mode 100755
index 00000000..38c07bd1
--- /dev/null
+++ b/t/verify.py
@@ -0,0 +1,799 @@
+#!/usr/bin/env python3
+"""
+# verify.py
+#
+# Test fio's verify options.
+#
+# USAGE
+# see python3 verify.py --help
+#
+# EXAMPLES
+# python3 t/verify.py
+# python3 t/verify.py --fio ./fio
+#
+# REQUIREMENTS
+# Python 3.6
+# - 4 CPUs
+#
+"""
+import os
+import sys
+import time
+import errno
+import logging
+import argparse
+import platform
+import itertools
+from pathlib import Path
+from fiotestlib import FioJobCmdTest, run_fio_tests
+from fiotestcommon import SUCCESS_DEFAULT, SUCCESS_NONZERO, Requirements
+
+
+VERIFY_OPT_LIST = [
+    'direct',
+    'iodepth',
+    'filesize',
+    'bs',
+    'time_based',
+    'runtime',
+    'io_size',
+    'offset',
+    'number_ios',
+    'output-format',
+    'directory',
+    'norandommap',
+    'numjobs',
+    'nrfiles',
+    'openfiles',
+    'cpus_allowed',
+    'experimental_verify',
+    'verify_backlog',
+    'verify_backlog_batch',
+    'verify_interval',
+    'verify_offset',
+    'verify_async',
+    'verify_async_cpus',
+    'verify_pattern',
+    'verify_only',
+]
+
+class VerifyTest(FioJobCmdTest):
+    """
+    Verify test class.
+    """
+
+    def setup(self, parameters):
+        """Setup a test."""
+
+        fio_args = [
+            "--name=verify",
+            f"--ioengine={self.fio_opts['ioengine']}",
+            f"--rw={self.fio_opts['rw']}",
+            f"--verify={self.fio_opts['verify']}",
+            f"--output={self.filenames['output']}",
+        ]
+        for opt in VERIFY_OPT_LIST:
+            if opt in self.fio_opts:
+                option = f"--{opt}={self.fio_opts[opt]}"
+                fio_args.append(option)
+
+        super().setup(fio_args)
+
+
+class VerifyCSUMTest(FioJobCmdTest):
+    """
+    Verify test class. Run standard verify jobs, modify the data, and then run
+    more verify jobs. Hopefully fio will detect that the data has chagned.
+    """
+
+    @staticmethod
+    def add_verify_opts(opt_list, adds):
+        """Add optional options."""
+
+        fio_opts = []
+
+        for opt in adds:
+            if opt in opt_list:
+                option = f"--{opt}={opt_list[opt]}"
+                fio_opts.append(option)
+
+        return fio_opts
+
+    def setup(self, parameters):
+        """Setup a test."""
+
+        logging.debug("ioengine is %s", self.fio_opts['ioengine'])
+        fio_args_base = [
+            "--filename=verify",
+            "--stonewall",
+            f"--ioengine={self.fio_opts['ioengine']}",
+        ]
+
+        extra_options = self.add_verify_opts(self.fio_opts, VERIFY_OPT_LIST)
+
+        verify_only = [
+            "--verify_only",
+            f"--rw={self.fio_opts['rw']}",
+            f"--verify={self.fio_opts['verify']}",
+        ] + fio_args_base + extra_options
+
+        verify_read = [
+            "--rw=randread" if 'rand' in self.fio_opts['rw'] else "--rw=read",
+            f"--verify={self.fio_opts['verify']}",
+        ] + fio_args_base + extra_options
+
+        layout = [
+            "--name=layout",
+            f"--rw={self.fio_opts['rw']}",
+            f"--verify={self.fio_opts['verify']}",
+        ] + fio_args_base + extra_options
+
+        success_only = ["--name=success_only"] + verify_only
+        success_read = ["--name=success_read"] + verify_read
+
+        mangle = [
+            "--name=mangle",
+            "--rw=randwrite",
+            "--randrepeat=0",
+            f"--bs={self.fio_opts['mangle_bs']}",
+            "--number_ios=1",
+        ] + fio_args_base + self.add_verify_opts(self.fio_opts, ['filesize'])
+
+        failure_only = ["--name=failure_only"] + verify_only
+        failure_read = ["--name=failure_read"] + verify_read
+
+        fio_args = layout + success_only + success_read + mangle + failure_only + failure_read + [f"--output={self.filenames['output']}"]
+        logging.debug("fio_args: %s", fio_args)
+
+        super().setup(fio_args)
+
+    def check_result(self):
+        super().check_result()
+
+        checked = {}
+
+        for job in self.json_data['jobs']:
+            if job['jobname'] == 'layout':
+                checked[job['jobname']] = True
+                if job['error']:
+                    self.passed = False
+                    self.failure_reason += " layout job failed"
+            elif 'success' in job['jobname']:
+                checked[job['jobname']] = True
+                if job['error']:
+                    self.passed = False
+                    self.failure_reason += f" verify pass {job['jobname']} that should have succeeded actually failed"
+            elif job['jobname'] == 'mangle':
+                checked[job['jobname']] = True
+                if job['error']:
+                    self.passed = False
+                    self.failure_reason += " mangle job failed"
+            elif 'failure' in job['jobname']:
+                checked[job['jobname']] = True
+                if self.fio_opts['verify'] == 'null' and not job['error']:
+                    continue
+                if job['error'] != errno.EILSEQ:
+                    self.passed = False
+                    self.failure_reason += f" verify job {job['jobname']} produced {job['error']} instead of errno {errno.EILSEQ} Illegal byte sequence"
+                    logging.debug(self.json_data)
+            else:
+                self.passed = False
+                self.failure_reason += " unknown job name"
+
+        if len(checked) != 6:
+            self.passed = False
+            self.failure_reason += " six phases not completed"
+
+        with open(self.filenames['stderr'], "r") as se:
+            contents = se.read()
+            logging.debug("stderr: %s", contents)
+
+
+#
+# These tests exercise fio's decisions about verifying the sequence number and
+# random seed in the verify header.
+#
+TEST_LIST_HEADER = [
+    {
+        # Basic test with options at default values
+        "test_id": 2000,
+        "fio_opts": {
+            "ioengine": "libaio",
+            "filesize": "1M",
+            "bs": 4096,
+            "output-format": "json",
+            },
+        "test_class": VerifyTest,
+        "success": SUCCESS_DEFAULT,
+    },
+    {
+        # Basic test with iodepth 16
+        "test_id": 2001,
+        "fio_opts": {
+            "ioengine": "libaio",
+            "filesize": "1M",
+            "bs": 4096,
+            "iodepth": 16,
+            "output-format": "json",
+            },
+        "test_class": VerifyTest,
+        "success": SUCCESS_DEFAULT,
+    },
+    {
+        # Basic test with 3 files
+        "test_id": 2002,
+        "fio_opts": {
+            "ioengine": "libaio",
+            "filesize": "1M",
+            "bs": 4096,
+            "nrfiles": 3,
+            "output-format": "json",
+            },
+        "test_class": VerifyTest,
+        "success": SUCCESS_DEFAULT,
+    },
+    {
+        # Basic test with iodepth 16 and 3 files
+        "test_id": 2003,
+        "fio_opts": {
+            "ioengine": "libaio",
+            "filesize": "1M",
+            "bs": 4096,
+            "iodepth": 16,
+            "nrfiles": 3,
+            "output-format": "json",
+            },
+        "test_class": VerifyTest,
+        "success": SUCCESS_DEFAULT,
+    },
+]
+
+#
+# These tests are mainly intended to assess the checksum functions. They write
+# out data, run some verify jobs, then modify the data, and try to verify the
+# data again, expecting to see failures.
+#
+TEST_LIST_CSUM = [
+    {
+        # basic seq write verify job
+        "test_id": 1000,
+        "fio_opts": {
+            "ioengine": "psync",
+            "filesize": "1M",
+            "bs": 4096,
+            "rw": "write",
+            "output-format": "json",
+            },
+        "test_class": VerifyCSUMTest,
+        "success": SUCCESS_NONZERO,
+    },
+    {
+        # basic rand write verify job
+        "test_id": 1001,
+        "fio_opts": {
+            "ioengine": "psync",
+            "filesize": "1M",
+            "bs": 4096,
+            "rw": "randwrite",
+            "output-format": "json",
+            },
+        "test_class": VerifyCSUMTest,
+        "success": SUCCESS_NONZERO,
+    },
+    {
+        # basic libaio seq write test
+        "test_id": 1002,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 16,
+            "filesize": "1M",
+            "bs": 4096,
+            "rw": "write",
+            "output-format": "json",
+            },
+        "test_class": VerifyCSUMTest,
+        "success": SUCCESS_NONZERO,
+    },
+    {
+        # basic libaio rand write test
+        "test_id": 1003,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 16,
+            "filesize": "1M",
+            "bs": 4096,
+            "rw": "randwrite",
+            "output-format": "json",
+            },
+        "test_class": VerifyCSUMTest,
+        "success": SUCCESS_NONZERO,
+    },
+]
+
+#
+# These tests are run for all combinations of data direction and checksum
+# methods.
+#
+TEST_LIST = [
+    {
+        # norandommap with verify backlog
+        "test_id": 1,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 32,
+            "filesize": "2M",
+            "norandommap": 1,
+            "bs": 512,
+            "time_based": 1,
+            "runtime": 3,
+            "verify_backlog": 128,
+            "verify_backlog_batch": 64,
+            },
+        "test_class": VerifyTest,
+    },
+    {
+        # norandommap with verify offset and interval
+        "test_id": 2,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 32,
+            "filesize": "2M",
+            "io_size": "4M",
+            "norandommap": 1,
+            "bs": 4096,
+            "verify_interval": 2048,
+            "verify_offset": 1024,
+            },
+        "test_class": VerifyTest,
+    },
+    {
+        # norandommap with verify offload to async threads
+        "test_id": 3,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 32,
+            "filesize": "2M",
+            "norandommap": 1,
+            "bs": 4096,
+            "cpus_allowed": "0-3",
+            "verify_async": 2,
+            "verify_async_cpus": "0-1",
+            },
+        "test_class": VerifyTest,
+        "requirements":     [Requirements.not_macos,
+                             Requirements.cpucount4],
+        # mac os does not support CPU affinity
+    },
+    {
+        # tausworthe combine all verify options
+        "test_id": 4,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 32,
+            "filesize": "4M",
+            "bs": 4096,
+            "cpus_allowed": "0-3",
+            "time_based": 1,
+            "random_generator": "tausworthe",
+            "runtime": 3,
+            "verify_interval": 2048,
+            "verify_offset": 1024,
+            "verify_backlog": 128,
+            "verify_backlog_batch": 128,
+            "verify_async": 2,
+            "verify_async_cpus": "0-1",
+            },
+        "test_class": VerifyTest,
+        "requirements":     [Requirements.not_macos,
+                             Requirements.cpucount4],
+        # mac os does not support CPU affinity
+    },
+    {
+        # norandommap combine all verify options
+        "test_id": 5,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 32,
+            "filesize": "4M",
+            "norandommap": 1,
+            "bs": 4096,
+            "cpus_allowed": "0-3",
+            "time_based": 1,
+            "runtime": 3,
+            "verify_interval": 2048,
+            "verify_offset": 1024,
+            "verify_backlog": 128,
+            "verify_backlog_batch": 128,
+            "verify_async": 2,
+            "verify_async_cpus": "0-1",
+            },
+        "test_class": VerifyTest,
+        "requirements":     [Requirements.not_macos,
+                             Requirements.cpucount4],
+        # mac os does not support CPU affinity
+    },
+    {
+        # multiple jobs and files with verify
+        "test_id": 6,
+        "fio_opts": {
+            "direct": 1,
+            "ioengine": "libaio",
+            "iodepth": 32,
+            "filesize": "512K",
+            "nrfiles": 3,
+            "openfiles": 2,
+            "numjobs": 2,
+            "norandommap": 1,
+            "bs": 4096,
+            "time_based": 1,
+            "runtime": 20,
+            "verify_interval": 2048,
+            "verify_offset": 1024,
+            "verify_backlog": 16,
+            "verify_backlog_batch": 16,
+            },
+        "test_class": VerifyTest,
+    },
+]
+
+
+def parse_args():
+    """Parse command-line arguments."""
+
+    parser = argparse.ArgumentParser()
+    parser.add_argument('-r', '--fio-root', help='fio root path')
+    parser.add_argument('-d', '--debug', help='Enable debug messages', action='store_true')
+    parser.add_argument('-f', '--fio', help='path to file executable (e.g., ./fio)')
+    parser.add_argument('-a', '--artifact-root', help='artifact root directory')
+    parser.add_argument('-c', '--complete', help='Enable all checksums', action='store_true')
+    parser.add_argument('-s', '--skip', nargs='+', type=int,
+                        help='list of test(s) to skip')
+    parser.add_argument('-o', '--run-only', nargs='+', type=int,
+                        help='list of test(s) to run, skipping all others')
+    parser.add_argument('-k', '--skip-req', action='store_true',
+                        help='skip requirements checking')
+    parser.add_argument('--csum', nargs='+', type=str,
+                        help='list of checksum methods to use, skipping all others')
+    args = parser.parse_args()
+
+    return args
+
+
+def verify_test_header(test_env, args, csum, mode, sequence):
+    """
+    Adjust test arguments based on values of mode and sequence. Then run the
+    tests. This function is intended to run a set of tests that test
+    conditions under which the header random seed and sequence number are
+    checked.
+
+    The result should be a matrix with these combinations:
+        {write, write w/verify_only, read/write, read/write w/verify_only, read} x
+        {sequential, random w/randommap, random w/norandommap, sequence modifiers}
+    """
+    for test in TEST_LIST_HEADER:
+        # experimental_verify does not work in verify_only=1 mode
+        if "_vo" in mode and 'experimental_verify' in test['fio_opts'] and \
+        test['fio_opts']['experimental_verify']:
+            test['force_skip'] = True
+        else:
+            test['force_skip'] = False
+
+        test['fio_opts']['verify'] = csum
+        if csum == 'pattern':
+            test['fio_opts']['verify_pattern'] = '"abcd"-120xdeadface'
+        else:
+            test['fio_opts'].pop('verify_pattern', None)
+
+        if 'norandommap' in sequence:
+            test['fio_opts']['norandommap'] = 1
+        else:
+            test['fio_opts']['norandommap'] = 0
+
+        if 'randommap' in sequence:
+            prefix = "rand"
+        else:
+            prefix = ""
+
+        if 'sequence_modifier' in sequence:
+            suffix = ":4096"
+        else:
+            suffix = ""
+
+        if 'readwrite' in mode:
+            fio_ddir = 'rw'
+        elif 'write' in mode:
+            fio_ddir = 'write'
+        elif 'read' in mode:
+            fio_ddir = 'read'
+        else:
+            fio_ddir = ""
+            # TODO throw an exception here
+        test['fio_opts']['rw'] = prefix + fio_ddir + suffix
+        logging.debug("ddir is %s", test['fio_opts']['rw'])
+
+        if '_vo' in mode:
+            vo = 1
+        else:
+            vo = 0
+        test['fio_opts']['verify_only'] = vo
+
+        # For 100% read workloads we need to read a file that was written with
+        # verify enabled. Use a previous test case for this by pointing fio to
+        # write to a file in a specific directory.
+        #
+        # For verify_only tests we also need to point fio to a file that was
+        # written with verify enabled
+        #
+        # Finally, we have observed on Windows platforms that fio may not
+        # create files of the size specified by filesize. This causes false
+        # positive test failures when we later try to run 100% read verify jobs
+        # on the files as the verify job may lay out a file of the size
+        # specified by filesize and overwrite the previously written file,
+        # inducing verificaiton failures. Until this issue is resolved, just
+        # skip these tests on Windows.
+        # https://github.com/axboe/fio/issues/1872
+        if mode == 'read':
+            directory = os.path.join(test_env['artifact_root'].replace(f'mode_{mode}','mode_write'),
+                        f"{test['test_id']:04d}")
+            test['fio_opts']['directory'] = str(Path(directory).absolute()) if \
+                platform.system() != "Windows" else str(Path(directory).absolute()).replace(':', '\\:')
+            if platform.system() == "Windows":
+                test['force_skip'] = True
+        elif vo:
+            directory = os.path.join(test_env['artifact_root'].replace('write_vo','write'),
+                        f"{test['test_id']:04d}")
+            test['fio_opts']['directory'] = str(Path(directory).absolute()) if \
+                platform.system() != "Windows" else str(Path(directory).absolute()).replace(':', '\\:')
+            if platform.system() == "Windows":
+                test['force_skip'] = True
+        else:
+            test['fio_opts'].pop('directory', None)
+
+    return run_fio_tests(TEST_LIST_HEADER, test_env, args)
+
+
+MANGLE_JOB_BS = 0
+def verify_test_csum(test_env, args, mbs, csum):
+    """
+    Adjust test arguments based on values of csum. Then run the tests.
+    This function is designed for a series of tests that check that checksum
+    methods can reliably detect data integrity issues.
+    """
+    for test in TEST_LIST_CSUM:
+        test['force_skip'] = False
+        test['fio_opts']['verify'] = csum
+
+        if csum == 'pattern':
+            test['fio_opts']['verify_pattern'] = '"abcd"-120xdeadface'
+        else:
+            test['fio_opts'].pop('verify_pattern', None)
+
+        if mbs == MANGLE_JOB_BS:
+            test['fio_opts']['mangle_bs'] = test['fio_opts']['bs']
+        else:
+            test['fio_opts']['mangle_bs'] = mbs
+
+        # These tests produce verification failures but not when verify=null,
+        # so adjust the success criterion.
+        if csum == 'null':
+            test['success'] = SUCCESS_DEFAULT
+        else:
+            test['success'] = SUCCESS_NONZERO
+
+    return run_fio_tests(TEST_LIST_CSUM, test_env, args)
+
+
+def verify_test(test_env, args, ddir, csum):
+    """
+    Adjust test arguments based on values of ddir and csum.  Then run
+    the tests.
+    """
+    for test in TEST_LIST:
+        test['force_skip'] = False
+
+        test['fio_opts']['rw'] = ddir
+        test['fio_opts']['verify'] = csum
+
+        if csum == 'pattern':
+            test['fio_opts']['verify_pattern'] = '"abcd"-120xdeadface'
+        else:
+            test['fio_opts'].pop('verify_pattern', None)
+
+        # For 100% read data directions we need the write file that was written with
+        # verify enabled. Use a previous test case for this by telling fio to
+        # write to a file in a specific directory.
+        if ddir == 'read':
+            directory = os.path.join(test_env['artifact_root'].replace(f'ddir_{ddir}','ddir_write'),
+                        f"{test['test_id']:04d}")
+            test['fio_opts']['directory'] = str(Path(directory).absolute()) if \
+                platform.system() != "Windows" else str(Path(directory).absolute()).replace(':', '\\:')
+        elif ddir == 'randread':
+            directory = os.path.join(test_env['artifact_root'].replace(f'ddir_{ddir}','ddir_randwrite'),
+                        f"{test['test_id']:04d}")
+            test['fio_opts']['directory'] = str(Path(directory).absolute()) if \
+                platform.system() != "Windows" else str(Path(directory).absolute()).replace(':', '\\:')
+        else:
+            test['fio_opts'].pop('directory', None)
+
+        # On Windows we are skipping the tests below because of the filesize
+        # issue noted above.
+        if ddir in [ 'read', 'randread' ] and platform.system() == 'Windows':
+            test['force_skip'] = True
+
+    return run_fio_tests(TEST_LIST, test_env, args)
+
+
+# 100% read workloads below must follow write workloads so that the 100% read
+# workloads will be reading data written with verification enabled.
+DDIR_LIST = [
+        'write',
+        'readwrite',
+        'read',
+        'randwrite',
+        'randrw',
+        'randread',
+             ]
+CSUM_LIST1 = [
+        'md5',
+        'crc64',
+        'pattern',
+             ]
+CSUM_LIST2 = [
+        'md5',
+        'crc64',
+        'crc32c',
+        'crc32c-intel',
+        'crc16',
+        'crc7',
+        'xxhash',
+        'sha512',
+        'sha256',
+        'sha1',
+        'sha3-224',
+        'sha3-384',
+        'sha3-512',
+        'pattern',
+        'null',
+             ]
+
+def main():
+    """
+    Run tests for fio's verify feature.
+    """
+
+    args = parse_args()
+
+    if args.debug:
+        logging.basicConfig(level=logging.DEBUG)
+    else:
+        logging.basicConfig(level=logging.INFO)
+
+    artifact_root = args.artifact_root if args.artifact_root else \
+        f"verify-test-{time.strftime('%Y%m%d-%H%M%S')}"
+    os.mkdir(artifact_root)
+    print(f"Artifact directory is {artifact_root}")
+
+    if args.fio:
+        fio_path = str(Path(args.fio).absolute())
+    else:
+        fio_path = os.path.join(os.path.dirname(__file__), '../fio')
+    print(f"fio path is {fio_path}")
+
+    if args.fio_root:
+        fio_root = args.fio_root
+    else:
+        fio_root = str(Path(__file__).absolute().parent.parent)
+    print(f"fio root is {fio_root}")
+
+    if not args.skip_req:
+        Requirements(fio_root, args)
+
+    test_env = {
+              'fio_path': fio_path,
+              'fio_root': str(Path(__file__).absolute().parent.parent),
+              'artifact_root': artifact_root,
+              'basename': 'verify',
+              }
+
+    if platform.system() == 'Linux':
+        aio = 'libaio'
+        sync = 'psync'
+    elif platform.system() == 'Windows':
+        aio = 'windowsaio'
+        sync = 'sync'
+    else:
+        aio = 'posixaio'
+        sync = 'psync'
+    for test in TEST_LIST:
+        if 'aio' in test['fio_opts']['ioengine']:
+            test['fio_opts']['ioengine'] = aio
+        if 'sync' in test['fio_opts']['ioengine']:
+            test['fio_opts']['ioengine'] = sync
+    for test in TEST_LIST_CSUM:
+        if 'aio' in test['fio_opts']['ioengine']:
+            test['fio_opts']['ioengine'] = aio
+        if 'sync' in test['fio_opts']['ioengine']:
+            test['fio_opts']['ioengine'] = sync
+    for test in TEST_LIST_HEADER:
+        if 'aio' in test['fio_opts']['ioengine']:
+            test['fio_opts']['ioengine'] = aio
+        if 'sync' in test['fio_opts']['ioengine']:
+            test['fio_opts']['ioengine'] = sync
+
+    total = { 'passed':  0, 'failed': 0, 'skipped': 0 }
+
+    if args.complete:
+        csum_list = CSUM_LIST2
+    else:
+        csum_list = CSUM_LIST1
+
+    if args.csum:
+        csum_list = args.csum
+
+    try:
+        for ddir, csum in itertools.product(DDIR_LIST, csum_list):
+            print(f"\nddir: {ddir}, checksum: {csum}")
+
+            test_env['artifact_root'] = os.path.join(artifact_root,
+                                                     f"ddir_{ddir}_csum_{csum}")
+            os.mkdir(test_env['artifact_root'])
+
+            passed, failed, skipped = verify_test(test_env, args, ddir, csum)
+
+            total['passed'] += passed
+            total['failed'] += failed
+            total['skipped'] += skipped
+
+        # MANGLE_JOB_BS means to mangle an entire block which should result in
+        #  a header magic number error
+        # 4 means to mangle 4 bytes which should result in a checksum error
+        #  unless the 4 bytes occur in the verification header
+        mangle_bs = [MANGLE_JOB_BS, 4]
+        for mbs, csum in itertools.product(mangle_bs, csum_list):
+            print(f"\nmangle block size: {mbs}, checksum: {csum}")
+
+            test_env['artifact_root'] = os.path.join(artifact_root,
+                                                     f"mbs_{mbs}_csum_{csum}")
+            os.mkdir(test_env['artifact_root'])
+
+            passed, failed, skipped = verify_test_csum(test_env, args, mbs, csum)
+
+            total['passed'] += passed
+            total['failed'] += failed
+            total['skipped'] += skipped
+
+        # The loop below tests combinations of options that exercise fio's
+        # decisions about disabling checks for the sequence number and random
+        # seed in the verify header.
+        mode_list = [ "write", "write_vo", "readwrite", "readwrite_vo", "read" ]
+        sequence_list = [ "sequential", "randommap", "norandommap", "sequence_modifier" ]
+        for mode, sequence in itertools.product(mode_list, sequence_list):
+            print(f"\nmode: {mode}, sequence: {sequence}")
+
+            test_env['artifact_root'] = os.path.join(artifact_root,
+                                                     f"mode_{mode}_seq_{sequence}")
+            os.mkdir(test_env['artifact_root'])
+
+            passed, failed, skipped = verify_test_header(test_env, args, 'md5', mode, sequence)
+
+            total['passed'] += passed
+            total['failed'] += failed
+            total['skipped'] += skipped
+
+    except KeyboardInterrupt:
+        pass
+
+    print(f"\n\n{total['passed']} test(s) passed, {total['failed']} failed, " \
+            f"{total['skipped']} skipped")
+    sys.exit(total['failed'])
+
+
+if __name__ == '__main__':
+    main()
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 19/21] t/fiotestcommon: add a success pattern for long tests
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (18 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 18/21] t/verify.py: Add verify test script Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 20/21] t/run-fio-test: add t/verify.py Ankit Kumar
                     ` (2 subsequent siblings)
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Vincent Fu

From: Vincent Fu <vincent.fu@samsung.com>

On Windows the verify test script runs for longer than 10 minutes. Add a
success pattern that accommodates this test.

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
 t/fiotestcommon.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/t/fiotestcommon.py b/t/fiotestcommon.py
index 6c146b66..9003b4c1 100644
--- a/t/fiotestcommon.py
+++ b/t/fiotestcommon.py
@@ -19,6 +19,11 @@ SUCCESS_DEFAULT = {
     'stderr_empty': True,
     'timeout': 600,
     }
+SUCCESS_LONG = {
+    'zero_return': True,
+    'stderr_empty': True,
+    'timeout': 1800,
+    }
 SUCCESS_NONZERO = {
     'zero_return': False,
     'stderr_empty': False,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 20/21] t/run-fio-test: add t/verify.py
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (19 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 19/21] t/fiotestcommon: add a success pattern for long tests Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 10:47   ` [PATCH 21/21] ci: add nightly test for verify Ankit Kumar
  2025-02-27 16:54   ` [PATCH 00/21] verify fixes and a new test suite Vincent Fu
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Vincent Fu

From: Vincent Fu <vincent.fu@samsung.com>

Add the verify test script to our test runner.

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
 t/run-fio-tests.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/run-fio-tests.py b/t/run-fio-tests.py
index 101e95f7..7ceda067 100755
--- a/t/run-fio-tests.py
+++ b/t/run-fio-tests.py
@@ -1107,6 +1107,14 @@ TEST_LIST = [
         'success':          SUCCESS_DEFAULT,
         'requirements':     [Requirements.linux],
     },
+    {
+        'test_id':          1017,
+        'test_class':       FioExeTest,
+        'exe':              't/verify.py',
+        'parameters':       ['-f', '{fio_path}'],
+        'success':          SUCCESS_LONG,
+        'requirements':     [],
+    },
 ]
 
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 21/21] ci: add nightly test for verify
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (20 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 20/21] t/run-fio-test: add t/verify.py Ankit Kumar
@ 2025-02-27 10:47   ` Ankit Kumar
  2025-02-27 16:54   ` [PATCH 00/21] verify fixes and a new test suite Vincent Fu
  22 siblings, 0 replies; 26+ messages in thread
From: Ankit Kumar @ 2025-02-27 10:47 UTC (permalink / raw)
  To: axboe, vincentfu; +Cc: fio, Vincent Fu

From: Vincent Fu <vincent.fu@samsung.com>

Signed-off-by: Vincent Fu <vincent.fu@samsung.com>
---
 .github/workflows/ci.yml |  2 ++
 ci/actions-full-test.sh  | 13 +++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index d20a2d01..556ebb41 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -4,6 +4,8 @@ on:
   push:
   pull_request:
   workflow_dispatch:
+  schedule:
+    - cron: "35 0 * * *"  # 0:35 UTC which is 19:35 ET
 
 jobs:
   build-containers:
diff --git a/ci/actions-full-test.sh b/ci/actions-full-test.sh
index 23bdd219..854788c1 100755
--- a/ci/actions-full-test.sh
+++ b/ci/actions-full-test.sh
@@ -33,6 +33,19 @@ main() {
 
     fi
 
+    # If we are running a nightly test just run the verify tests.
+    # Otherwise skip the verify test script because it takes so long.
+    if [ "${GITHUB_EVENT_NAME}" == "schedule" ]; then
+	args+=(
+	    --run-only
+	    1017
+	)
+    else
+	skip+=(
+	    1017
+	)
+    fi
+
     echo python3 t/run-fio-tests.py --skip "${skip[@]}" "${args[@]}"
     python3 t/run-fio-tests.py --skip "${skip[@]}" "${args[@]}"
     make -C doc html
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/21] verify fixes and a new test suite
  2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
                     ` (21 preceding siblings ...)
  2025-02-27 10:47   ` [PATCH 21/21] ci: add nightly test for verify Ankit Kumar
@ 2025-02-27 16:54   ` Vincent Fu
  2025-03-05 12:36     ` Shinichiro Kawasaki
  22 siblings, 1 reply; 26+ messages in thread
From: Vincent Fu @ 2025-02-27 16:54 UTC (permalink / raw)
  To: Ankit Kumar, axboe; +Cc: fio, Shin'ichiro Kawasaki, john.g.garry

On 2/27/25 5:47 AM, Ankit Kumar wrote:
> This series fixes few issues with verify, and introduces a new test
> suite.
> 
> Inital few patches add the missing client/server support for
> verify_write_sequence. This also changes its behavior such that if
> verify_write_sequence is explicitly enabled fio will not disable it
> under any circumstance.
> 
> Numerous header seed mismatch issues have been reported. This series
> introduces a new option verify_header_seed which is similar to
> verify_write_sequence, which allow users to disable any header seed
> verification. For certain workloads which used to overwrite header seed
> before verification, we simply disable the header seed checks now. This
> now includes a few more scenarios such as verify_only mode, read only
> workloads and workloads with norandommap, where the header seed match is
> not guaranteed.
> 
> Few more fixes related to verify_offset, workloads that have offset
> modifiers, verification issue when multiple files are specified, are part
> of this series.
> 
> Lastly this includes robust test suite for verify.
> CI run result: https://github.com/vincentkfu/fio/actions/runs/13552248490
> 
> Note: The fixes in this series doesn't cover experimental_verify and any
> workload with variable block sizes.
> 

I have worked closely with Ankit on these patches and hope others can 
take a look to point out anything we may have missed.

Vincent

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/21] verify fixes and a new test suite
  2025-02-27 16:54   ` [PATCH 00/21] verify fixes and a new test suite Vincent Fu
@ 2025-03-05 12:36     ` Shinichiro Kawasaki
  2025-03-06 19:13       ` Vincent Fu
  0 siblings, 1 reply; 26+ messages in thread
From: Shinichiro Kawasaki @ 2025-03-05 12:36 UTC (permalink / raw)
  To: Vincent Fu
  Cc: Ankit Kumar, axboe@kernel.dk, fio@vger.kernel.org,
	john.g.garry@oracle.com

On Feb 27, 2025 / 11:54, Vincent Fu wrote:
> On 2/27/25 5:47 AM, Ankit Kumar wrote:
> > This series fixes few issues with verify, and introduces a new test
> > suite.
> > 
> > Inital few patches add the missing client/server support for
> > verify_write_sequence. This also changes its behavior such that if
> > verify_write_sequence is explicitly enabled fio will not disable it
> > under any circumstance.
> > 
> > Numerous header seed mismatch issues have been reported. This series
> > introduces a new option verify_header_seed which is similar to
> > verify_write_sequence, which allow users to disable any header seed
> > verification. For certain workloads which used to overwrite header seed
> > before verification, we simply disable the header seed checks now. This
> > now includes a few more scenarios such as verify_only mode, read only
> > workloads and workloads with norandommap, where the header seed match is
> > not guaranteed.
> > 
> > Few more fixes related to verify_offset, workloads that have offset
> > modifiers, verification issue when multiple files are specified, are part
> > of this series.
> > 
> > Lastly this includes robust test suite for verify.
> > CI run result: https://github.com/vincentkfu/fio/actions/runs/13552248490
> > 
> > Note: The fixes in this series doesn't cover experimental_verify and any
> > workload with variable block sizes.
> > 
> 
> I have worked closely with Ankit on these patches and hope others can take a
> look to point out anything we may have missed.

Hi Ankit, Vincent, thanks for the work and the heads up.

I ran t/zbd/run-tests-against-nullb with this series, and observed no failure.
So it looks good from zonemode=zbd testing point of view.

I took a glance on the changes and did not find anything wrong. It's ok for me
to apply the changes.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 00/21] verify fixes and a new test suite
  2025-03-05 12:36     ` Shinichiro Kawasaki
@ 2025-03-06 19:13       ` Vincent Fu
  0 siblings, 0 replies; 26+ messages in thread
From: Vincent Fu @ 2025-03-06 19:13 UTC (permalink / raw)
  To: Shinichiro Kawasaki
  Cc: Ankit Kumar, axboe@kernel.dk, fio@vger.kernel.org,
	john.g.garry@oracle.com

On 3/5/25 7:36 AM, Shinichiro Kawasaki wrote:
> On Feb 27, 2025 / 11:54, Vincent Fu wrote:
>> On 2/27/25 5:47 AM, Ankit Kumar wrote:
>>> This series fixes few issues with verify, and introduces a new test
>>> suite.
>>>
>>> Inital few patches add the missing client/server support for
>>> verify_write_sequence. This also changes its behavior such that if
>>> verify_write_sequence is explicitly enabled fio will not disable it
>>> under any circumstance.
>>>
>>> Numerous header seed mismatch issues have been reported. This series
>>> introduces a new option verify_header_seed which is similar to
>>> verify_write_sequence, which allow users to disable any header seed
>>> verification. For certain workloads which used to overwrite header seed
>>> before verification, we simply disable the header seed checks now. This
>>> now includes a few more scenarios such as verify_only mode, read only
>>> workloads and workloads with norandommap, where the header seed match is
>>> not guaranteed.
>>>
>>> Few more fixes related to verify_offset, workloads that have offset
>>> modifiers, verification issue when multiple files are specified, are part
>>> of this series.
>>>
>>> Lastly this includes robust test suite for verify.
>>> CI run result: https://github.com/vincentkfu/fio/actions/runs/13552248490
>>>
>>> Note: The fixes in this series doesn't cover experimental_verify and any
>>> workload with variable block sizes.
>>>
>>
>> I have worked closely with Ankit on these patches and hope others can take a
>> look to point out anything we may have missed.
> 
> Hi Ankit, Vincent, thanks for the work and the heads up.
> 
> I ran t/zbd/run-tests-against-nullb with this series, and observed no failure.
> So it looks good from zonemode=zbd testing point of view.
> 
> I took a glance on the changes and did not find anything wrong. It's ok for me
> to apply the changes.

I have applied the patches with small changes to the test script to skip 
a flaky test on macOS and fix a test that did not run reliably on Windows.

Shinichiro, thank you for testing and taking a look at the patches.

Vincent

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2025-03-06 19:13 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20250227053350epcas5p31b3b5b47590d70804dade59842d87b27@epcas5p3.samsung.com>
2025-02-27 10:47 ` [PATCH 00/21] verify fixes and a new test suite Ankit Kumar
2025-02-27  7:28   ` fiotestbot
2025-02-27 10:47   ` [PATCH 01/21] filesetup: remove unnecessary check Ankit Kumar
2025-02-27 10:47   ` [PATCH 02/21] verify: add missing client/server support for verify_write_sequence Ankit Kumar
2025-02-27 10:47   ` [PATCH 03/21] init: write sequence behavior change for verify_only mode Ankit Kumar
2025-02-27 10:47   ` [PATCH 04/21] fio: add verify_header_seed option Ankit Kumar
2025-02-27 10:47   ` [PATCH 05/21] verify: disable header seed checking instead of overwriting it Ankit Kumar
2025-02-27 10:47   ` [PATCH 06/21] verify: enable header seed check for 100% write jobs Ankit Kumar
2025-02-27 10:47   ` [PATCH 07/21] verify: disable header seed check for verify_only jobs Ankit Kumar
2025-02-27 10:47   ` [PATCH 08/21] verify: header seed check for read only workloads Ankit Kumar
2025-02-27 10:47   ` [PATCH 09/21] verify: fix verify issues with norandommap Ankit Kumar
2025-02-27 10:47   ` [PATCH 10/21] verify: disable write sequence checks with norandommap and iodepth > 1 Ankit Kumar
2025-02-27 10:47   ` [PATCH 11/21] backend: fix verify issue during readwrite Ankit Kumar
2025-02-27 10:47   ` [PATCH 12/21] init: fixup verify_offset option Ankit Kumar
2025-02-27 10:47   ` [PATCH 13/21] verify: fix verify issue with offest modifiers Ankit Kumar
2025-02-27 10:47   ` [PATCH 14/21] verify: adjust fio_offset_overlap_risk to include randommap Ankit Kumar
2025-02-27 10:47   ` [PATCH 15/21] t/fiotestcommon: do not require nvmecdev argument for Requirements Ankit Kumar
2025-02-27 10:47   ` [PATCH 16/21] t/fiotestlib: improve JSON decoding Ankit Kumar
2025-02-27 10:47   ` [PATCH 17/21] t/fiotestlib: display stderr size when it is not empty but should be Ankit Kumar
2025-02-27 10:47   ` [PATCH 18/21] t/verify.py: Add verify test script Ankit Kumar
2025-02-27 10:47   ` [PATCH 19/21] t/fiotestcommon: add a success pattern for long tests Ankit Kumar
2025-02-27 10:47   ` [PATCH 20/21] t/run-fio-test: add t/verify.py Ankit Kumar
2025-02-27 10:47   ` [PATCH 21/21] ci: add nightly test for verify Ankit Kumar
2025-02-27 16:54   ` [PATCH 00/21] verify fixes and a new test suite Vincent Fu
2025-03-05 12:36     ` Shinichiro Kawasaki
2025-03-06 19:13       ` Vincent Fu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox