Skip to content

Commit

Permalink
block: fix spoiling all dirty bitmaps by mirror and migration
Browse files Browse the repository at this point in the history
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 <[email protected]>
CC: John Snow <[email protected]>
CC: Fam Zheng <[email protected]>
CC: Denis V. Lunev <[email protected]>
CC: Stefan Hajnoczi <[email protected]>
CC: Kevin Wolf <[email protected]>
Reviewed-by: John Snow <[email protected]>
Reviewed-by: Fam Zheng <[email protected]>
Message-id: [email protected]
Signed-off-by: Max Reitz <[email protected]>
  • Loading branch information
Vladimir Sementsov-Ogievskiy authored and stefanhaRH committed Jan 13, 2015
1 parent a06e435 commit c4237df
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 11 deletions.
23 changes: 20 additions & 3 deletions block.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -5411,16 +5415,29 @@ 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) {
hbitmap_set(bitmap->bitmap, cur_sector, nr_sectors);
}
}

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) {
Expand Down
11 changes: 7 additions & 4 deletions block/mirror.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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++;
Expand Down Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions include/block/block.h
Original file line number Diff line number Diff line change
Expand Up @@ -438,8 +438,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);
Expand Down
5 changes: 3 additions & 2 deletions migration/block.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit c4237df

Please sign in to comment.