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

[Stefan Hajnoczi]

Thanks for your comments Kevin!

I'd like to merge the overlapping I/O patch I sent today since
blkverify is not in mainline yet.  Are you okay with that or should I
keep them separate?

On Mon, Sep 20, 2010 at 3:48 PM, Kevin Wolf <address@hidden> wrote:
> Am 04.09.2010 17:34, schrieb Stefan Hajnoczi:
>> +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.

Neither posix-aio-compat.c nor block/qcow2.c invoke the callback.  Am
I missing something?

>> +    acb->cb = blkverify_aio_cancel_cb;
>> +    acb->opaque = &done;
>> +    while (!done) {
>> +        qemu_aio_wait();
>> +    }
>
> qemu_aio_release(acb)?

Will fix.

>> +    /* 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.

s->test_file needs bdrv_delete().  block.c:bdrv_open_common() calls
bdrv_delete(bs->file) on failure to close and delete bs->file, but we
don't need to rely on that.  I will fix both.

Stefan