Re: [Qemu-devel] [PATCH 2/3] virtio-blk: Use blk_drain() to drain IO requests

[Christian Borntraeger <borntraeger@de.ibm.com>]

Am 11.06.2015 um 04:51 schrieb Fam Zheng:
> On Wed, 06/10 14:38, Alexander Yarygin wrote:
>> Each call of the virtio_blk_reset() function calls blk_drain_all(),
>> which works for all existing BlockDriverStates, while draining only
>> one is needed.
>>
>> This patch replaces blk_drain_all() by blk_drain() in virtio_blk_reset().
> 
> Please add a note "virtio_blk_data_plane_stop should be called after draining
> because it restores vblk->complete_request" as well.
>>
>> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> 
> Cc: qemu-stable@nongnu.org

Only for the move of virtio_blk_data_plane_stop? Are we sure that we can
call blk_drain safely on old version?

What about the following:
respin without RFC
patch 1: move  virtio_blk_data_plane_stop, remove comment.  cc stable
patch 2: introduce blk_drain
patch 3: blk_drain_all -> blk_drain
patch 4: As RFC  "Let bdrv_drain_all() to call aio_poll() for each AioContext"

So that Stefan/Kevin can apply 1-3. and we can then review 4.

Makes sense?

Christian


> 
>> Signed-off-by: Alexander Yarygin <yarygin@linux.vnet.ibm.com>
>> ---
>>  hw/block/virtio-blk.c | 11 ++++++-----
>>  1 file changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
>> index e6afe97..2009092 100644
>> --- a/hw/block/virtio-blk.c
>> +++ b/hw/block/virtio-blk.c
>> @@ -652,15 +652,16 @@ static void virtio_blk_reset(VirtIODevice *vdev)
>>  {
>>      VirtIOBlock *s = VIRTIO_BLK(vdev);
>>  
>> -    if (s->dataplane) {
>> -        virtio_blk_data_plane_stop(s->dataplane);
>> -    }
>> -
>>      /*
>>       * This should cancel pending requests, but can't do nicely until there
>>       * are per-device request lists.
>>       */
> 
> This comment can be dropped now.
> 
>> -    blk_drain_all();
>> +    blk_drain(s->blk);
>> +
>> +    if (s->dataplane) {
>> +        virtio_blk_data_plane_stop(s->dataplane);
>> +    }
>> +
>>      blk_set_enable_write_cache(s->blk, s->original_wce);
>>  }
>>  
>> -- 
>> 1.9.1
>>
>>
>