Am 04.09.2010 17:34, 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 <stefa...@linux.vnet.ibm.com>
> ---
> This is embarassing, I sent out the v2 patch instead of the v3 patch the first
> time!
>
> v3:
> * Fix compile error in blkverify_aio_cancel()
>
> v2:
> * Implement aio_cancel() by waiting for pending requests
> * Fix conflict in Makefile.objs
>
> Makefile.objs | 2 +-
> block/blkverify.c | 324
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> docs/blkverify.txt | 69 +++++++++++
> 3 files changed, 394 insertions(+), 1 deletions(-)
> create mode 100644 block/blkverify.c
> create mode 100644 docs/blkverify.txt
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 4a1eaa1..b640ec8 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 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
> diff --git a/block/blkverify.c b/block/blkverify.c
> new file mode 100644
> index 0000000..97717a6
> --- /dev/null
> +++ b/block/blkverify.c
> @@ -0,0 +1,324 @@
> +/*
> + * 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_cb(void *opaque, int ret)
> +{
> + *(bool *)opaque = true;
> +}
> +
> +static void blkverify_aio_cancel(BlockDriverAIOCB *acb)
> +{
> + bool done = false;
> +
> + /* Allow the request to complete so the raw file and test file stay in
> + * sync. Hijack the callback (since the request is cancelled anyway) to
> + * wait for completion.
> + */
The approach of completing the request sounds okay, but I think you need
to call the original callback if you let it complete.
> + acb->cb = blkverify_aio_cancel_cb;
> + acb->opaque = &done;
> + while (!done) {
> + qemu_aio_wait();
> + }
qemu_aio_release(acb)?
> +}
> +
> +static AIOPool blkverify_aio_pool = {
> + .aiocb_size = sizeof(BlkverifyAIOCB),
> + .cancel = blkverify_aio_cancel,
> +};
> +
> +static void blkverify_err(BlkverifyAIOCB *acb, const char *fmt, ...)
> +{
> + va_list ap;
> +
> + va_start(ap, fmt);
> + fprintf(stderr, "blkverify: %s sector_num=%ld nb_sectors=%d ",
> + acb->is_write ? "write" : "read", acb->sector_num,
> + acb->nb_sectors);
> + vfprintf(stderr, fmt, ap);
> + fprintf(stderr, "\n");
> + va_end(ap);
> + exit(1);
> +}
> +
> +/* Valid blkverify filenames look like
> blkverify:path/to/raw_image:path/to/image */
> +static int blkverify_open(BlockDriverState *bs, const char *filename, int
> flags)
> +{
> + BDRVBlkverifyState *s = bs->opaque;
> + int ret;
> + char *raw, *c;
> +
> + /* Parse the blkverify: prefix */
> + if (strncmp(filename, "blkverify:", strlen("blkverify:"))) {
> + return -EINVAL;
> + }
> + filename += strlen("blkverify:");
> +
> + /* Parse the raw image filename */
> + c = strchr(filename, ':');
> + if (c == NULL) {
> + return -EINVAL;
> + }
> +
> + raw = strdup(filename);
> + raw[c - filename] = '\0';
> + ret = bdrv_file_open(&bs->file, raw, flags);
> + free(raw);
> + if (ret < 0) {
> + return ret;
> + }
> + filename = c + 1;
> +
> + /* Open the test file */
> + s->test_file = bdrv_new("");
> + ret = bdrv_open(s->test_file, filename, flags, NULL);
> + if (ret < 0) {
> + return ret;
This leaks bs->file.
Kevin
