On Wed, 11/26 12:23, Vladimir Sementsov-Ogievskiy wrote: > Mirror and migration use dirty bitmaps for their purposes, and since > commit [block: per caller dirty bitmap] they use their own bitmaps, not > the global one. But they use old functions bdrv_set_dirty and > bdrv_reset_dirty, which change all dirty bitmaps. > > Named dirty bitmaps series by Fam and Snow are affected: mirroring and > migration will spoil all (not related to this mirroring or migration) > named dirty bitmaps. > > This patch fixes this by adding bdrv_set_dirty_bitmap and > bdrv_reset_dirty_bitmap, which change concrete bitmap. Also, to prevent > such mistakes in future, old functions bdrv_(set,reset)_dirty are made > static, for internal block usage. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@parallels.com>
Looks good to me except for a few lines are over 80 characters... > --- > block-migration.c | 4 ++-- > block.c | 23 ++++++++++++++++++++--- > block/mirror.c | 8 ++++---- > include/block/block.h | 6 ++++-- > 4 files changed, 30 insertions(+), 11 deletions(-) > > diff --git a/block-migration.c b/block-migration.c > index 08db01a..2b077ad 100644 > --- a/block-migration.c > +++ b/block-migration.c > @@ -303,7 +303,7 @@ static int mig_save_device_bulk(QEMUFile *f, > BlkMigDevState *bmds) > blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov, > nr_sectors, blk_mig_read_cb, blk); > > - bdrv_reset_dirty(bs, cur_sector, nr_sectors); > + bdrv_reset_dirty_bitmap(bs, bmds->dirty_bitmap, cur_sector, nr_sectors); > qemu_mutex_unlock_iothread(); > > bmds->cur_sector = cur_sector + nr_sectors; > @@ -496,7 +496,7 @@ static int mig_save_device_dirty(QEMUFile *f, > BlkMigDevState *bmds, > g_free(blk); > } > > - bdrv_reset_dirty(bmds->bs, sector, nr_sectors); > + bdrv_reset_dirty_bitmap(bmds->bs, bmds->dirty_bitmap, sector, > nr_sectors); (*) > break; > } > sector += BDRV_SECTORS_PER_DIRTY_CHUNK; > diff --git a/block.c b/block.c > index a612594..4d12c0d 100644 > --- a/block.c > +++ b/block.c > @@ -97,6 +97,10 @@ static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states = > static QLIST_HEAD(, BlockDriver) bdrv_drivers = > QLIST_HEAD_INITIALIZER(bdrv_drivers); > > +static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, > + int nr_sectors); > +static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, > + int nr_sectors); > /* If non-zero, use only whitelisted block drivers */ > static int use_bdrv_whitelist; > > @@ -5361,8 +5365,20 @@ void bdrv_dirty_iter_init(BlockDriverState *bs, > hbitmap_iter_init(hbi, bitmap->bitmap, 0); > } > > -void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, > - int nr_sectors) > +void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + int64_t cur_sector, int nr_sectors) > +{ > + hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors); > +} > + > +void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + int64_t cur_sector, int nr_sectors) > +{ > + hbitmap_reset(bitmap->bitmap, cur_sector, nr_sectors); > +} > + > +static void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, > + int nr_sectors) > { > BdrvDirtyBitmap *bitmap; > QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { > @@ -5370,7 +5386,8 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t > cur_sector, > } > } > > -void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int > nr_sectors) > +static void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, > + int nr_sectors) > { > BdrvDirtyBitmap *bitmap; > QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { > diff --git a/block/mirror.c b/block/mirror.c > index 2c6dd2a..72011c4 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -128,7 +128,7 @@ static void mirror_write_complete(void *opaque, int ret) > BlockDriverState *source = s->common.bs; > BlockErrorAction action; > > - bdrv_set_dirty(source, op->sector_num, op->nb_sectors); > + bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num, > op->nb_sectors); (*) > action = mirror_error_action(s, false, -ret); > if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) { > s->ret = ret; > @@ -145,7 +145,7 @@ static void mirror_read_complete(void *opaque, int ret) > BlockDriverState *source = s->common.bs; > BlockErrorAction action; > > - bdrv_set_dirty(source, op->sector_num, op->nb_sectors); > + bdrv_set_dirty_bitmap(source, s->dirty_bitmap, op->sector_num, > op->nb_sectors); (*) Fam > action = mirror_error_action(s, true, -ret); > if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) { > s->ret = ret; > @@ -286,7 +286,7 @@ static uint64_t coroutine_fn > mirror_iteration(MirrorBlockJob *s) > next_sector += sectors_per_chunk; > } > > - bdrv_reset_dirty(source, sector_num, nb_sectors); > + bdrv_reset_dirty_bitmap(source, s->dirty_bitmap, sector_num, nb_sectors); > > /* Copy the dirty cluster. */ > s->in_flight++; > @@ -442,7 +442,7 @@ static void coroutine_fn mirror_run(void *opaque) > > assert(n > 0); > if (ret == 1) { > - bdrv_set_dirty(bs, sector_num, n); > + bdrv_set_dirty_bitmap(bs, s->dirty_bitmap, sector_num, n); > sector_num = next; > } else { > sector_num += n; > diff --git a/include/block/block.h b/include/block/block.h > index 5450610..237ddb0 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -433,8 +433,10 @@ BdrvDirtyBitmap > *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity, > void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap > *bitmap); > BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs); > int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t > sector); > -void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, int > nr_sectors); > -void bdrv_reset_dirty(BlockDriverState *bs, int64_t cur_sector, int > nr_sectors); > +void bdrv_set_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + int64_t cur_sector, int nr_sectors); > +void bdrv_reset_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + int64_t cur_sector, int nr_sectors); > void bdrv_dirty_iter_init(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi); > int64_t bdrv_get_dirty_count(BlockDriverState *bs, BdrvDirtyBitmap *bitmap); > -- > 1.9.1 > >