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 <kw...@redhat.com> 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
