Re: [Qemu-block] block migration and dirty bitmap reset
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, &blk->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-block] block migration and dirty bitmap reset
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, >>> &blk->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-block] block migration and dirty bitmap reset
> 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, &blk->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-block] block migration and dirty bitmap reset
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, &blk->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