[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/3] qmp: Add command 'blockdev-backup'
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4 1/3] qmp: Add command 'blockdev-backup' |
Date: |
Fri, 19 Dec 2014 09:47:03 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Max Reitz <address@hidden> 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!