Re: [Qemu-devel] [PATCH v4 1/3] qmp: Add command 'blockdev-backup'

[Markus Armbruster <armbru@redhat.com>]

Max Reitz <mreitz@redhat.com> writes:

> On 2014-12-04 at 03:29, Fam Zheng wrote:
>> Similar to drive-backup, but this command uses a device id as target
>> instead of creating/opening an image file.
>>
>> Also add blocker on target bs, since the target is also a named device
>> now.
>>
>> Add check and report error for bs == target which became possible but is
>> an illegal case with introduction of blockdev-backup.
[...]
>> diff --git a/blockdev.c b/blockdev.c
>> index 5651a8e..f44441a 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
[...]
>> +    if (!bs) {
>> +        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
>> +        return;
>> +    }
>> +
>> +    target_bs = bdrv_find(target);
>> +    if (!target_bs) {
>> +        error_set(errp, QERR_DEVICE_NOT_FOUND, target);
>> +        return;
>> +    }
>> +
>> +    bdrv_ref(target_bs);
>> +    bdrv_set_aio_context(target_bs, bdrv_get_aio_context(bs));
>
> In the cover letter you said you were acquiring the AIO context but
> you're not. Something like the aio_context_acquire() call in
> qmp_drive_backup() seems missing.

The fact that I missed this in my review demonstrates that I have to pay
much more attention to AIO contexts.  Thanks, Max!