Re: [Qemu-devel] block migration and dirty bitmap reset

[Peter Lieven]

Am 08.03.2018 um 10:01 schrieb Fam Zheng:

On Thu, Mar 8, 2018 at 4:57 PM, Peter Lieven  wrote:



Am 08.03.2018 um 02:28 schrieb Fam Zheng :


On Wed, 03/07 09:06, Peter Lieven wrote:
Hi,

while looking at the code I wonder if the blk_aio_preadv and the 
bdrv_reset_dirty_bitmap order must
be swapped in mig_save_device_bulk:

qemu_mutex_lock_iothread();
aio_context_acquire(blk_get_aio_context(bmds->blk));
blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, >qiov,
0, blk_mig_read_cb, blk);

bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * BDRV_SECTOR_SIZE,
nr_sectors * BDRV_SECTOR_SIZE);
aio_context_release(blk_get_aio_context(bmds->blk));
qemu_mutex_unlock_iothread();

In mig_save_device_dirty we first reset the dirty bitmap and read then which 
shoulds like
a better idea.

Yes, that sounds right to me.

Fam

You mean the order should be swapped in mig_save_device_bulk as well?

Yeah, resetting the dirty bitmap before reading makes sure we don't
miss any new data.


I wonder if this can really happen during blk_aio_preadv?
Anyway better safe than sorry. It at least will not hurt to swap the order.

Peter

Re: [Qemu-devel] block migration and dirty bitmap reset

[Fam Zheng]

On Thu, Mar 8, 2018 at 4:57 PM, Peter Lieven  wrote:
>
>
>> Am 08.03.2018 um 02:28 schrieb Fam Zheng :
>>
>>> On Wed, 03/07 09:06, Peter Lieven wrote:
>>> Hi,
>>>
>>> while looking at the code I wonder if the blk_aio_preadv and the 
>>> bdrv_reset_dirty_bitmap order must
>>> be swapped in mig_save_device_bulk:
>>>
>>>qemu_mutex_lock_iothread();
>>>aio_context_acquire(blk_get_aio_context(bmds->blk));
>>>blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, 
>>> >qiov,
>>>0, blk_mig_read_cb, blk);
>>>
>>>bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * 
>>> BDRV_SECTOR_SIZE,
>>>nr_sectors * BDRV_SECTOR_SIZE);
>>>aio_context_release(blk_get_aio_context(bmds->blk));
>>>qemu_mutex_unlock_iothread();
>>>
>>> In mig_save_device_dirty we first reset the dirty bitmap and read then 
>>> which shoulds like
>>> a better idea.
>>
>> Yes, that sounds right to me.
>>
>> Fam
>
> You mean the order should be swapped in mig_save_device_bulk as well?

Yeah, resetting the dirty bitmap before reading makes sure we don't
miss any new data.

Fam

>
> Peter
>

Re: [Qemu-devel] block migration and dirty bitmap reset

[Peter Lieven]

> Am 08.03.2018 um 02:28 schrieb Fam Zheng :
> 
>> On Wed, 03/07 09:06, Peter Lieven wrote:
>> Hi,
>> 
>> while looking at the code I wonder if the blk_aio_preadv and the 
>> bdrv_reset_dirty_bitmap order must
>> be swapped in mig_save_device_bulk:
>> 
>>qemu_mutex_lock_iothread();
>>aio_context_acquire(blk_get_aio_context(bmds->blk));
>>blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, >qiov,
>>0, blk_mig_read_cb, blk);
>> 
>>bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * BDRV_SECTOR_SIZE,
>>nr_sectors * BDRV_SECTOR_SIZE);
>>aio_context_release(blk_get_aio_context(bmds->blk));
>>qemu_mutex_unlock_iothread();
>> 
>> In mig_save_device_dirty we first reset the dirty bitmap and read then which 
>> shoulds like
>> a better idea.
> 
> Yes, that sounds right to me.
> 
> Fam

You mean the order should be swapped in mig_save_device_bulk as well?

Peter

Re: [Qemu-devel] block migration and dirty bitmap reset

[Fam Zheng]

On Wed, 03/07 09:06, Peter Lieven wrote:
> Hi,
> 
> while looking at the code I wonder if the blk_aio_preadv and the 
> bdrv_reset_dirty_bitmap order must
> be swapped in mig_save_device_bulk:
> 
> qemu_mutex_lock_iothread();
> aio_context_acquire(blk_get_aio_context(bmds->blk));
> blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, >qiov,
> 0, blk_mig_read_cb, blk);
> 
> bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * BDRV_SECTOR_SIZE,
> nr_sectors * BDRV_SECTOR_SIZE);
> aio_context_release(blk_get_aio_context(bmds->blk));
> qemu_mutex_unlock_iothread();
> 
> In mig_save_device_dirty we first reset the dirty bitmap and read then which 
> shoulds like
> a better idea.

Yes, that sounds right to me.

Fam

[Qemu-devel] block migration and dirty bitmap reset

[Peter Lieven]

Hi,

while looking at the code I wonder if the blk_aio_preadv and the 
bdrv_reset_dirty_bitmap order must
be swapped in mig_save_device_bulk:

qemu_mutex_lock_iothread();
aio_context_acquire(blk_get_aio_context(bmds->blk));
blk->aiocb = blk_aio_preadv(bb, cur_sector * BDRV_SECTOR_SIZE, >qiov,
0, blk_mig_read_cb, blk);

bdrv_reset_dirty_bitmap(bmds->dirty_bitmap, cur_sector * BDRV_SECTOR_SIZE,
nr_sectors * BDRV_SECTOR_SIZE);
aio_context_release(blk_get_aio_context(bmds->blk));
qemu_mutex_unlock_iothread();

In mig_save_device_dirty we first reset the dirty bitmap and read then which 
shoulds like
a better idea. Maybe it doesn't matter while we acquire the aioctx and the 
iothread lock...

Peter