[Qemu-devel] Re: [PATCH] blkverify: Add block driver for verifying I/O

[Kevin Wolf]

Am 03.09.2010 13:55, schrieb Stefan Hajnoczi:
> The blkverify block driver makes investigating image format data
> corruption much easier.  A raw image initialized with the same contents
> as the test image (e.g. qcow2 file) must be provided.  The raw image
> mirrors read/write operations and is used to verify that data read from
> the test image is correct.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> Please see docs/blkverify.txt below for a full description.
> 
>  Makefile.objs      |    2 +-
>  block/blkverify.c  |  309 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  docs/blkverify.txt |   69 ++++++++++++
>  3 files changed, 379 insertions(+), 1 deletions(-)
>  create mode 100644 block/blkverify.c
>  create mode 100644 docs/blkverify.txt
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index b25f573..40a4594 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -14,7 +14,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
>  
>  block-nested-y += raw.o cow.o cow2.o qcow.o vdi.o vmdk.o cloop.o dmg.o 
> bochs.o vpc.o vvfat.o
>  block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o
> -block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o
> +block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
>  block-nested-$(CONFIG_WIN32) += raw-win32.o
>  block-nested-$(CONFIG_POSIX) += raw-posix.o
>  block-nested-$(CONFIG_CURL) += curl.o

This hunk doesn't apply, there is no cow2. :-)

> diff --git a/block/blkverify.c b/block/blkverify.c
> new file mode 100644
> index 0000000..9cebafe
> --- /dev/null
> +++ b/block/blkverify.c
> @@ -0,0 +1,309 @@
> +/*
> + * Block protocol for block driver correctness testing
> + *
> + * Copyright (C) 2010 IBM, Corp.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include <stdarg.h>
> +#include "block_int.h"
> +
> +typedef struct {
> +    BlockDriverState *test_file;
> +} BDRVBlkverifyState;
> +
> +typedef struct BlkverifyAIOCB BlkverifyAIOCB;
> +struct BlkverifyAIOCB {
> +    BlockDriverAIOCB common;
> +    QEMUBH *bh;
> +
> +    /* Request metadata */
> +    bool is_write;
> +    int64_t sector_num;
> +    int nb_sectors;
> +
> +    int ret;                    /* first completed request's result */
> +    unsigned int done;          /* completion counter */
> +
> +    QEMUIOVector *qiov;         /* user I/O vector */
> +    QEMUIOVector raw_qiov;      /* cloned I/O vector for raw file */
> +    void *buf;                  /* buffer for raw file I/O */
> +
> +    void (*verify)(BlkverifyAIOCB *acb);
> +};
> +
> +static void blkverify_aio_cancel(BlockDriverAIOCB *acb)
> +{
> +    qemu_aio_release(acb);
> +}

I would be surprised if this implementation didn't segfault in one way
or another.

I think you'd better cancel the two requests that are combined in this
acb. Or wait for their completion actually because you want to have both
in the same state.

Other than that the patch looks good me.

Kevin