[Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[Christoph Hellwig]

Address a couple of review comments from Kevin.


Signed-off-by: Christoph Hellwig <hch@lst.de>

Index: qemu/qemu-io.c
===================================================================
--- qemu.orig/qemu-io.c	2009-07-01 10:45:24.488370800 +0200
+++ qemu/qemu-io.c	2009-07-01 10:52:44.962241654 +0200
@@ -769,25 +769,23 @@ aio_write_done(void *opaque, int ret)
 {
 	struct aio_ctx *ctx = opaque;
 	struct timeval t2;
-	int total;
-	int cnt = 1;
 
 	gettimeofday(&t2, NULL);
 
-	total = ctx->qiov.size;
 
 	if (ret < 0) {
 		printf("aio_write failed: %s\n", strerror(-ret));
 		return;
 	}
 
-	if (ctx->qflag)
+	if (ctx->qflag) {
 		return;
+	}
 
 	/* Finally, report back -- -C gives a parsable format */
 	t2 = tsub(t2, ctx->t1);
-	print_report("wrote", &t2, ctx->offset, ctx->qiov.size, total, cnt,
-		     ctx->Cflag);
+	print_report("wrote", &t2, ctx->offset, ctx->qiov.size,
+		     ctx->qiov.size, 1, ctx->Cflag);
 
 	qemu_io_free(ctx->buf);
 	free(ctx);
@@ -800,44 +798,41 @@ aio_read_done(void *opaque, int ret)
 {
 	struct aio_ctx *ctx = opaque;
 	struct timeval t2;
-	int total;
-	int cnt = 1;
 
 	gettimeofday(&t2, NULL);
 
-	total = ctx->qiov.size;
-
 	if (ret < 0) {
 		printf("readv failed: %s\n", strerror(-ret));
 		return;
 	}
 
 	if (ctx->Pflag) {
-		void *cmp_buf = malloc(total);
+		void *cmp_buf = malloc(ctx->qiov.size);
 
-		memset(cmp_buf, ctx->pattern, total);
-		if (memcmp(ctx->buf, cmp_buf, total)) {
+		memset(cmp_buf, ctx->pattern, ctx->qiov.size);
+		if (memcmp(ctx->buf, cmp_buf, ctx->qiov.size)) {
 			printf("Pattern verification failed at offset %lld, "
 				"%d bytes\n",
-				(long long) ctx->offset, total);
+				(long long) ctx->offset, ctx->qiov.size);
 		}
 		free(cmp_buf);
 	}
 
-	if (ctx->qflag)
+	if (ctx->qflag) {
 		return;
+	}
 
-        if (ctx->vflag)
-		dump_buffer(ctx->buf, ctx->offset, total);
+	if (ctx->vflag) {
+		dump_buffer(ctx->buf, ctx->offset, ctx->qiov.size);
+	}
 
 	/* Finally, report back -- -C gives a parsable format */
 	t2 = tsub(t2, ctx->t1);
-	print_report("read", &t2, ctx->offset, ctx->qiov.size, total, cnt,
-		     ctx->Cflag);
+	print_report("read", &t2, ctx->offset, ctx->qiov.size,
+		     ctx->qiov.size, 1, ctx->Cflag);
 
 	qemu_io_free(ctx->buf);
 	free(ctx);
-
 }
 
 static void
@@ -870,8 +865,6 @@ aio_read_f(int argc, char **argv)
 	struct aio_ctx *ctx = calloc(1, sizeof(struct aio_ctx));
 	BlockDriverAIOCB *acb;
 
-	ctx->pattern = 0xcd;
-
 	while ((c = getopt(argc, argv, "CP:qv")) != EOF) {
 		switch (c) {
 		case 'C':
@@ -1035,7 +1028,6 @@ aio_write_f(int argc, char **argv)
 		return 0;
 	}
 
-
 	for (i = optind; i < argc; i++) {
 	        size_t len;
 

Re: [Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[Anthony Liguori]

Christoph Hellwig wrote:
>  	if (ctx->Pflag) {
> -		void *cmp_buf = malloc(total);
> +		void *cmp_buf = malloc(ctx->qiov.size);
>  
> -		memset(cmp_buf, ctx->pattern, total);
> -		if (memcmp(ctx->buf, cmp_buf, total)) {
> +		memset(cmp_buf, ctx->pattern, ctx->qiov.size);
> +		if (memcmp(ctx->buf, cmp_buf, ctx->qiov.size)) {
>  			printf("Pattern verification failed at offset %lld, "
>  				"%d bytes\n",
> -				(long long) ctx->offset, total);
> +				(long long) ctx->offset, ctx->qiov.size);
>   

This breaks the build because ctx->qiov.size is a size_t and your format 
parameter is a %d.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[Christoph Hellwig]

On Thu, Jul 09, 2009 at 03:02:59PM -0500, Anthony Liguori wrote:
> >-		if (memcmp(ctx->buf, cmp_buf, total)) {
> >+		memset(cmp_buf, ctx->pattern, ctx->qiov.size);
> >+		if (memcmp(ctx->buf, cmp_buf, ctx->qiov.size)) {
> > 			printf("Pattern verification failed at offset %lld, "
> > 				"%d bytes\n",
> >-				(long long) ctx->offset, total);
> >+				(long long) ctx->offset, ctx->qiov.size);
> >  
> 
> This breaks the build because ctx->qiov.size is a size_t and your format 
> parameter is a %d.

Work fine on 32 bit x86 :)  But yeah, the correct format that works on
all platforms would be %zd.

Re: [Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[malc]

On Fri, 10 Jul 2009, Christoph Hellwig wrote:

> On Thu, Jul 09, 2009 at 03:02:59PM -0500, Anthony Liguori wrote:
> > >-		if (memcmp(ctx->buf, cmp_buf, total)) {
> > >+		memset(cmp_buf, ctx->pattern, ctx->qiov.size);
> > >+		if (memcmp(ctx->buf, cmp_buf, ctx->qiov.size)) {
> > > 			printf("Pattern verification failed at offset %lld, "
> > > 				"%d bytes\n",
> > >-				(long long) ctx->offset, total);
> > >+				(long long) ctx->offset, ctx->qiov.size);
> > >  
> > 
> > This breaks the build because ctx->qiov.size is a size_t and your format 
> > parameter is a %d.
> 
> Work fine on 32 bit x86 :)  But yeah, the correct format that works on
> all platforms would be %zd.

Just a small correction - it wouldn't work on Windows.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[Christoph Hellwig]

On Fri, Jul 10, 2009 at 02:20:07PM +0400, malc wrote:
> > Work fine on 32 bit x86 :)  But yeah, the correct format that works on
> > all platforms would be %zd.
> 
> Just a small correction - it wouldn't work on Windows.

Why would it generally not work on windows?  %zd is in C99 and thus it
should be supported by any recent Windows compiler and library.

Re: [Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[Jamie Lokier]

Christoph Hellwig wrote:
> On Fri, Jul 10, 2009 at 02:20:07PM +0400, malc wrote:
> > > Work fine on 32 bit x86 :)  But yeah, the correct format that works on
> > > all platforms would be %zd.
> > 
> > Just a small correction - it wouldn't work on Windows.
> 
> Why would it generally not work on windows?  %zd is in C99 and thus it
> should be supported by any recent Windows compiler and library.

Windows compilers don't generally support C99.

-- Jamie

Re: [Qemu-devel] [PATCH 1/2] qemu-io: small cleanups for the aio code

[malc]

On Fri, 10 Jul 2009, Christoph Hellwig wrote:

> On Fri, Jul 10, 2009 at 02:20:07PM +0400, malc wrote:
> > > Work fine on 32 bit x86 :)  But yeah, the correct format that works on
> > > all platforms would be %zd.
> > 
> > Just a small correction - it wouldn't work on Windows.
> 
> Why would it generally not work on windows?  %zd is in C99 and thus it
> should be supported by any recent Windows compiler and library.

GCC the compiler does support it, the mingw32's runtime library though
does not.

-- 
mailto:av1474@comtv.ru