[PATCH] Adds check for numberio during verify phase.

[PATCH] Adds check for numberio during verify phase.

[Juan Casse]

Currently, fio checks the block offset number in a block's header during
the verify phase.
We add a check for the io number (numberio) to detect stale blocks. This
check is performed only on workloads that write data, as those workloads
know what numberio was written to each block.
td->io_issues[ddir] = 0; was removed so that numberio does not get reset
at each iteration; we want numberio to keep incrementing to reflect
how many times the same data was written.

Signed-off-by: Juan Casse <jcasse@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>

Fixed typo.
---
 ioengine.h |  1 +
 iolog.c    |  1 +
 iolog.h    |  1 +
 libfio.c   |  1 -
 verify.c   | 14 +++++++++++++-
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/ioengine.h b/ioengine.h
index 31662eb..812febd 100644
--- a/ioengine.h
+++ b/ioengine.h
@@ -50,6 +50,7 @@ struct io_u {
 	 */
 	unsigned long buflen;
 	unsigned long long offset;
+	unsigned short numberio;
 	void *buf;
 
 	/*
diff --git a/iolog.c b/iolog.c
index 9bcf0d8..6459d3b 100644
--- a/iolog.c
+++ b/iolog.c
@@ -188,6 +188,7 @@ void log_io_piece(struct thread_data *td, struct io_u *io_u)
 	ipo->file = io_u->file;
 	ipo->offset = io_u->offset;
 	ipo->len = io_u->buflen;
+	ipo->numberio = io_u->numberio;
 
 	if (io_u_should_trim(td, io_u)) {
 		flist_add_tail(&ipo->trim_list, &td->trim_list);
diff --git a/iolog.h b/iolog.h
index 8fedc19..94e0fc1 100644
--- a/iolog.h
+++ b/iolog.h
@@ -78,6 +78,7 @@ struct io_piece {
 		struct fio_file *file;
 	};
 	unsigned long long offset;
+	unsigned short numberio;
 	unsigned long len;
 	unsigned int flags;
 	enum fio_ddir ddir;
diff --git a/libfio.c b/libfio.c
index c26d6a3..6e290f4 100644
--- a/libfio.c
+++ b/libfio.c
@@ -83,7 +83,6 @@ static void reset_io_counters(struct thread_data *td)
 		td->this_io_blocks[ddir] = 0;
 		td->rate_bytes[ddir] = 0;
 		td->rate_blocks[ddir] = 0;
-		td->io_issues[ddir] = 0;
 	}
 	td->zone_bytes = 0;
 
diff --git a/verify.c b/verify.c
index 9e88d61..63def12 100644
--- a/verify.c
+++ b/verify.c
@@ -369,6 +369,15 @@ static int verify_io_u_meta(struct verify_header *hdr, struct vcont *vc)
 	if (td->o.verify_pattern_bytes)
 		ret |= verify_io_u_pattern(hdr, vc);
 
+	/*
+	 * For read-only workloads, the program cannot be certain of the
+	 * last numberio written to a block. Checking of numberio will be done
+	 * only for workloads that write data.
+	 */
+	if (td_write(td) || td_rw(td))
+		if (vh->numberio != io_u->numberio)
+			ret = EILSEQ;
+
 	if (!ret)
 		return 0;
 
@@ -768,7 +777,7 @@ static void fill_meta(struct verify_header *hdr, struct thread_data *td,
 	vh->time_sec = io_u->start_time.tv_sec;
 	vh->time_usec = io_u->start_time.tv_usec;
 
-	vh->numberio = td->io_issues[DDIR_WRITE];
+	vh->numberio = io_u->numberio;
 
 	vh->offset = io_u->offset + header_num * td->o.verify_interval;
 }
@@ -942,6 +951,8 @@ void populate_verify_io_u(struct thread_data *td, struct io_u *io_u)
 	if (td->o.verify == VERIFY_NULL)
 		return;
 
+	io_u->numberio = td->io_issues[io_u->ddir];
+
 	fill_pattern_headers(td, io_u, 0, 0);
 }
 
@@ -974,6 +985,7 @@ int get_next_verify(struct thread_data *td, struct io_u *io_u)
 
 		io_u->offset = ipo->offset;
 		io_u->buflen = ipo->len;
+		io_u->numberio = ipo->numberio;
 		io_u->file = ipo->file;
 		io_u->flags |= IO_U_F_VER_LIST;
 
-- 
1.7.12.4

Re: [PATCH] Adds check for numberio during verify phase.

[Juan Casse]

Hi Jens,

I know you're very busy. Have you had a chance to look at this change?
I need this for the next patch: "[PATCH V3] Adds verify_only option".

Best regards,
Juan

On Thu, Sep 5, 2013 at 8:10 PM, Juan Casse <jcasse@chromium.org> wrote:
> Currently, fio checks the block offset number in a block's header during
> the verify phase.
> We add a check for the io number (numberio) to detect stale blocks. This
> check is performed only on workloads that write data, as those workloads
> know what numberio was written to each block.
> td->io_issues[ddir] = 0; was removed so that numberio does not get reset
> at each iteration; we want numberio to keep incrementing to reflect
> how many times the same data was written.
>
> Signed-off-by: Juan Casse <jcasse@chromium.org>
> Reviewed-by: Grant Grundler <grundler@chromium.org>
>
> Fixed typo.
> ---
>  ioengine.h |  1 +
>  iolog.c    |  1 +
>  iolog.h    |  1 +
>  libfio.c   |  1 -
>  verify.c   | 14 +++++++++++++-
>  5 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/ioengine.h b/ioengine.h
> index 31662eb..812febd 100644
> --- a/ioengine.h
> +++ b/ioengine.h
> @@ -50,6 +50,7 @@ struct io_u {
>          */
>         unsigned long buflen;
>         unsigned long long offset;
> +       unsigned short numberio;
>         void *buf;
>
>         /*
> diff --git a/iolog.c b/iolog.c
> index 9bcf0d8..6459d3b 100644
> --- a/iolog.c
> +++ b/iolog.c
> @@ -188,6 +188,7 @@ void log_io_piece(struct thread_data *td, struct io_u *io_u)
>         ipo->file = io_u->file;
>         ipo->offset = io_u->offset;
>         ipo->len = io_u->buflen;
> +       ipo->numberio = io_u->numberio;
>
>         if (io_u_should_trim(td, io_u)) {
>                 flist_add_tail(&ipo->trim_list, &td->trim_list);
> diff --git a/iolog.h b/iolog.h
> index 8fedc19..94e0fc1 100644
> --- a/iolog.h
> +++ b/iolog.h
> @@ -78,6 +78,7 @@ struct io_piece {
>                 struct fio_file *file;
>         };
>         unsigned long long offset;
> +       unsigned short numberio;
>         unsigned long len;
>         unsigned int flags;
>         enum fio_ddir ddir;
> diff --git a/libfio.c b/libfio.c
> index c26d6a3..6e290f4 100644
> --- a/libfio.c
> +++ b/libfio.c
> @@ -83,7 +83,6 @@ static void reset_io_counters(struct thread_data *td)
>                 td->this_io_blocks[ddir] = 0;
>                 td->rate_bytes[ddir] = 0;
>                 td->rate_blocks[ddir] = 0;
> -               td->io_issues[ddir] = 0;
>         }
>         td->zone_bytes = 0;
>
> diff --git a/verify.c b/verify.c
> index 9e88d61..63def12 100644
> --- a/verify.c
> +++ b/verify.c
> @@ -369,6 +369,15 @@ static int verify_io_u_meta(struct verify_header *hdr, struct vcont *vc)
>         if (td->o.verify_pattern_bytes)
>                 ret |= verify_io_u_pattern(hdr, vc);
>
> +       /*
> +        * For read-only workloads, the program cannot be certain of the
> +        * last numberio written to a block. Checking of numberio will be done
> +        * only for workloads that write data.
> +        */
> +       if (td_write(td) || td_rw(td))
> +               if (vh->numberio != io_u->numberio)
> +                       ret = EILSEQ;
> +
>         if (!ret)
>                 return 0;
>
> @@ -768,7 +777,7 @@ static void fill_meta(struct verify_header *hdr, struct thread_data *td,
>         vh->time_sec = io_u->start_time.tv_sec;
>         vh->time_usec = io_u->start_time.tv_usec;
>
> -       vh->numberio = td->io_issues[DDIR_WRITE];
> +       vh->numberio = io_u->numberio;
>
>         vh->offset = io_u->offset + header_num * td->o.verify_interval;
>  }
> @@ -942,6 +951,8 @@ void populate_verify_io_u(struct thread_data *td, struct io_u *io_u)
>         if (td->o.verify == VERIFY_NULL)
>                 return;
>
> +       io_u->numberio = td->io_issues[io_u->ddir];
> +
>         fill_pattern_headers(td, io_u, 0, 0);
>  }
>
> @@ -974,6 +985,7 @@ int get_next_verify(struct thread_data *td, struct io_u *io_u)
>
>                 io_u->offset = ipo->offset;
>                 io_u->buflen = ipo->len;
> +               io_u->numberio = ipo->numberio;
>                 io_u->file = ipo->file;
>                 io_u->flags |= IO_U_F_VER_LIST;
>
> --
> 1.7.12.4
>

[PATCH] Adds check for numberio during verify phase.

[Juan Casse]

Currently, fio checks the block offset number in a block's header during
the verify phase.
We add a check for the io number (numberio) to detect stale blocks. This
check is performed only on workloads that write data, as those workloads
know what numberio was written to each block.
td->io_issues[ddir] = 0; was removed so that numberio does not get reset
at each iteration; we want numberio to keep incrementing to reflect
how many times the same data was written.

Signed-off-by: Juan Casse <jcasse@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>

Fixed typo.
---
 ioengine.h |  1 +
 iolog.c    |  1 +
 iolog.h    |  1 +
 libfio.c   |  1 -
 verify.c   | 14 +++++++++++++-
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/ioengine.h b/ioengine.h
index 31662eb..812febd 100644
--- a/ioengine.h
+++ b/ioengine.h
@@ -50,6 +50,7 @@ struct io_u {
 	 */
 	unsigned long buflen;
 	unsigned long long offset;
+	unsigned short numberio;
 	void *buf;
 
 	/*
diff --git a/iolog.c b/iolog.c
index 9bcf0d8..6459d3b 100644
--- a/iolog.c
+++ b/iolog.c
@@ -188,6 +188,7 @@ void log_io_piece(struct thread_data *td, struct io_u *io_u)
 	ipo->file = io_u->file;
 	ipo->offset = io_u->offset;
 	ipo->len = io_u->buflen;
+	ipo->numberio = io_u->numberio;
 
 	if (io_u_should_trim(td, io_u)) {
 		flist_add_tail(&ipo->trim_list, &td->trim_list);
diff --git a/iolog.h b/iolog.h
index 8fedc19..94e0fc1 100644
--- a/iolog.h
+++ b/iolog.h
@@ -78,6 +78,7 @@ struct io_piece {
 		struct fio_file *file;
 	};
 	unsigned long long offset;
+	unsigned short numberio;
 	unsigned long len;
 	unsigned int flags;
 	enum fio_ddir ddir;
diff --git a/libfio.c b/libfio.c
index c26d6a3..6e290f4 100644
--- a/libfio.c
+++ b/libfio.c
@@ -83,7 +83,6 @@ static void reset_io_counters(struct thread_data *td)
 		td->this_io_blocks[ddir] = 0;
 		td->rate_bytes[ddir] = 0;
 		td->rate_blocks[ddir] = 0;
-		td->io_issues[ddir] = 0;
 	}
 	td->zone_bytes = 0;
 
diff --git a/verify.c b/verify.c
index 9e88d61..63def12 100644
--- a/verify.c
+++ b/verify.c
@@ -369,6 +369,15 @@ static int verify_io_u_meta(struct verify_header *hdr, struct vcont *vc)
 	if (td->o.verify_pattern_bytes)
 		ret |= verify_io_u_pattern(hdr, vc);
 
+	/*
+	 * For read-only workloads, the program cannot be certain of the
+	 * last numberio written to a block. Checking of numberio will be done
+	 * only for workloads that write data.
+	 */
+	if (td_write(td) || td_rw(td))
+		if (vh->numberio != io_u->numberio)
+			ret = EILSEQ;
+
 	if (!ret)
 		return 0;
 
@@ -768,7 +777,7 @@ static void fill_meta(struct verify_header *hdr, struct thread_data *td,
 	vh->time_sec = io_u->start_time.tv_sec;
 	vh->time_usec = io_u->start_time.tv_usec;
 
-	vh->numberio = td->io_issues[DDIR_WRITE];
+	vh->numberio = io_u->numberio;
 
 	vh->offset = io_u->offset + header_num * td->o.verify_interval;
 }
@@ -942,6 +951,8 @@ void populate_verify_io_u(struct thread_data *td, struct io_u *io_u)
 	if (td->o.verify == VERIFY_NULL)
 		return;
 
+	io_u->numberio = td->io_issues[io_u->ddir];
+
 	fill_pattern_headers(td, io_u, 0, 0);
 }
 
@@ -974,6 +985,7 @@ int get_next_verify(struct thread_data *td, struct io_u *io_u)
 
 		io_u->offset = ipo->offset;
 		io_u->buflen = ipo->len;
+		io_u->numberio = ipo->numberio;
 		io_u->file = ipo->file;
 		io_u->flags |= IO_U_F_VER_LIST;
 
-- 
1.7.12.4

[PATCH V3] Adds verify_only option

[Juan Casse]

When this option is set, a dry run (no actual io is performed) of the
workload will be done in order to compute the numberio for each block
header without overwriting the data on disk. Then, do_verify() will be
effectively verifying data that was written in a previous fio run.
In the case that "loops" is set to more than 1, do_verify() will delay
the verification of numberio to the last iteration when the same
numberio state that would have been written to disk in a previous
fio run has been reached.

Signed-off-by: Juan Casse <jcasse@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>
---
This patch depends on "[PATCH] Adds check for numberio during verify phase"
sent on September 5, 2013.

 HOWTO            | 11 ++++++++++-
 README           |  2 ++
 backend.c        | 43 ++++++++++++++++++++++++++++++++++++++++++-
 init.c           | 17 +++++++++++++++++
 options.c        |  9 +++++++++
 thread_options.h |  2 ++
 verify.c         |  8 ++++++--
 7 files changed, 88 insertions(+), 4 deletions(-)

diff --git a/HOWTO b/HOWTO
index 005dac2..a3ca770 100644
--- a/HOWTO
+++ b/HOWTO
@@ -1039,6 +1039,13 @@ loops=int	Run the specified number of iterations of this job. Used
 		to repeat the same workload a given number of times. Defaults
 		to 1.
 
+verify_only	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 time_based option set.
+
 do_verify=bool	Run the verify phase after a write phase. Only makes sense if
 		verify is set. Defaults to 1.
 
@@ -1077,7 +1084,9 @@ verify=str	If writing to a file, fio can verify the file contents
 
 			meta	Write extra information about each io
 				(timestamp, block number etc.). The block
-				number is verified. See also verify_pattern.
+				number is verified. The io sequence number is
+				verified for workloads that write data.
+				See also verify_pattern.
 
 			null	Only pretend to verify. Useful for testing
 				internals with ioengine=null, not for much
diff --git a/README b/README
index 15a0731..7942069 100644
--- a/README
+++ b/README
@@ -141,6 +141,8 @@ $ fio
 	--latency-log		Generate per-job latency logs
 	--bandwidth-log		Generate per-job bandwidth logs
 	--minimal		Minimal (terse) output
+	--verifyonly		Skip workload io and only verify data
+				(includes stale data check)
 	--output-format=type	Output format (terse,json,normal)
 	--terse-version=type	Terse version output format (default 3, or 2 or 4).
 	--version		Print version info and exit
diff --git a/backend.c b/backend.c
index b9c1c12..c74c001 100644
--- a/backend.c
+++ b/backend.c
@@ -1104,6 +1104,44 @@ static int exec_string(struct thread_options *o, const char *string, const char
 }
 
 /*
+ * Dry run to compute correct state of numberio for verification.
+ */
+static uint64_t do_dry_run(struct thread_data *td)
+{
+	uint64_t bytes_done[DDIR_RWDIR_CNT] = { 0, 0, 0 };
+
+	td_set_runstate(td, TD_RUNNING);
+
+	while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
+		(!flist_empty(&td->trim_list)) || !io_bytes_exceeded(td)) {
+		struct io_u *io_u;
+		int ret;
+
+		if (td->terminate || td->done)
+			break;
+
+		io_u = get_io_u(td);
+		if (!io_u)
+			break;
+
+		io_u->flags |= IO_U_F_FLIGHT;
+		io_u->error = 0;
+		io_u->resid = 0;
+		if (ddir_rw(acct_ddir(io_u)))
+			td->io_issues[acct_ddir(io_u)]++;
+		if (ddir_rw(io_u->ddir)) {
+			io_u_mark_depth(td, 1);
+			td->ts.total_io_u[io_u->ddir]++;
+		}
+
+		ret = io_u_sync_complete(td, io_u, bytes_done);
+		(void) ret;
+	}
+
+	return bytes_done[DDIR_WRITE] + bytes_done[DDIR_TRIM];
+}
+
+/*
  * Entry point for the thread based jobs. The process based jobs end up
  * here as well, after a little setup.
  */
@@ -1311,7 +1349,10 @@ static void *thread_main(void *data)
 
 		prune_io_piece_log(td);
 
-		verify_bytes = do_io(td);
+		if (td->o.verify_only && (td_write(td) || td_rw(td)))
+			verify_bytes = do_dry_run(td);
+		else
+			verify_bytes = do_io(td);
 
 		clear_state = 1;
 
diff --git a/init.c b/init.c
index 1afc341..34d1f14 100644
--- a/init.c
+++ b/init.c
@@ -60,6 +60,8 @@ int write_bw_log = 0;
 int read_only = 0;
 int status_interval = 0;
 
+int verify_only = 0;
+
 static int write_lat_log;
 
 static int prev_group_jobs;
@@ -139,6 +141,11 @@ static struct option l_opts[FIO_NR_OPTIONS] = {
 		.val		= 'r' | FIO_CLIENT_FLAG,
 	},
 	{
+		.name		= (char *) "verifyonly",
+		.has_arg	= no_argument,
+		.val		= 'y' | FIO_CLIENT_FLAG,
+	},
+	{
 		.name		= (char *) "eta",
 		.has_arg	= required_argument,
 		.val		= 'e' | FIO_CLIENT_FLAG,
@@ -928,6 +935,13 @@ static int add_job(struct thread_data *td, const char *jobname, int job_add_num,
 	int numjobs, file_alloced;
 	struct thread_options *o = &td->o;
 
+	/* 
+	 * Ensure job option verify_only is set when provided as a
+	 * command-line argument.
+	 */
+	if (verify_only)
+		o->verify_only = 1;
+
 	/*
 	 * the def_thread is just for options, it's not a real job
 	 */
@@ -1661,6 +1675,9 @@ int parse_cmd_line(int argc, char *argv[], int client_type)
 		case 'r':
 			read_only = 1;
 			break;
+		case 'y':
+			verify_only = 1;
+			break;
 		case 'v':
 			if (!cur_client) {
 				log_info("%s\n", fio_version_string);
diff --git a/options.c b/options.c
index caf89d3..e6b9ec9 100644
--- a/options.c
+++ b/options.c
@@ -1935,6 +1935,15 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
 		.group	= FIO_OPT_G_RUNTIME,
 	},
 	{
+		.name	= "verify_only",
+		.lname	= "Verify only",
+		.type	= FIO_OPT_STR_SET,
+		.off1	= td_var_offset(verify_only),
+		.help	= "Verifies previously written data is still valid",
+		.category = FIO_OPT_C_GENERAL,
+		.group	= FIO_OPT_G_RUNTIME,
+	},
+	{
 		.name	= "ramp_time",
 		.lname	= "Ramp time",
 		.type	= FIO_OPT_STR_VAL_TIME,
diff --git a/thread_options.h b/thread_options.h
index 3f345c5..c9660b4 100644
--- a/thread_options.h
+++ b/thread_options.h
@@ -107,6 +107,8 @@ struct thread_options {
 	unsigned int fsync_on_close;
 	unsigned int bs_is_seq_rand;
 
+	unsigned int verify_only;
+
 	unsigned int random_distribution;
 
 	fio_fp64_t zipf_theta;
diff --git a/verify.c b/verify.c
index 63def12..9343ab9 100644
--- a/verify.c
+++ b/verify.c
@@ -373,10 +373,14 @@ static int verify_io_u_meta(struct verify_header *hdr, struct vcont *vc)
 	 * For read-only workloads, the program cannot be certain of the
 	 * last numberio written to a block. Checking of numberio will be done
 	 * only for workloads that write data.
+	 * For verify_only, numberio will be checked in the last iteration when
+	 * the correct state of numberio, that would have been written to each
+	 * block in a previous run of fio, has been reached.
 	 */
 	if (td_write(td) || td_rw(td))
-		if (vh->numberio != io_u->numberio)
-			ret = EILSEQ;
+		if (!td->o.verify_only || td->o.loops == 0)
+			if (vh->numberio != io_u->numberio)
+				ret = EILSEQ;
 
 	if (!ret)
 		return 0;
-- 
1.7.12.4

[PATCH] Adds check for rand_seed during verify phase.

[Juan Casse]

Improve data integrity checking of header (meta) data.
verify_header() will now return an additional error:
"verify: bad header rand seed ..."
The addition of the check of rand_seed helps detect stale data from
previous fio runs.
This patch also disambiguates the different data mismatches by returning
different error codes from verify_header().

Signed-off-by: Juan Casse <jcasse@chromium.org>
Reviewed-by: Grant Grundler <grundler@chromium.org>
---
 verify.c | 45 ++++++++++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/verify.c b/verify.c
index 9e88d61..957613c 100644
--- a/verify.c
+++ b/verify.c
@@ -644,18 +644,17 @@ static int verify_header(struct io_u *io_u, struct verify_header *hdr)
 	uint32_t crc;
 
 	if (hdr->magic != FIO_HDR_MAGIC)
-		return 0;
-	if (hdr->len > io_u->buflen) {
-		log_err("fio: verify header exceeds buffer length (%u > %lu)\n", hdr->len, io_u->buflen);
-		return 0;
-	}
+		return 1;
+	if (hdr->len > io_u->buflen)
+		return 2;
+	if (hdr->rand_seed != io_u->rand_seed)
+		return 3;
 
 	crc = fio_crc32c(p, offsetof(struct verify_header, crc32));
 	if (crc == hdr->crc32)
-		return 1;
-
+		return 0;
 	log_err("fio: verify header crc %x, calculated %x\n", hdr->crc32, crc);
-	return 0;
+	return 4;
 }
 
 int verify_io_u(struct thread_data *td, struct io_u *io_u)
@@ -692,13 +691,41 @@ int verify_io_u(struct thread_data *td, struct io_u *io_u)
 			memswp(p, p + td->o.verify_offset, header_size);
 		hdr = p;
 
-		if (!verify_header(io_u, hdr)) {
+		ret = verify_header(io_u, hdr);
+		switch (ret) {
+		case 0:
+			break;
+		case 1:
 			log_err("verify: bad magic header %x, wanted %x at "
 				"file %s offset %llu, length %u\n",
 				hdr->magic, FIO_HDR_MAGIC,
 				io_u->file->file_name,
 				io_u->offset + hdr_num * hdr->len, hdr->len);
 			return EILSEQ;
+			break;
+		case 2:
+			log_err("fio: verify header exceeds buffer length (%u "
+				"> %lu)\n", hdr->len, io_u->buflen);
+			return EILSEQ;
+			break;
+		case 3:
+			log_err("verify: bad header rand_seed %"PRIu64
+				", wanted %"PRIu64" at file %s offset %llu, "
+				"length %u\n",
+				hdr->rand_seed, io_u->rand_seed,
+				io_u->file->file_name,
+				io_u->offset + hdr_num * hdr->len, hdr->len);
+			return EILSEQ;
+			break;
+		case 4:
+			return EILSEQ;
+			break;
+		default:
+			log_err("verify: unknown header error at file %s "
+			"offset %llu, length %u\n",
+			io_u->file->file_name,
+			io_u->offset + hdr_num * hdr->len, hdr->len);
+			return EILSEQ;
 		}
 
 		if (td->o.verify != VERIFY_NONE)
-- 
1.7.12.4

Re: [PATCH] Adds check for numberio during verify phase.

[Grant Grundler]

Jens, Ping?
You think you can still integrate the three patches from Juan?

thanks!
grant

On Tue, Sep 17, 2013 at 2:06 PM, Juan Casse <jcasse@chromium.org> wrote:
> Currently, fio checks the block offset number in a block's header during
> the verify phase.
> We add a check for the io number (numberio) to detect stale blocks. This
> check is performed only on workloads that write data, as those workloads
> know what numberio was written to each block.
> td->io_issues[ddir] = 0; was removed so that numberio does not get reset
> at each iteration; we want numberio to keep incrementing to reflect
> how many times the same data was written.
>
> Signed-off-by: Juan Casse <jcasse@chromium.org>
> Reviewed-by: Grant Grundler <grundler@chromium.org>
>
> Fixed typo.
> ---
>  ioengine.h |  1 +
>  iolog.c    |  1 +
>  iolog.h    |  1 +
>  libfio.c   |  1 -
>  verify.c   | 14 +++++++++++++-
>  5 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/ioengine.h b/ioengine.h
> index 31662eb..812febd 100644
> --- a/ioengine.h
> +++ b/ioengine.h
> @@ -50,6 +50,7 @@ struct io_u {
>          */
>         unsigned long buflen;
>         unsigned long long offset;
> +       unsigned short numberio;
>         void *buf;
>
>         /*
> diff --git a/iolog.c b/iolog.c
> index 9bcf0d8..6459d3b 100644
> --- a/iolog.c
> +++ b/iolog.c
> @@ -188,6 +188,7 @@ void log_io_piece(struct thread_data *td, struct io_u *io_u)
>         ipo->file = io_u->file;
>         ipo->offset = io_u->offset;
>         ipo->len = io_u->buflen;
> +       ipo->numberio = io_u->numberio;
>
>         if (io_u_should_trim(td, io_u)) {
>                 flist_add_tail(&ipo->trim_list, &td->trim_list);
> diff --git a/iolog.h b/iolog.h
> index 8fedc19..94e0fc1 100644
> --- a/iolog.h
> +++ b/iolog.h
> @@ -78,6 +78,7 @@ struct io_piece {
>                 struct fio_file *file;
>         };
>         unsigned long long offset;
> +       unsigned short numberio;
>         unsigned long len;
>         unsigned int flags;
>         enum fio_ddir ddir;
> diff --git a/libfio.c b/libfio.c
> index c26d6a3..6e290f4 100644
> --- a/libfio.c
> +++ b/libfio.c
> @@ -83,7 +83,6 @@ static void reset_io_counters(struct thread_data *td)
>                 td->this_io_blocks[ddir] = 0;
>                 td->rate_bytes[ddir] = 0;
>                 td->rate_blocks[ddir] = 0;
> -               td->io_issues[ddir] = 0;
>         }
>         td->zone_bytes = 0;
>
> diff --git a/verify.c b/verify.c
> index 9e88d61..63def12 100644
> --- a/verify.c
> +++ b/verify.c
> @@ -369,6 +369,15 @@ static int verify_io_u_meta(struct verify_header *hdr, struct vcont *vc)
>         if (td->o.verify_pattern_bytes)
>                 ret |= verify_io_u_pattern(hdr, vc);
>
> +       /*
> +        * For read-only workloads, the program cannot be certain of the
> +        * last numberio written to a block. Checking of numberio will be done
> +        * only for workloads that write data.
> +        */
> +       if (td_write(td) || td_rw(td))
> +               if (vh->numberio != io_u->numberio)
> +                       ret = EILSEQ;
> +
>         if (!ret)
>                 return 0;
>
> @@ -768,7 +777,7 @@ static void fill_meta(struct verify_header *hdr, struct thread_data *td,
>         vh->time_sec = io_u->start_time.tv_sec;
>         vh->time_usec = io_u->start_time.tv_usec;
>
> -       vh->numberio = td->io_issues[DDIR_WRITE];
> +       vh->numberio = io_u->numberio;
>
>         vh->offset = io_u->offset + header_num * td->o.verify_interval;
>  }
> @@ -942,6 +951,8 @@ void populate_verify_io_u(struct thread_data *td, struct io_u *io_u)
>         if (td->o.verify == VERIFY_NULL)
>                 return;
>
> +       io_u->numberio = td->io_issues[io_u->ddir];
> +
>         fill_pattern_headers(td, io_u, 0, 0);
>  }
>
> @@ -974,6 +985,7 @@ int get_next_verify(struct thread_data *td, struct io_u *io_u)
>
>                 io_u->offset = ipo->offset;
>                 io_u->buflen = ipo->len;
> +               io_u->numberio = ipo->numberio;
>                 io_u->file = ipo->file;
>                 io_u->flags |= IO_U_F_VER_LIST;
>
> --
> 1.7.12.4
>

Re: [PATCH V3] Adds verify_only option

[Grant Grundler]

Jens, Ping?
Still able to integrate this patch?

thanks,
grant

On Tue, Sep 17, 2013 at 2:06 PM, Juan Casse <jcasse@chromium.org> wrote:
> When this option is set, a dry run (no actual io is performed) of the
> workload will be done in order to compute the numberio for each block
> header without overwriting the data on disk. Then, do_verify() will be
> effectively verifying data that was written in a previous fio run.
> In the case that "loops" is set to more than 1, do_verify() will delay
> the verification of numberio to the last iteration when the same
> numberio state that would have been written to disk in a previous
> fio run has been reached.
>
> Signed-off-by: Juan Casse <jcasse@chromium.org>
> Reviewed-by: Grant Grundler <grundler@chromium.org>
> ---
> This patch depends on "[PATCH] Adds check for numberio during verify phase"
> sent on September 5, 2013.
>
>  HOWTO            | 11 ++++++++++-
>  README           |  2 ++
>  backend.c        | 43 ++++++++++++++++++++++++++++++++++++++++++-
>  init.c           | 17 +++++++++++++++++
>  options.c        |  9 +++++++++
>  thread_options.h |  2 ++
>  verify.c         |  8 ++++++--
>  7 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/HOWTO b/HOWTO
> index 005dac2..a3ca770 100644
> --- a/HOWTO
> +++ b/HOWTO
> @@ -1039,6 +1039,13 @@ loops=int        Run the specified number of iterations of this job. Used
>                 to repeat the same workload a given number of times. Defaults
>                 to 1.
>
> +verify_only    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 time_based option set.
> +
>  do_verify=bool Run the verify phase after a write phase. Only makes sense if
>                 verify is set. Defaults to 1.
>
> @@ -1077,7 +1084,9 @@ verify=str        If writing to a file, fio can verify the file contents
>
>                         meta    Write extra information about each io
>                                 (timestamp, block number etc.). The block
> -                               number is verified. See also verify_pattern.
> +                               number is verified. The io sequence number is
> +                               verified for workloads that write data.
> +                               See also verify_pattern.
>
>                         null    Only pretend to verify. Useful for testing
>                                 internals with ioengine=null, not for much
> diff --git a/README b/README
> index 15a0731..7942069 100644
> --- a/README
> +++ b/README
> @@ -141,6 +141,8 @@ $ fio
>         --latency-log           Generate per-job latency logs
>         --bandwidth-log         Generate per-job bandwidth logs
>         --minimal               Minimal (terse) output
> +       --verifyonly            Skip workload io and only verify data
> +                               (includes stale data check)
>         --output-format=type    Output format (terse,json,normal)
>         --terse-version=type    Terse version output format (default 3, or 2 or 4).
>         --version               Print version info and exit
> diff --git a/backend.c b/backend.c
> index b9c1c12..c74c001 100644
> --- a/backend.c
> +++ b/backend.c
> @@ -1104,6 +1104,44 @@ static int exec_string(struct thread_options *o, const char *string, const char
>  }
>
>  /*
> + * Dry run to compute correct state of numberio for verification.
> + */
> +static uint64_t do_dry_run(struct thread_data *td)
> +{
> +       uint64_t bytes_done[DDIR_RWDIR_CNT] = { 0, 0, 0 };
> +
> +       td_set_runstate(td, TD_RUNNING);
> +
> +       while ((td->o.read_iolog_file && !flist_empty(&td->io_log_list)) ||
> +               (!flist_empty(&td->trim_list)) || !io_bytes_exceeded(td)) {
> +               struct io_u *io_u;
> +               int ret;
> +
> +               if (td->terminate || td->done)
> +                       break;
> +
> +               io_u = get_io_u(td);
> +               if (!io_u)
> +                       break;
> +
> +               io_u->flags |= IO_U_F_FLIGHT;
> +               io_u->error = 0;
> +               io_u->resid = 0;
> +               if (ddir_rw(acct_ddir(io_u)))
> +                       td->io_issues[acct_ddir(io_u)]++;
> +               if (ddir_rw(io_u->ddir)) {
> +                       io_u_mark_depth(td, 1);
> +                       td->ts.total_io_u[io_u->ddir]++;
> +               }
> +
> +               ret = io_u_sync_complete(td, io_u, bytes_done);
> +               (void) ret;
> +       }
> +
> +       return bytes_done[DDIR_WRITE] + bytes_done[DDIR_TRIM];
> +}
> +
> +/*
>   * Entry point for the thread based jobs. The process based jobs end up
>   * here as well, after a little setup.
>   */
> @@ -1311,7 +1349,10 @@ static void *thread_main(void *data)
>
>                 prune_io_piece_log(td);
>
> -               verify_bytes = do_io(td);
> +               if (td->o.verify_only && (td_write(td) || td_rw(td)))
> +                       verify_bytes = do_dry_run(td);
> +               else
> +                       verify_bytes = do_io(td);
>
>                 clear_state = 1;
>
> diff --git a/init.c b/init.c
> index 1afc341..34d1f14 100644
> --- a/init.c
> +++ b/init.c
> @@ -60,6 +60,8 @@ int write_bw_log = 0;
>  int read_only = 0;
>  int status_interval = 0;
>
> +int verify_only = 0;
> +
>  static int write_lat_log;
>
>  static int prev_group_jobs;
> @@ -139,6 +141,11 @@ static struct option l_opts[FIO_NR_OPTIONS] = {
>                 .val            = 'r' | FIO_CLIENT_FLAG,
>         },
>         {
> +               .name           = (char *) "verifyonly",
> +               .has_arg        = no_argument,
> +               .val            = 'y' | FIO_CLIENT_FLAG,
> +       },
> +       {
>                 .name           = (char *) "eta",
>                 .has_arg        = required_argument,
>                 .val            = 'e' | FIO_CLIENT_FLAG,
> @@ -928,6 +935,13 @@ static int add_job(struct thread_data *td, const char *jobname, int job_add_num,
>         int numjobs, file_alloced;
>         struct thread_options *o = &td->o;
>
> +       /*
> +        * Ensure job option verify_only is set when provided as a
> +        * command-line argument.
> +        */
> +       if (verify_only)
> +               o->verify_only = 1;
> +
>         /*
>          * the def_thread is just for options, it's not a real job
>          */
> @@ -1661,6 +1675,9 @@ int parse_cmd_line(int argc, char *argv[], int client_type)
>                 case 'r':
>                         read_only = 1;
>                         break;
> +               case 'y':
> +                       verify_only = 1;
> +                       break;
>                 case 'v':
>                         if (!cur_client) {
>                                 log_info("%s\n", fio_version_string);
> diff --git a/options.c b/options.c
> index caf89d3..e6b9ec9 100644
> --- a/options.c
> +++ b/options.c
> @@ -1935,6 +1935,15 @@ struct fio_option fio_options[FIO_MAX_OPTS] = {
>                 .group  = FIO_OPT_G_RUNTIME,
>         },
>         {
> +               .name   = "verify_only",
> +               .lname  = "Verify only",
> +               .type   = FIO_OPT_STR_SET,
> +               .off1   = td_var_offset(verify_only),
> +               .help   = "Verifies previously written data is still valid",
> +               .category = FIO_OPT_C_GENERAL,
> +               .group  = FIO_OPT_G_RUNTIME,
> +       },
> +       {
>                 .name   = "ramp_time",
>                 .lname  = "Ramp time",
>                 .type   = FIO_OPT_STR_VAL_TIME,
> diff --git a/thread_options.h b/thread_options.h
> index 3f345c5..c9660b4 100644
> --- a/thread_options.h
> +++ b/thread_options.h
> @@ -107,6 +107,8 @@ struct thread_options {
>         unsigned int fsync_on_close;
>         unsigned int bs_is_seq_rand;
>
> +       unsigned int verify_only;
> +
>         unsigned int random_distribution;
>
>         fio_fp64_t zipf_theta;
> diff --git a/verify.c b/verify.c
> index 63def12..9343ab9 100644
> --- a/verify.c
> +++ b/verify.c
> @@ -373,10 +373,14 @@ static int verify_io_u_meta(struct verify_header *hdr, struct vcont *vc)
>          * For read-only workloads, the program cannot be certain of the
>          * last numberio written to a block. Checking of numberio will be done
>          * only for workloads that write data.
> +        * For verify_only, numberio will be checked in the last iteration when
> +        * the correct state of numberio, that would have been written to each
> +        * block in a previous run of fio, has been reached.
>          */
>         if (td_write(td) || td_rw(td))
> -               if (vh->numberio != io_u->numberio)
> -                       ret = EILSEQ;
> +               if (!td->o.verify_only || td->o.loops == 0)
> +                       if (vh->numberio != io_u->numberio)
> +                               ret = EILSEQ;
>
>         if (!ret)
>                 return 0;
> --
> 1.7.12.4
>

Re: [PATCH] Adds check for rand_seed during verify phase.

[Grant Grundler]

Jens,
Ping?

Still able to integrate this patch?

thanks,
grant

On Tue, Sep 17, 2013 at 2:06 PM, Juan Casse <jcasse@chromium.org> wrote:
> Improve data integrity checking of header (meta) data.
> verify_header() will now return an additional error:
> "verify: bad header rand seed ..."
> The addition of the check of rand_seed helps detect stale data from
> previous fio runs.
> This patch also disambiguates the different data mismatches by returning
> different error codes from verify_header().
>
> Signed-off-by: Juan Casse <jcasse@chromium.org>
> Reviewed-by: Grant Grundler <grundler@chromium.org>
> ---
>  verify.c | 45 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 36 insertions(+), 9 deletions(-)
>
> diff --git a/verify.c b/verify.c
> index 9e88d61..957613c 100644
> --- a/verify.c
> +++ b/verify.c
> @@ -644,18 +644,17 @@ static int verify_header(struct io_u *io_u, struct verify_header *hdr)
>         uint32_t crc;
>
>         if (hdr->magic != FIO_HDR_MAGIC)
> -               return 0;
> -       if (hdr->len > io_u->buflen) {
> -               log_err("fio: verify header exceeds buffer length (%u > %lu)\n", hdr->len, io_u->buflen);
> -               return 0;
> -       }
> +               return 1;
> +       if (hdr->len > io_u->buflen)
> +               return 2;
> +       if (hdr->rand_seed != io_u->rand_seed)
> +               return 3;
>
>         crc = fio_crc32c(p, offsetof(struct verify_header, crc32));
>         if (crc == hdr->crc32)
> -               return 1;
> -
> +               return 0;
>         log_err("fio: verify header crc %x, calculated %x\n", hdr->crc32, crc);
> -       return 0;
> +       return 4;
>  }
>
>  int verify_io_u(struct thread_data *td, struct io_u *io_u)
> @@ -692,13 +691,41 @@ int verify_io_u(struct thread_data *td, struct io_u *io_u)
>                         memswp(p, p + td->o.verify_offset, header_size);
>                 hdr = p;
>
> -               if (!verify_header(io_u, hdr)) {
> +               ret = verify_header(io_u, hdr);
> +               switch (ret) {
> +               case 0:
> +                       break;
> +               case 1:
>                         log_err("verify: bad magic header %x, wanted %x at "
>                                 "file %s offset %llu, length %u\n",
>                                 hdr->magic, FIO_HDR_MAGIC,
>                                 io_u->file->file_name,
>                                 io_u->offset + hdr_num * hdr->len, hdr->len);
>                         return EILSEQ;
> +                       break;
> +               case 2:
> +                       log_err("fio: verify header exceeds buffer length (%u "
> +                               "> %lu)\n", hdr->len, io_u->buflen);
> +                       return EILSEQ;
> +                       break;
> +               case 3:
> +                       log_err("verify: bad header rand_seed %"PRIu64
> +                               ", wanted %"PRIu64" at file %s offset %llu, "
> +                               "length %u\n",
> +                               hdr->rand_seed, io_u->rand_seed,
> +                               io_u->file->file_name,
> +                               io_u->offset + hdr_num * hdr->len, hdr->len);
> +                       return EILSEQ;
> +                       break;
> +               case 4:
> +                       return EILSEQ;
> +                       break;
> +               default:
> +                       log_err("verify: unknown header error at file %s "
> +                       "offset %llu, length %u\n",
> +                       io_u->file->file_name,
> +                       io_u->offset + hdr_num * hdr->len, hdr->len);
> +                       return EILSEQ;
>                 }
>
>                 if (td->o.verify != VERIFY_NONE)
> --
> 1.7.12.4
>

Re: [PATCH] Adds check for numberio during verify phase.

[Jens Axboe]

On Fri, Jan 24 2014, Grant Grundler wrote:
> Jens, Ping?
> You think you can still integrate the three patches from Juan?

I think that would be manageable. But really a new feature (or feature
modification) like this should be accompanies by a job file example for
it. Care to provide one?

-- 
Jens Axboe

Re: [PATCH] Adds check for numberio during verify phase.

[Grant Grundler]

[dropping jcasse since this account was deleted after his internship ended]

On Fri, Jan 24, 2014 at 10:58 AM, Jens Axboe <axboe@kernel.dk> wrote:
> On Fri, Jan 24 2014, Grant Grundler wrote:
>> Jens, Ping?
>> You think you can still integrate the three patches from Juan?
>
> I think that would be manageable. But really a new feature (or feature
> modification) like this should be accompanies by a job file example for
> it. Care to provide one?

Yes. Do you mind cloning a git repo?

Specifically this file is what chromium.org is using:
https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/client/site_tests/hardware_StorageFio/8k_async_randwrite

The git repo is hosted here:
    https://chromium.googlesource.com/chromiumos/third_party/autotest/

Use git clone to pull the entire repo:
    git clone https://chromium.googlesource.com/chromiumos/third_party/autotest


BTW, Verification is failing on the 1m_stress control file...working
on that now:
    https://code.google.com/p/chromium/issues/detail?id=337651

I suspect it's a problem of the control file though since we are
getting this warning:
    "Multiple writers may overwrite blocks that belong to other jobs.
This can cause verification failures."

cheers,
grant

Re: [PATCH] Adds check for numberio during verify phase.

[Jens Axboe]

On Fri, Jan 24 2014, Grant Grundler wrote:
> [dropping jcasse since this account was deleted after his internship ended]
> 
> On Fri, Jan 24, 2014 at 10:58 AM, Jens Axboe <axboe@kernel.dk> wrote:
> > On Fri, Jan 24 2014, Grant Grundler wrote:
> >> Jens, Ping?
> >> You think you can still integrate the three patches from Juan?
> >
> > I think that would be manageable. But really a new feature (or feature
> > modification) like this should be accompanies by a job file example for
> > it. Care to provide one?
> 
> Yes. Do you mind cloning a git repo?

It was big :-)

> Specifically this file is what chromium.org is using:
> https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/client/site_tests/hardware_StorageFio/8k_async_randwrite
> 
> The git repo is hosted here:
>     https://chromium.googlesource.com/chromiumos/third_party/autotest/
> 
> Use git clone to pull the entire repo:
>     git clone https://chromium.googlesource.com/chromiumos/third_party/autotest
> 
> 
> BTW, Verification is failing on the 1m_stress control file...working
> on that now:
>     https://code.google.com/p/chromium/issues/detail?id=337651
> 
> I suspect it's a problem of the control file though since we are
> getting this warning:
>     "Multiple writers may overwrite blocks that belong to other jobs.
> This can cause verification failures."

Yes, with 8 jobs going, they are going to be stomping on each others
blocks potentially.

I queued up the 3 patches, but I killed the --verify-only command line
switch. Seems unneeded, might as well just use the job option for that.

-- 
Jens Axboe

Re: [PATCH] Adds check for numberio during verify phase.

[Grant Grundler]

[dropping jcasse since this account was deleted after his internship ended]

On Fri, Jan 24, 2014 at 10:58 AM, Jens Axboe <axboe@kernel.dk> wrote:
> On Fri, Jan 24 2014, Grant Grundler wrote:
>> Jens, Ping?
>> You think you can still integrate the three patches from Juan?
>
> I think that would be manageable. But really a new feature (or feature
> modification) like this should be accompanies by a job file example for
> it. Care to provide one?

Yes. Do you mind cloning a git repo?

Specifically this file is what chromium.org is using:
https://chromium.googlesource.com/chromiumos/third_party/autotest/+/master/client/site_tests/hardware_StorageFio/8k_async_randwrite

The git repo is hosted here:
    https://chromium.googlesource.com/chromiumos/third_party/autotest/

Use git clone to pull the entire repo:
    git clone https://chromium.googlesource.com/chromiumos/third_party/autotest

cheers,
grant

Re: [PATCH] Adds check for numberio during verify phase.

[Grant Grundler]

On Fri, Jan 24, 2014 at 12:07 PM, Jens Axboe <axboe@kernel.dk> wrote:
> On Fri, Jan 24 2014, Grant Grundler wrote:
>> [dropping jcasse since this account was deleted after his internship ended]
>>
>> On Fri, Jan 24, 2014 at 10:58 AM, Jens Axboe <axboe@kernel.dk> wrote:
>> > On Fri, Jan 24 2014, Grant Grundler wrote:
>> >> Jens, Ping?
>> >> You think you can still integrate the three patches from Juan?
>> >
>> > I think that would be manageable. But really a new feature (or feature
>> > modification) like this should be accompanies by a job file example for
>> > it. Care to provide one?
>>
>> Yes. Do you mind cloning a git repo?
>
> It was big :-)

Sorry...but I don't know how to check out a partial repo /o\

upside is you can take a look at all the fio job and autotest control
files we are using. :)

Gwendal is cleaning up our autotest so we only use fio-2.1.2 with
verify/integrity patches applied.
CL is pending for that:
    https://chromium-review.googlesource.com/#/c/183364/

We are trying to make it easier for vendors to pick up these tests and run them.
In particular the "control.hwqual" autotest file.

...
>> BTW, Verification is failing on the 1m_stress control file...working
>> on that now:
>>     https://code.google.com/p/chromium/issues/detail?id=337651
>>
>> I suspect it's a problem of the control file though since we are
>> getting this warning:
>>     "Multiple writers may overwrite blocks that belong to other jobs.
>> This can cause verification failures."
>
> Yes, with 8 jobs going, they are going to be stomping on each others
> blocks potentially.

Yeah - that was my guess too - which means the warning is helpful.

Just to confirm: with numjobs=1, verify completes successfully.

> I queued up the 3 patches,

Awesome - thanks! :)

> but I killed the --verify-only command line
> switch. Seems unneeded, might as well just use the job option for that.

Please reconsider. We currently use --verify.  See hardware_StorageFio.py:

hardware_StorageFio/hardware_StorageFio.py:
('8k_async_randwrite', ['--verifyonly'])

I want to re-use the same job file to describe the workload but
override the "write" stage to not be executed.  Just perform verify. I
don't care what the option is called as long as I can reuse the fio
job file.

Having to clone a job file and make sure both files specify the same
things is possible but provides the opportunity for simple, stupid
mistakes. Adding --verify option eliminates that opportunity and means
we have one less fio job file to maintain....note we have quite a few
already.

thanks!
grant

Re: [PATCH] Adds check for numberio during verify phase.

[Jens Axboe]

On Fri, Jan 24 2014, Grant Grundler wrote:
> On Fri, Jan 24, 2014 at 12:07 PM, Jens Axboe <axboe@kernel.dk> wrote:
> > On Fri, Jan 24 2014, Grant Grundler wrote:
> >> [dropping jcasse since this account was deleted after his internship ended]
> >>
> >> On Fri, Jan 24, 2014 at 10:58 AM, Jens Axboe <axboe@kernel.dk> wrote:
> >> > On Fri, Jan 24 2014, Grant Grundler wrote:
> >> >> Jens, Ping?
> >> >> You think you can still integrate the three patches from Juan?
> >> >
> >> > I think that would be manageable. But really a new feature (or feature
> >> > modification) like this should be accompanies by a job file example for
> >> > it. Care to provide one?
> >>
> >> Yes. Do you mind cloning a git repo?
> >
> > It was big :-)
> 
> Sorry...but I don't know how to check out a partial repo /o\
> 
> upside is you can take a look at all the fio job and autotest control
> files we are using. :)
> 
> Gwendal is cleaning up our autotest so we only use fio-2.1.2 with
> verify/integrity patches applied.
> CL is pending for that:
>     https://chromium-review.googlesource.com/#/c/183364/
> 
> We are trying to make it easier for vendors to pick up these tests and run them.
> In particular the "control.hwqual" autotest file.
> 
> ...
> >> BTW, Verification is failing on the 1m_stress control file...working
> >> on that now:
> >>     https://code.google.com/p/chromium/issues/detail?id=337651
> >>
> >> I suspect it's a problem of the control file though since we are
> >> getting this warning:
> >>     "Multiple writers may overwrite blocks that belong to other jobs.
> >> This can cause verification failures."
> >
> > Yes, with 8 jobs going, they are going to be stomping on each others
> > blocks potentially.
> 
> Yeah - that was my guess too - which means the warning is helpful.
> 
> Just to confirm: with numjobs=1, verify completes successfully.
> 
> > I queued up the 3 patches,
> 
> Awesome - thanks! :)
> 
> > but I killed the --verify-only command line
> > switch. Seems unneeded, might as well just use the job option for that.
> 
> Please reconsider. We currently use --verify.  See hardware_StorageFio.py:
> 
> hardware_StorageFio/hardware_StorageFio.py:
> ('8k_async_randwrite', ['--verifyonly'])
> 
> I want to re-use the same job file to describe the workload but
> override the "write" stage to not be executed.  Just perform verify. I
> don't care what the option is called as long as I can reuse the fio
> job file.
> 
> Having to clone a job file and make sure both files specify the same
> things is possible but provides the opportunity for simple, stupid
> mistakes. Adding --verify option eliminates that opportunity and means
> we have one less fio job file to maintain....note we have quite a few
> already.

Why not just use an environment variable, like you do for other things?
Then just have:

verify_only=${VERIFY_ONLY}

and you could easily reuse the same job file.

-- 
Jens Axboe

Re: [PATCH] Adds check for numberio during verify phase.

[Grant Grundler]

On Fri, Jan 24, 2014 at 1:19 PM, Jens Axboe <axboe@kernel.dk> wrote:
...
> Why not just use an environment variable, like you do for other things?
> Then just have:
>
> verify_only=${VERIFY_ONLY}
>
> and you could easily reuse the same job file.

Good idea! That would work.  We'll do that next time we update fio if
there is no verify_only option.

thanks!
grant

Re: [PATCH] Adds check for numberio during verify phase.

[Jens Axboe]

On Fri, Jan 24 2014, Grant Grundler wrote:
> On Fri, Jan 24, 2014 at 1:19 PM, Jens Axboe <axboe@kernel.dk> wrote:
> ...
> > Why not just use an environment variable, like you do for other things?
> > Then just have:
> >
> > verify_only=${VERIFY_ONLY}
> >
> > and you could easily reuse the same job file.
> 
> Good idea! That would work.  We'll do that next time we update fio if
> there is no verify_only option.

Good, so we're all happy! I'm not adding the command line option if I
can avoid it, it seems redundant and I prefer to keep those options if
they have implicit global scope. The ones that exist and could be
options, I've retained them only to avoid unecessarily breaking peoples
setups.

-- 
Jens Axboe