Re: [Qemu-devel] [PATCH 3/9] block: fix spoiling all dirty bitmaps by mirror and migration

2015-01-08 Thread John Snow



On 12/11/2014 09:17 AM, 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
CC: John Snow js...@redhat.com
CC: Fam Zheng f...@redhat.com
CC: Denis V. Lunev d...@openvz.org
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Kevin Wolf kw...@redhat.com
---
  block-migration.c |  5 +++--
  block.c   | 23 ---
  block/mirror.c| 11 +++
  include/block/block.h |  6 --
  4 files changed, 34 insertions(+), 11 deletions(-)


[snip]

A version of this already went in upstream, so it can be dropped here now.



[Qemu-devel] [PATCH 3/9] block: fix spoiling all dirty bitmaps by mirror and migration

2014-12-11 Thread Vladimir Sementsov-Ogievskiy
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
CC: John Snow js...@redhat.com
CC: Fam Zheng f...@redhat.com
CC: Denis V. Lunev d...@openvz.org
CC: Stefan Hajnoczi stefa...@redhat.com
CC: Kevin Wolf kw...@redhat.com
---
 block-migration.c |  5 +++--
 block.c   | 23 ---
 block/mirror.c| 11 +++
 include/block/block.h |  6 --
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 8df30d9..5b4aa0f 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,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 a37dc2f..6edf1dc 100644
--- a/block.c
+++ b/block.c
@@ -101,6 +101,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;
 
@@ -5495,8 +5499,20 @@ void bdrv_dirty_iter_set(HBitmapIter *hbi, int64_t 
offset)
 hbitmap_iter_init(hbi, hbi-hb, offset);
 }
 
-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) {
@@ -5507,7 +5523,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 af91ae0..f22ebd4 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