[PATCH] Add code for verify_dump option

[PATCH] Add code for verify_dump option

[Steven Lang]

The verify_dump option is defined and documented, but does nothing.
This one-liner enables the option.

One thing I wanted to open a small discussion on is the default
behavior.  When the verify dump behavior was added, it was something
new, so it seems more sensible to disable it by default to keep the
old behavior; and had it been in for less time I might have been
tempted to include that in the patch.  However, even ignoring the
change in behavior, it seems odd and possibly surprising/unexpected to
someone who does not have experience with fio to have it suddenly spit
out extra files; all the other options to write files
(write_iolog/write_bw_log/write_lat_log) only write anything if
requested.  Is there a good argument to why this option would be
enabled by default?

diff --git a/verify.c b/verify.c
index 68ee60f..36b49f2 100644
--- a/verify.c
+++ b/verify.c
@@ -269,6 +269,9 @@ static void dump_verify_buffers(struct
verify_header *hdr, struct vcont *vc)
 	struct io_u dummy;
 	void *buf;

+        if (!vc->td->o.verify_dump)
+          return;
+
 	/*
 	 * Dump the contents we just read off disk
 	 */

Re: [PATCH] Add code for verify_dump option

[Jens Axboe]

On 2011-10-25 22:33, Steven Lang wrote:
> The verify_dump option is defined and documented, but does nothing.
> This one-liner enables the option.
> 
> One thing I wanted to open a small discussion on is the default
> behavior.  When the verify dump behavior was added, it was something
> new, so it seems more sensible to disable it by default to keep the
> old behavior; and had it been in for less time I might have been
> tempted to include that in the patch.  However, even ignoring the
> change in behavior, it seems odd and possibly surprising/unexpected to
> someone who does not have experience with fio to have it suddenly spit
> out extra files; all the other options to write files
> (write_iolog/write_bw_log/write_lat_log) only write anything if
> requested.  Is there a good argument to why this option would be
> enabled by default?

Thanks, applied. You are right, it probably should have been off by
default, and in fact probably should be even now.

-- 
Jens Axboe