Re: [Qemu-devel] [PATCH] qemu-io: Verify read data by patterns

[Kevin Wolf <mail@kevin-wolf.de>]

Am Donnerstag, 9. April 2009 17:33 schrieb Christoph Hellwig:
> On Thu, Apr 09, 2009 at 03:46:16PM +0200, Kevin Wolf wrote:
> > This patch adds a -P option to read and readv which allows to compare the
> > read data to a given pattern. This can be used to verify data written by
> > write -P.
>
> Very good, I was doing this manually in my WIP testsuite, but having
> qemu-io do this is even better.

Is this testsuite available anywhere, like a git repo? I'm going to continue 
testing and debugging qcow2 next week and I'd rather avoid doing things 
you're already working on.

Oh, and thanks for qemu-io. It's a great tool for that work.

> >  " -q, -- quite mode, do not show I/O statistics\n"
> > @@ -207,8 +208,10 @@ read_f(int argc, char **argv)
> >  	char *buf;
> >  	int64_t offset;
> >  	int count, total;
> > +	int pattern = 0;
> > +	int use_pattern = 0;
> >
> > -	while ((c = getopt(argc, argv, "Cpqv")) != EOF) {
> > +	while ((c = getopt(argc, argv, "CpP:qv")) != EOF) {
> >  		switch (c) {
> >  		case 'C':
> >  			Cflag = 1;
> > @@ -216,6 +219,10 @@ read_f(int argc, char **argv)
> >  		case 'p':
> >  			pflag = 1;
> >  			break;
> > +		case 'P':
> > +			use_pattern = 1;
>
> Following the conventions this variable would be called Pflag, but I'm
> not dead set on this.  (The conventions come from the original xfs_io
> tool)

I considered that, but actually it's not a simple flag anymore as soon as it 
has a parameter. Don't know, you could argue either way.

> > +	if (use_pattern) {
> > +		void* cmp_buf = malloc(count);
> > +		memset(cmp_buf, pattern, count);
> > +		if (memcmp(buf, cmp_buf, count)) {
> > +			printf("Pattern verification failed at offset %lld, %d bytes\n",
> > +				(long long) offset, count);
> > +		}
>
> Can we keep the line shorter than 80 characters?

This is what you get for using tabs. You deserve it. ;-)

Seriously, I found this really annoying when I did the patch because almost 
everything else in qemu has four spaces and I needed to change my editor 
settings for just this file. But I still left indentation on four characters, 
so this line did fit for me.

I can either send a second version of the patch which fixes this line, or we 
could have a patch which changes the indentation of the whole file to four 
spaces (as specified in the coding style document). qemu-io might be new 
enough to not destroy valuable svn blame information with such a patch.

Kevin