qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] block: fix spoiling all dirty bitmaps by mir


From: Fam Zheng
Subject: Re: [Qemu-devel] [PATCH v2] block: fix spoiling all dirty bitmaps by mirror and migration
Date: Tue, 16 Dec 2014 08:57:49 +0800
User-agent: Mutt/1.5.23 (2014-03-12)

On Thu, 11/27 12:40, 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 <address@hidden>
> CC: John Snow <address@hidden>
> CC: Fam Zheng <address@hidden>
> CC: Denis V. Lunev <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> CC: Kevin Wolf <address@hidden>
> ---
> 
> This patch conflicts with
> [PATCH v8 05/10] block: Add bdrv_copy_dirty_bitmap and bdrv_reset_dirty_bitmap
> by John Snow, which introduces bdrv_reset_dirty_bitmap too, but it resets the 
> whole
> bitmap, not specified sectors range.
> 
>  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 08db01a..5866987 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 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..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 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
> 

Reviewed-by: Fam Zheng <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]