| Message ID | 1421080265-2228-2-git-send-email-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
it is already in master and can be dropped here Best regards, Vladimir On 12.01.2015 19:30, John Snow wrote: > From: Vladimir Sementsov-Ogievskiy <vsementsov@parallels.com> > > 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 <vsementsov@parallels.com> > CC: John Snow <jsnow@redhat.com> > CC: Fam Zheng <famz@redhat.com> > CC: Denis V. Lunev <den@openvz.org> > CC: Stefan Hajnoczi <stefanha@redhat.com> > CC: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: John Snow <jsnow@redhat.com> > --- > block.c | 23 ++++++++++++++++++++--- > block/mirror.c | 11 +++++++---- > include/block/block.h | 6 ++++-- > migration/block.c | 5 +++-- > 4 files changed, 34 insertions(+), 11 deletions(-) > > diff --git a/block.c b/block.c > index 4165d42..6c1b49a 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; > > @@ -5389,8 +5393,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) { > @@ -5398,7 +5414,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..9019d1b 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -128,7 +128,8 @@ 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 +146,8 @@ 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); > action = mirror_error_action(s, true, -ret); > if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) { > s->ret = ret; > @@ -286,7 +288,8 @@ 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 +445,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 6e7275d..a77ed30 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -434,8 +434,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); > diff --git a/migration/block.c b/migration/block.c > index 74d9eb1..a0f908c 100644 > --- a/migration/block.c > +++ b/migration/block.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,8 @@ 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 4165d42..6c1b49a 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; @@ -5389,8 +5393,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) { @@ -5398,7 +5414,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..9019d1b 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -128,7 +128,8 @@ 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 +146,8 @@ 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); action = mirror_error_action(s, true, -ret); if (action == BLOCK_ERROR_ACTION_REPORT && s->ret >= 0) { s->ret = ret; @@ -286,7 +288,8 @@ 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 +445,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 6e7275d..a77ed30 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -434,8 +434,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); diff --git a/migration/block.c b/migration/block.c index 74d9eb1..a0f908c 100644 --- a/migration/block.c +++ b/migration/block.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,8 @@ 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;