From patchwork Wed Jan 20 06:11:38 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Fam Zheng X-Patchwork-Id: 8068711 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id DA22BBEEE5 for ; Wed, 20 Jan 2016 06:15:32 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id EFC3F20373 for ; Wed, 20 Jan 2016 06:15:31 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id D5A6B20351 for ; Wed, 20 Jan 2016 06:15:30 +0000 (UTC) Received: from localhost ([::1]:40305 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLm3G-0002UT-2u for patchwork-qemu-devel@patchwork.kernel.org; Wed, 20 Jan 2016 01:15:30 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34810) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLm0L-0005bU-HU for qemu-devel@nongnu.org; Wed, 20 Jan 2016 01:12:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aLm0K-0004OJ-3I for qemu-devel@nongnu.org; Wed, 20 Jan 2016 01:12:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56487) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aLm0G-0004Mo-Cw; Wed, 20 Jan 2016 01:12:24 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (Postfix) with ESMTPS id 23F0AC0AA563; Wed, 20 Jan 2016 06:12:24 +0000 (UTC) Received: from fam-t430.nay.redhat.com. (dhcp-15-42.nay.redhat.com [10.66.15.42]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u0K6BlW3014641; Wed, 20 Jan 2016 01:12:19 -0500 From: Fam Zheng To: qemu-devel@nongnu.org Date: Wed, 20 Jan 2016 14:11:38 +0800 Message-Id: <1453270306-16608-6-git-send-email-famz@redhat.com> In-Reply-To: <1453270306-16608-1-git-send-email-famz@redhat.com> References: <1453270306-16608-1-git-send-email-famz@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: Kevin Wolf , Jeff Cody , Vladimir Sementsov-Ogievskiy , jsnow@redhat.com, qemu-block@nongnu.org Subject: [Qemu-devel] [PATCH v2 05/13] block: Hide HBitmap in block dirty bitmap interface X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP HBitmap is an implementation detail of block dirty bitmap that should be hidden from users. Introduce a BdrvDirtyBitmapIter to encapsulate the underlying HBitmapIter. A small difference in the interface is, before, an HBitmapIter is initialized in place, now the new BdrvDirtyBitmapIter must be dynamically allocated because the structure definition is in block.c. Two current users are converted too. Signed-off-by: Fam Zheng Reviewed-by: John Snow --- block/backup.c | 14 ++++++++------ block/dirty-bitmap.c | 37 ++++++++++++++++++++++++++++++++----- block/mirror.c | 14 ++++++++------ include/block/dirty-bitmap.h | 7 +++++-- include/qemu/typedefs.h | 1 + 5 files changed, 54 insertions(+), 19 deletions(-) diff --git a/block/backup.c b/block/backup.c index 56ddec0..2388039 100644 --- a/block/backup.c +++ b/block/backup.c @@ -326,14 +326,14 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) int64_t end; int64_t last_cluster = -1; BlockDriverState *bs = job->common.bs; - HBitmapIter hbi; + BdrvDirtyBitmapIter *dbi; granularity = bdrv_dirty_bitmap_granularity(job->sync_bitmap); clusters_per_iter = MAX((granularity / BACKUP_CLUSTER_SIZE), 1); - bdrv_dirty_iter_init(job->sync_bitmap, &hbi); + dbi = bdrv_dirty_iter_new(job->sync_bitmap, 0); /* Find the next dirty sector(s) */ - while ((sector = hbitmap_iter_next(&hbi)) != -1) { + while ((sector = bdrv_dirty_iter_next(dbi)) != -1) { cluster = sector / BACKUP_SECTORS_PER_CLUSTER; /* Fake progress updates for any clusters we skipped */ @@ -345,7 +345,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) for (end = cluster + clusters_per_iter; cluster < end; cluster++) { do { if (yield_and_check(job)) { - return ret; + goto out; } ret = backup_do_cow(bs, cluster * BACKUP_SECTORS_PER_CLUSTER, BACKUP_SECTORS_PER_CLUSTER, &error_is_read, @@ -353,7 +353,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) if ((ret < 0) && backup_error_action(job, error_is_read, -ret) == BLOCK_ERROR_ACTION_REPORT) { - return ret; + goto out; } } while (ret < 0); } @@ -361,7 +361,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) /* If the bitmap granularity is smaller than the backup granularity, * we need to advance the iterator pointer to the next cluster. */ if (granularity < BACKUP_CLUSTER_SIZE) { - bdrv_set_dirty_iter(&hbi, cluster * BACKUP_SECTORS_PER_CLUSTER); + bdrv_set_dirty_iter(dbi, cluster * BACKUP_SECTORS_PER_CLUSTER); } last_cluster = cluster - 1; @@ -373,6 +373,8 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) job->common.offset += ((end - last_cluster - 1) * BACKUP_CLUSTER_SIZE); } +out: + bdrv_dirty_iter_free(dbi); return ret; } diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 7924c38..bd7758b 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -41,9 +41,15 @@ struct BdrvDirtyBitmap { char *name; /* Optional non-empty unique ID */ int64_t size; /* Size of the bitmap (Number of sectors) */ bool disabled; /* Bitmap is read-only */ + int active_iterators; /* How many iterators are active */ QLIST_ENTRY(BdrvDirtyBitmap) list; }; +struct BdrvDirtyBitmapIter { + HBitmapIter hbi; + BdrvDirtyBitmap *bitmap; +}; + BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name) { BdrvDirtyBitmap *bm; @@ -211,6 +217,7 @@ void bdrv_dirty_bitmap_truncate(BlockDriverState *bs) QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) { assert(!bdrv_dirty_bitmap_frozen(bitmap)); + assert(!bitmap->active_iterators); hbitmap_truncate(bitmap->bitmap, size); bitmap->size = size; } @@ -221,6 +228,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) BdrvDirtyBitmap *bm, *next; QLIST_FOREACH_SAFE(bm, &bs->dirty_bitmaps, list, next) { if (bm == bitmap) { + assert(!bitmap->active_iterators); assert(!bdrv_dirty_bitmap_frozen(bm)); QLIST_REMOVE(bitmap, list); hbitmap_free(bitmap->bitmap); @@ -299,9 +307,29 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap) return BDRV_SECTOR_SIZE << hbitmap_granularity(bitmap->bitmap); } -void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, HBitmapIter *hbi) +BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap, + uint64_t first_sector) { - hbitmap_iter_init(hbi, bitmap->bitmap, 0); + BdrvDirtyBitmapIter *iter = g_new(BdrvDirtyBitmapIter, 1); + hbitmap_iter_init(&iter->hbi, bitmap->bitmap, first_sector); + iter->bitmap = bitmap; + bitmap->active_iterators++; + return iter; +} + +void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter) +{ + if (!iter) { + return; + } + assert(iter->bitmap->active_iterators > 0); + iter->bitmap->active_iterators--; + g_free(iter); +} + +int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter) +{ + return hbitmap_iter_next(&iter->hbi); } void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, @@ -354,10 +382,9 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector, /** * Advance an HBitmapIter to an arbitrary offset. */ -void bdrv_set_dirty_iter(HBitmapIter *hbi, int64_t offset) +void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *iter, int64_t sector_num) { - assert(hbi->hb); - hbitmap_iter_init(hbi, hbi->hb, offset); + hbitmap_iter_init(&iter->hbi, iter->bitmap->bitmap, sector_num); } int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap) diff --git a/block/mirror.c b/block/mirror.c index f201f2b..70ca844 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -52,7 +52,7 @@ typedef struct MirrorBlockJob { int64_t bdev_length; unsigned long *cow_bitmap; BdrvDirtyBitmap *dirty_bitmap; - HBitmapIter hbi; + BdrvDirtyBitmapIter *dbi; uint8_t *buf; QSIMPLEQ_HEAD(, MirrorBuffer) buf_free; int buf_free_count; @@ -170,10 +170,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) max_iov = MIN(source->bl.max_iov, s->target->bl.max_iov); - s->sector_num = hbitmap_iter_next(&s->hbi); + s->sector_num = bdrv_dirty_iter_next(s->dbi); if (s->sector_num < 0) { - bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi); - s->sector_num = hbitmap_iter_next(&s->hbi); + bdrv_dirty_iter_free(s->dbi); + s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0); + s->sector_num = bdrv_dirty_iter_next(s->dbi); trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap)); assert(s->sector_num >= 0); } @@ -291,7 +292,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) */ if (next_sector > hbitmap_next_sector && bdrv_get_dirty(source, s->dirty_bitmap, next_sector)) { - hbitmap_next_sector = hbitmap_iter_next(&s->hbi); + hbitmap_next_sector = bdrv_dirty_iter_next(s->dbi); } next_sector += sectors_per_chunk; @@ -502,7 +503,7 @@ static void coroutine_fn mirror_run(void *opaque) } } - bdrv_dirty_iter_init(s->dirty_bitmap, &s->hbi); + s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0); for (;;) { uint64_t delay_ns = 0; int64_t cnt; @@ -615,6 +616,7 @@ immediate_exit: qemu_vfree(s->buf); g_free(s->cow_bitmap); g_free(s->in_flight_bitmap); + bdrv_dirty_iter_free(s->dbi); bdrv_release_dirty_bitmap(bs, s->dirty_bitmap); if (s->target->blk) { blk_iostatus_disable(s->target->blk); diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index 02a9337..120bac6 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -35,8 +35,11 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t cur_sector, int nr_sectors); void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, int64_t cur_sector, int nr_sectors); -void bdrv_dirty_iter_init(BdrvDirtyBitmap *bitmap, struct HBitmapIter *hbi); -void bdrv_set_dirty_iter(struct HBitmapIter *hbi, int64_t offset); +BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap, + uint64_t first_sector); +void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter); +int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter); +void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num); int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap); void bdrv_dirty_bitmap_truncate(BlockDriverState *bs); diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index 0cf9d74..a1bdb5a 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -11,6 +11,7 @@ typedef struct AioContext AioContext; typedef struct AllwinnerAHCIState AllwinnerAHCIState; typedef struct AudioState AudioState; typedef struct BdrvDirtyBitmap BdrvDirtyBitmap; +typedef struct BdrvDirtyBitmapIter BdrvDirtyBitmapIter; typedef struct BlockBackend BlockBackend; typedef struct BlockBackendRootState BlockBackendRootState; typedef struct BlockDriverState BlockDriverState;