make dirty-bitmap byte-based
v3 cover make dirty-bitmap byte-based
There are patches floating around to add NBD_CMD_BLOCK_STATUS, but NBD wants to report status on byte granularity (even if the reporting will probably be naturally aligned to sectors or even much higher levels). I've therefore started the task of converting our block status code to report at a byte granularity rather than sectors.
This is part two of that conversion: dirty-bitmap. Other parts include bdrv_is_allocated (at v3 [1]) and replacing bdrv_get_block_status with a byte based callback in all the drivers (at v1, needs a rebase [3]).
Available as a tag at:
git fetch git://repo.or.cz/qemu/ericb.git nbd-byte-dirty-v3
Depends on Kevin's block branch and my v3 bdrv_is_allocated [1]
Since v2 [2], I had to rebase on top of Paolo's locking fixes; patch v2 2/12 is gone, and many of the others had a lot of context conflicts. But I felt the resolution was simple enough that I kept R-b on all but patch 8.
[1] https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg06077.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03859.html
[3] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02642.html
(git backport-diff doesn't like the rename in 7/11)
001/11:[----] [--] 'dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented' 002/11:[0024] [FC] 'dirty-bitmap: Drop unused functions'
003/11:[----] [-C] 'dirty-bitmap: Track size in bytes'
004/11:[----] [-C] 'dirty-bitmap: Set iterator start by offset, not sector' 005/11:[----] [-C] 'dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset' 006/11:[----] [-C] 'dirty-bitmap: Change bdrv_get_dirty_count() to report bytes' 007/11:[down] 'dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes' 008/11:[0036] [FC] 'dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes' 009/11:[----] [--] 'mirror: Switch mirror_dirty_init() to byte-based iteration' 010/11:[0001] [FC] 'dirty-bitmap: Switch bdrv_set_dirty() to bytes' 011/11:[----] [-C] 'dirty-bitmap: Convert internal hbitmap size/granularity'
Eric Blake (11):
dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
dirty-bitmap: Drop unused functions
dirty-bitmap: Track size in bytes
dirty-bitmap: Set iterator start by offset, not sector
dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset
dirty-bitmap: Change bdrv_get_dirty_count() to report bytes
dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes
dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes
mirror: Switch mirror_dirty_init() to byte-based iteration
dirty-bitmap: Switch bdrv_set_dirty() to bytes
dirty-bitmap: Convert internal hbitmap size/granularity
include/block/block_int.h | 2 +- include/block/dirty-bitmap.h | 28 ++++-------- block/backup.c | 7 ++- block/dirty-bitmap.c | 105 +++++++++++-------------------------------- block/io.c | 6 +-- block/mirror.c | 73 +++++++++++++----------------- migration/block.c | 12 +++-- 7 files changed, 79 insertions(+), 154 deletions(-)
Signature · 1 line
2.9.4
PATCH v3 1/11 dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
We've been documenting the value in bytes since its introduction in commit b9a9b3a4 (v1.3), where it was actually reported in bytes.
Commit e4654d2 (v2.0) then removed things from block/qapi.c, in preparation for a rewrite to a list of dirty sectors in the next commit 21b5683 in block.c, but the new code mistakenly started reporting in sectors.
---
Too late for 2.9, since the regression has been unnoticed for nine releases. But worth putting in 2.9.1.
v2: no change
--- block/dirty-bitmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
block/dirty-bitmap.c +1 -1
@@ -410,7 +410,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {BlockDirtyInfo *info =g_new0 (BlockDirtyInfo,1 );BlockDirtyInfoList *entry =g_new0 (BlockDirtyInfoList,1 ); - info->count =bdrv_get_dirty_count (bm); + info->count =bdrv_get_dirty_count (bm) <<BDRV_SECTOR_BITS ; info->granularity =bdrv_dirty_bitmap_granularity (bm); info->has_name = !!bm->name; info->name =g_strdup (bm->name);
Signature · 1 line
2.9.4
PATCH v3 2/11 dirty-bitmap: Drop unused functions
In reply to Eric BlakeWe had several functions that no one is currently using, and which use sector-based interfaces. I'm trying to convert towards byte-based interfaces, so it's easier to just drop the unused functions:
bdrv_dirty_bitmap_sizebdrv_dirty_bitmap_get_metabdrv_dirty_bitmap_get_meta_lockedbdrv_dirty_bitmap_reset_metabdrv_dirty_bitmap_meta_granularity
Vladimir may re-add bdrv_dirty_bitmap_size() for persistent bitmaps, but has agreed to do so with byte rather than sector access at the point where it is needed.
---
v3: rebase to upstream changes (bdrv_dirty_bitmap_get_meta_locked was added in b64bd51e with no clients), kept R-b
v2: tweak commit message based on review, no code change
--- include/block/dirty-bitmap.h | 11 ---------- block/dirty-bitmap.c | 49 -------------------------------------------- 2 files changed, 60 deletions(-)
include/block/dirty-bitmap.h +0 -11
@@ -30,25 +30,14 @@ void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap);BlockDirtyInfoList *bdrv_query_dirty_bitmaps (BlockDriverState *bs);uint32_t bdrv_get_default_bitmap_granularity (BlockDriverState *bs);uint32_t bdrv_dirty_bitmap_granularity (BdrvDirtyBitmap *bitmap); -uint32_t bdrv_dirty_bitmap_meta_granularity (BdrvDirtyBitmap *bitmap);bool bdrv_dirty_bitmap_enabled (BdrvDirtyBitmap *bitmap);bool bdrv_dirty_bitmap_frozen (BdrvDirtyBitmap *bitmap);const char *bdrv_dirty_bitmap_name (const BdrvDirtyBitmap *bitmap); -int64_t bdrv_dirty_bitmap_size (const BdrvDirtyBitmap *bitmap);DirtyBitmapStatus bdrv_dirty_bitmap_status (BdrvDirtyBitmap *bitmap);void bdrv_set_dirty_bitmap (BdrvDirtyBitmap *bitmap,int64_t cur_sector,int64_t nr_sectors);void bdrv_reset_dirty_bitmap (BdrvDirtyBitmap *bitmap,int64_t cur_sector,int64_t nr_sectors); -int bdrv_dirty_bitmap_get_meta (BlockDriverState *bs, -BdrvDirtyBitmap *bitmap,int64_t sector, -int nb_sectors); -int bdrv_dirty_bitmap_get_meta_locked (BlockDriverState *bs, -BdrvDirtyBitmap *bitmap,int64_t sector, -int nb_sectors); -void bdrv_dirty_bitmap_reset_meta (BlockDriverState *bs, -BdrvDirtyBitmap *bitmap,int64_t sector, -int nb_sectors);BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new (BdrvDirtyBitmap *bitmap);BdrvDirtyBitmapIter *bdrv_dirty_iter_new (BdrvDirtyBitmap *bitmap,uint64_t first_sector);
block/dirty-bitmap.c +0 -49
61 lines not on this page · raw message
Signature · 1 line
2.9.4
PATCH v3 3/11 dirty-bitmap: Track size in bytes
In reply to Eric BlakeWe are still using an internal hbitmap that tracks a size in sectors, with the granularity scaled down accordingly, because it lets us use a shortcut for our iterators which are currently sector-based. But there's no reason we can't track the dirty bitmap size in bytes, since it is an internal-only variable.
Use is_power_of_2() while at it, instead of open-coding that.
---
v2: tweak commit message, no code change
--- block/dirty-bitmap.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-)
block/dirty-bitmap.c +13 -10
@@ -42,7 +42,7 @@ struct BdrvDirtyBitmap {HBitmap *meta;/* Meta dirty bitmap */ BdrvDirtyBitmap *successor;/* Anonymous child; implies frozen status */ char *name;/* Optional non-empty unique ID */ -int64_t size;/* Size of the bitmap (Number of sectors) */ +int64_t size;/* Size of the bitmap, in bytes */ bool disabled;/* Bitmap is read-only */ int active_iterators;/* How many iterators are active */ QLIST_ENTRY (BdrvDirtyBitmap) list;
@@ -103,17 +103,14 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, {int64_t bitmap_size;BdrvDirtyBitmap *bitmap; -uint32_t sector_granularity; -assert ((granularity & (granularity -1 )) ==0 ); +assert (is_power_of_2 (granularity) && granularity >=BDRV_SECTOR_SIZE );if (name &&bdrv_find_dirty_bitmap (bs, name)) {error_setg (errp,"Bitmap already exists: %s" , name);return NULL ; } - sector_granularity = granularity >>BDRV_SECTOR_BITS ; -assert (sector_granularity); - bitmap_size =bdrv_nb_sectors (bs); + bitmap_size =bdrv_getlength (bs);if (bitmap_size <0 ) {error_setg_errno (errp, -bitmap_size,"could not get length of device" ); errno = -bitmap_size;
@@ -121,7 +118,12 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, } bitmap =g_new0 (BdrvDirtyBitmap,1 ); bitmap->mutex = &bs->dirty_bitmap_mutex; - bitmap->bitmap =hbitmap_alloc (bitmap_size,ctz32 (sector_granularity)); +/* +* TODO - let hbitmap track full granularity. For now, it is tracking +* only sector granularity, as a shortcut for our iterators. +*/ + bitmap->bitmap =hbitmap_alloc (bitmap_size >>BDRV_SECTOR_BITS , +ctz32 (granularity) -BDRV_SECTOR_BITS ); bitmap->size = bitmap_size; bitmap->name =g_strdup (name); bitmap->disabled = false;
@@ -284,13 +286,14 @@ BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,void bdrv_dirty_bitmap_truncate (BlockDriverState *bs) {BdrvDirtyBitmap *bitmap; -uint64_t size =bdrv_nb_sectors (bs); +int64_t size =bdrv_getlength (bs); +assert (size >=0 );bdrv_dirty_bitmaps_lock (bs); QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {assert (!bdrv_dirty_bitmap_frozen (bitmap));assert (!bitmap->active_iterators); -hbitmap_truncate (bitmap->bitmap, size); +hbitmap_truncate (bitmap->bitmap, size >>BDRV_SECTOR_BITS ); bitmap->size = size; }bdrv_dirty_bitmaps_unlock (bs);
@@ -490,7 +493,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)hbitmap_reset_all (bitmap->bitmap); }else {HBitmap *backup = bitmap->bitmap; - bitmap->bitmap =hbitmap_alloc (bitmap->size, + bitmap->bitmap =hbitmap_alloc (bitmap->size >>BDRV_SECTOR_BITS ,hbitmap_granularity (backup)); *out = backup; }
Signature · 1 line
2.9.4
PATCH v3 4/11 dirty-bitmap: Set iterator start by offset, not sector
In reply to Eric BlakeAll callers to bdrv_dirty_iter_new() passed 0 for their initial starting point, drop that parameter.
All callers to bdrv_set_dirty_iter() were scaling an offset to a sector number; move the scaling to occur internally to dirty bitmap code instead.
---
v2: no change
--- include/block/dirty-bitmap.h | 5 ++--- block/backup.c | 5 ++--- block/dirty-bitmap.c | 9 ++++----- block/mirror.c | 4 ++-- 4 files changed, 10 insertions(+), 13 deletions(-)
include/block/dirty-bitmap.h +2 -3
@@ -39,8 +39,7 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,void bdrv_reset_dirty_bitmap (BdrvDirtyBitmap *bitmap,int64_t cur_sector,int64_t nr_sectors);BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new (BdrvDirtyBitmap *bitmap); -BdrvDirtyBitmapIter *bdrv_dirty_iter_new (BdrvDirtyBitmap *bitmap, -uint64_t first_sector); +BdrvDirtyBitmapIter *bdrv_dirty_iter_new (BdrvDirtyBitmap *bitmap);void bdrv_dirty_iter_free (BdrvDirtyBitmapIter *iter); uint64_t bdrv_dirty_bitmap_serialization_size(const BdrvDirtyBitmap *bitmap,
@@ -67,7 +66,7 @@ void bdrv_set_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,void bdrv_reset_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap,int64_t cur_sector,int64_t nr_sectors);int64_t bdrv_dirty_iter_next (BdrvDirtyBitmapIter *iter); -void bdrv_set_dirty_iter (BdrvDirtyBitmapIter *hbi,int64_t sector_num); +void bdrv_set_dirty_iter (BdrvDirtyBitmapIter *hbi,int64_t offset);int64_t bdrv_get_dirty_count (BdrvDirtyBitmap *bitmap);int64_t bdrv_get_meta_dirty_count (BdrvDirtyBitmap *bitmap);void bdrv_dirty_bitmap_truncate (BlockDriverState *bs);
block/backup.c +2 -3
@@ -372,7 +372,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) granularity =bdrv_dirty_bitmap_granularity (job->sync_bitmap); clusters_per_iter =MAX ((granularity / job->cluster_size),1 ); - dbi =bdrv_dirty_iter_new (job->sync_bitmap,0 ); + dbi =bdrv_dirty_iter_new (job->sync_bitmap);/* Find the next dirty sector(s) */ while ((offset =bdrv_dirty_iter_next (dbi) *BDRV_SECTOR_SIZE ) >=0 ) {
@@ -403,8 +403,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 < job->cluster_size) { -bdrv_set_dirty_iter (dbi, - cluster * job->cluster_size /BDRV_SECTOR_SIZE ); +bdrv_set_dirty_iter (dbi, cluster * job->cluster_size); } last_cluster = cluster -1 ;
block/dirty-bitmap.c +4 -5
@@ -419,11 +419,10 @@ uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap)return BDRV_SECTOR_SIZE <<hbitmap_granularity (bitmap->bitmap); } -BdrvDirtyBitmapIter *bdrv_dirty_iter_new (BdrvDirtyBitmap *bitmap, -uint64_t first_sector) +BdrvDirtyBitmapIter *bdrv_dirty_iter_new (BdrvDirtyBitmap *bitmap) {BdrvDirtyBitmapIter *iter =g_new (BdrvDirtyBitmapIter,1 ); -hbitmap_iter_init (&iter->hbi, bitmap->bitmap, first_sector); +hbitmap_iter_init (&iter->hbi, bitmap->bitmap,0 ); iter->bitmap = bitmap; bitmap->active_iterators++;return iter;
@@ -567,9 +566,9 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,/** * Advance a BdrvDirtyBitmapIter to an arbitrary offset. */ -void bdrv_set_dirty_iter (BdrvDirtyBitmapIter *iter,int64_t sector_num) +void bdrv_set_dirty_iter (BdrvDirtyBitmapIter *iter,int64_t offset) { -hbitmap_iter_init (&iter->hbi, iter->hbi.hb, sector_num); +hbitmap_iter_init (&iter->hbi, iter->hbi.hb, offset >>BDRV_SECTOR_BITS ); }int64_t bdrv_get_dirty_count (BdrvDirtyBitmap *bitmap)
block/mirror.c +2 -2
@@ -373,7 +373,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s) next_dirty =bdrv_dirty_iter_next (s->dbi) *BDRV_SECTOR_SIZE ;if (next_dirty > next_offset || next_dirty <0 ) {/* The bitmap iterator's cache is stale, refresh it */ -bdrv_set_dirty_iter (s->dbi, next_offset >>BDRV_SECTOR_BITS ); +bdrv_set_dirty_iter (s->dbi, next_offset); next_dirty =bdrv_dirty_iter_next (s->dbi) *BDRV_SECTOR_SIZE ; }assert (next_dirty == next_offset);
@@ -791,7 +791,7 @@ static void coroutine_fn mirror_run(void *opaque) }assert (!s->dbi); - s->dbi =bdrv_dirty_iter_new (s->dirty_bitmap,0 ); + s->dbi =bdrv_dirty_iter_new (s->dirty_bitmap);for (;;) {uint64_t delay_ns =0 ;int64_t cnt, delta;
Signature · 1 line
2.9.4
PATCH v3 5/11 dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset
In reply to Eric BlakeThanks to recent cleanups, all callers were scaling a return value of sectors into bytes; do the scaling internally instead.
---
v2: no change
--- block/backup.c | 2 +- block/dirty-bitmap.c | 2 +- block/mirror.c | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-)
block/backup.c +1 -1
@@ -375,7 +375,7 @@ static int coroutine_fn backup_run_incremental(BackupBlockJob *job) dbi =bdrv_dirty_iter_new (job->sync_bitmap);/* Find the next dirty sector(s) */ -while ((offset =bdrv_dirty_iter_next (dbi) *BDRV_SECTOR_SIZE ) >=0 ) { +while ((offset =bdrv_dirty_iter_next (dbi)) >=0 ) { cluster = offset / job->cluster_size;/* Fake progress updates for any clusters we skipped */
block/dirty-bitmap.c +1 -1
@@ -449,7 +449,7 @@ void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter)int64_t bdrv_dirty_iter_next (BdrvDirtyBitmapIter *iter) { -return hbitmap_iter_next (&iter->hbi); +return hbitmap_iter_next (&iter->hbi) *BDRV_SECTOR_SIZE ; }/* Called within bdrv_dirty_bitmap_lock..unlock */
block/mirror.c +4 -4
@@ -336,10 +336,10 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)int max_io_bytes =MAX (s->buf_size /MAX_IN_FLIGHT ,MAX_IO_BYTES );bdrv_dirty_bitmap_lock (s->dirty_bitmap); - offset =bdrv_dirty_iter_next (s->dbi) *BDRV_SECTOR_SIZE ; + offset =bdrv_dirty_iter_next (s->dbi);if (offset <0 ) {bdrv_set_dirty_iter (s->dbi,0 ); - offset =bdrv_dirty_iter_next (s->dbi) *BDRV_SECTOR_SIZE ; + offset =bdrv_dirty_iter_next (s->dbi);trace_mirror_restart_iter (s,bdrv_get_dirty_count (s->dirty_bitmap) *BDRV_SECTOR_SIZE );assert (offset >=0 );
@@ -370,11 +370,11 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)break ; } - next_dirty =bdrv_dirty_iter_next (s->dbi) *BDRV_SECTOR_SIZE ; + next_dirty =bdrv_dirty_iter_next (s->dbi);if (next_dirty > next_offset || next_dirty <0 ) {/* The bitmap iterator's cache is stale, refresh it */ bdrv_set_dirty_iter (s->dbi, next_offset); - next_dirty =bdrv_dirty_iter_next (s->dbi) *BDRV_SECTOR_SIZE ; + next_dirty =bdrv_dirty_iter_next (s->dbi); }assert (next_dirty == next_offset); nb_chunks++;
Signature · 1 line
2.9.4
PATCH v3 6/11 dirty-bitmap: Change bdrv_get_dirty_count() to report bytes
In reply to Eric BlakeThanks to recent cleanups, all callers were scaling a return value of sectors into bytes; do the scaling internally instead.
---
v3: no change, add R-b
v2: no change
--- block/dirty-bitmap.c | 4 ++-- block/mirror.c | 13 +++++-------- migration/block.c | 2 +- 3 files changed, 8 insertions(+), 11 deletions(-)
block/dirty-bitmap.c +2 -2
@@ -369,7 +369,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {BlockDirtyInfo *info =g_new0 (BlockDirtyInfo,1 );BlockDirtyInfoList *entry =g_new0 (BlockDirtyInfoList,1 ); - info->count =bdrv_get_dirty_count (bm) <<BDRV_SECTOR_BITS ; + info->count =bdrv_get_dirty_count (bm); info->granularity =bdrv_dirty_bitmap_granularity (bm); info->has_name = !!bm->name; info->name =g_strdup (bm->name);
@@ -573,7 +573,7 @@ void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *iter, int64_t offset)int64_t bdrv_get_dirty_count (BdrvDirtyBitmap *bitmap) { -return hbitmap_count (bitmap->bitmap); +return hbitmap_count (bitmap->bitmap) <<BDRV_SECTOR_BITS ; }int64_t bdrv_get_meta_dirty_count (BdrvDirtyBitmap *bitmap)
block/mirror.c +5 -8
@@ -806,11 +806,10 @@ static void coroutine_fn mirror_run(void *opaque) cnt =bdrv_get_dirty_count (s->dirty_bitmap);/* s->common.offset contains the number of bytes already processed so -* far, cnt is the number of dirty sectors remaining and +* far, cnt is the number of dirty bytes remaining and * s->bytes_in_flight is the number of bytes currently being * processed; together those are the current total operation length */ - s->common.len = s->common.offset + s->bytes_in_flight + - cnt *BDRV_SECTOR_SIZE ; + s->common.len = s->common.offset + s->bytes_in_flight + cnt;/* Note that even when no rate limit is applied we need to yield * periodically with no pending I/O so that bdrv_drain_all() returns.
@@ -822,8 +821,7 @@ static void coroutine_fn mirror_run(void *opaque) s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) {if (s->in_flight >=MAX_IN_FLIGHT || s->buf_free_count ==0 || (cnt ==0 && s->in_flight >0 )) { -trace_mirror_yield (s, cnt *BDRV_SECTOR_SIZE , - s->buf_free_count, s->in_flight); +trace_mirror_yield (s, cnt, s->buf_free_count, s->in_flight);mirror_wait_for_io (s);continue ; }else if (cnt !=0 ) {
@@ -864,7 +862,7 @@ static void coroutine_fn mirror_run(void *opaque)* whether to switch to target check one last time if I/O has * come in the meanwhile, and if not flush the data to disk. */ -trace_mirror_before_drain (s, cnt *BDRV_SECTOR_SIZE ); +trace_mirror_before_drain (s, cnt);bdrv_drained_begin (bs); cnt =bdrv_get_dirty_count (s->dirty_bitmap);
@@ -883,8 +881,7 @@ static void coroutine_fn mirror_run(void *opaque) } ret =0 ; -trace_mirror_before_sleep (s, cnt *BDRV_SECTOR_SIZE , - s->synced, delay_ns); +trace_mirror_before_sleep (s, cnt, s->synced, delay_ns);if (!s->synced) {block_job_sleep_ns (&s->common,QEMU_CLOCK_REALTIME , delay_ns);if (block_job_is_cancelled (&s->common)) {
migration/block.c +1 -1
@@ -664,7 +664,7 @@ static int64_t get_remaining_dirty(void)aio_context_release (blk_get_aio_context (bmds->blk)); } -return dirty <<BDRV_SECTOR_BITS ; +return dirty; }
Signature · 1 line
2.9.4
PATCH v3 7/11 dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes
In reply to Eric BlakeHalf the callers were already scaling bytes to sectors; the other half can eventually be simplified to use byte iteration. Both callers were already using the result as a bool, so make that explicit. Making the change also makes it easier for a future dirty-bitmap patch to offload scaling over to the internal hbitmap.
Remember, asking whether a byte is dirty is effectively asking whether the entire granularity containing the byte is dirty, since we only track dirtiness by granularity.
---
v3: rebase to _locked rename was straightforward enough that R-b kept
v2: tweak commit message, no code change
--- include/block/dirty-bitmap.h | 4 ++-- block/dirty-bitmap.c | 8 ++++---- block/mirror.c | 3 +-- migration/block.c | 3 ++- 4 files changed, 9 insertions(+), 9 deletions(-)
include/block/dirty-bitmap.h +2 -2
@@ -59,8 +59,8 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap);/* Functions that require manual locking. */ void bdrv_dirty_bitmap_lock (BdrvDirtyBitmap *bitmap);void bdrv_dirty_bitmap_unlock (BdrvDirtyBitmap *bitmap); -int bdrv_get_dirty_locked (BlockDriverState *bs,BdrvDirtyBitmap *bitmap, -int64_t sector); +bool bdrv_get_dirty_locked (BlockDriverState *bs,BdrvDirtyBitmap *bitmap, +int64_t offset);void bdrv_set_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap,int64_t cur_sector,int64_t nr_sectors); void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
block/dirty-bitmap.c +4 -4
@@ -384,13 +384,13 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) }/* Called within bdrv_dirty_bitmap_lock..unlock */ -int bdrv_get_dirty_locked (BlockDriverState *bs,BdrvDirtyBitmap *bitmap, -int64_t sector) +bool bdrv_get_dirty_locked (BlockDriverState *bs,BdrvDirtyBitmap *bitmap, +int64_t offset) {if (bitmap) { -return hbitmap_get (bitmap->bitmap, sector); +return hbitmap_get (bitmap->bitmap, offset >>BDRV_SECTOR_BITS ); }else { -return 0 ; +return false; } }
block/mirror.c +1 -2
@@ -362,8 +362,7 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)int64_t next_offset = offset + nb_chunks * s->granularity;int64_t next_chunk = next_offset / s->granularity;if (next_offset >= s->bdev_length || - !bdrv_get_dirty_locked (source, s->dirty_bitmap, - next_offset >>BDRV_SECTOR_BITS )) { + !bdrv_get_dirty_locked (source, s->dirty_bitmap, next_offset)) {break ; }if (test_bit (next_chunk, s->in_flight_bitmap)) {
migration/block.c +2 -1
@@ -527,7 +527,8 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds,blk_mig_unlock (); }bdrv_dirty_bitmap_lock (bmds->dirty_bitmap); -if (bdrv_get_dirty_locked (bs, bmds->dirty_bitmap, sector)) { +if (bdrv_get_dirty_locked (bs, bmds->dirty_bitmap, + sector *BDRV_SECTOR_SIZE )) {if (total_sectors - sector <BDRV_SECTORS_PER_DIRTY_CHUNK ) { nr_sectors = total_sectors - sector; }else {
Signature · 1 line
2.9.4
PATCH v3 8/11 dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes
In reply to Eric BlakeSome of the callers were already scaling bytes to sectors; others can be easily converted to pass byte offsets, all in our shift towards a consistent byte interface everywhere. Making the change will also make it easier to write the hold-out callers to use byte rather than sectors for their iterations; it also makes it easier for a future dirty-bitmap patch to offload scaling over to the internal hbitmap. Although all callers happen to pass sector-aligned values, make the internal scaling robust to any sub-sector requests.
---
v3: rebase to addition of _locked interfaces; complex enough that I dropped R-b
v2: no change
--- include/block/dirty-bitmap.h | 8 ++++---- block/dirty-bitmap.c | 22 ++++++++++++++-------- block/mirror.c | 16 ++++++++-------- migration/block.c | 7 +++++-- 4 files changed, 31 insertions(+), 22 deletions(-)
include/block/dirty-bitmap.h +4 -4
@@ -35,9 +35,9 @@ bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);const char *bdrv_dirty_bitmap_name (const BdrvDirtyBitmap *bitmap);DirtyBitmapStatus bdrv_dirty_bitmap_status (BdrvDirtyBitmap *bitmap);void bdrv_set_dirty_bitmap (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors); +int64_t offset,int64_t bytes);void bdrv_reset_dirty_bitmap (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors); +int64_t offset,int64_t bytes);BdrvDirtyBitmapIter *bdrv_dirty_meta_iter_new (BdrvDirtyBitmap *bitmap);BdrvDirtyBitmapIter *bdrv_dirty_iter_new (BdrvDirtyBitmap *bitmap);void bdrv_dirty_iter_free (BdrvDirtyBitmapIter *iter);
@@ -62,9 +62,9 @@ void bdrv_dirty_bitmap_unlock(BdrvDirtyBitmap *bitmap);bool bdrv_get_dirty_locked (BlockDriverState *bs,BdrvDirtyBitmap *bitmap,int64_t offset);void bdrv_set_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors); +int64_t offset,int64_t bytes);void bdrv_reset_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors); +int64_t offset,int64_t bytes);int64_t bdrv_dirty_iter_next (BdrvDirtyBitmapIter *iter);void bdrv_set_dirty_iter (BdrvDirtyBitmapIter *hbi,int64_t offset);int64_t bdrv_get_dirty_count (BdrvDirtyBitmap *bitmap);
block/dirty-bitmap.c +14 -8
@@ -454,33 +454,39 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter)/* Called within bdrv_dirty_bitmap_lock..unlock */ void bdrv_set_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors) +int64_t offset,int64_t bytes) { +int64_t end_sector =DIV_ROUND_UP (offset + bytes,BDRV_SECTOR_SIZE ); +assert (bdrv_dirty_bitmap_enabled (bitmap)); -hbitmap_set (bitmap->bitmap, cur_sector, nr_sectors); +hbitmap_set (bitmap->bitmap, offset >>BDRV_SECTOR_BITS , + end_sector - (offset >>BDRV_SECTOR_BITS )); }void bdrv_set_dirty_bitmap (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors) +int64_t offset,int64_t bytes) {bdrv_dirty_bitmap_lock (bitmap); -bdrv_set_dirty_bitmap_locked (bitmap, cur_sector, nr_sectors); +bdrv_set_dirty_bitmap_locked (bitmap, offset, bytes);bdrv_dirty_bitmap_unlock (bitmap); }/* Called within bdrv_dirty_bitmap_lock..unlock */ void bdrv_reset_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors) +int64_t offset,int64_t bytes) { +int64_t end_sector =DIV_ROUND_UP (offset + bytes,BDRV_SECTOR_SIZE ); +assert (bdrv_dirty_bitmap_enabled (bitmap)); -hbitmap_reset (bitmap->bitmap, cur_sector, nr_sectors); +hbitmap_reset (bitmap->bitmap, offset >>BDRV_SECTOR_BITS , + end_sector - (offset >>BDRV_SECTOR_BITS )); }void bdrv_reset_dirty_bitmap (BdrvDirtyBitmap *bitmap, -int64_t cur_sector,int64_t nr_sectors) +int64_t offset,int64_t bytes) {bdrv_dirty_bitmap_lock (bitmap); -bdrv_reset_dirty_bitmap_locked (bitmap, cur_sector, nr_sectors); +bdrv_reset_dirty_bitmap_locked (bitmap, offset, bytes);bdrv_dirty_bitmap_unlock (bitmap); }
block/mirror.c +8 -8
@@ -141,8 +141,7 @@ static void mirror_write_complete(void *opaque, int ret)if (ret <0 ) {BlockErrorAction action; -bdrv_set_dirty_bitmap (s->dirty_bitmap, op->offset >>BDRV_SECTOR_BITS , - op->bytes >>BDRV_SECTOR_BITS ); +bdrv_set_dirty_bitmap (s->dirty_bitmap, op->offset, op->bytes); action =mirror_error_action (s, false, -ret);if (action ==BLOCK_ERROR_ACTION_REPORT && s->ret >=0 ) { s->ret = ret;
@@ -161,8 +160,7 @@ static void mirror_read_complete(void *opaque, int ret)if (ret <0 ) {BlockErrorAction action; -bdrv_set_dirty_bitmap (s->dirty_bitmap, op->offset >>BDRV_SECTOR_BITS , - op->bytes >>BDRV_SECTOR_BITS ); +bdrv_set_dirty_bitmap (s->dirty_bitmap, op->offset, op->bytes); action =mirror_error_action (s, true, -ret);if (action ==BLOCK_ERROR_ACTION_REPORT && s->ret >=0 ) { s->ret = ret;
@@ -383,8 +381,8 @@ static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)* calling bdrv_get_block_status_above could yield - if some blocks are * marked dirty in this window, we need to know. */ -bdrv_reset_dirty_bitmap_locked (s->dirty_bitmap, offset >>BDRV_SECTOR_BITS , - nb_chunks * sectors_per_chunk); +bdrv_reset_dirty_bitmap_locked (s->dirty_bitmap, offset, + nb_chunks * s->granularity);bdrv_dirty_bitmap_unlock (s->dirty_bitmap);bitmap_set (s->in_flight_bitmap, offset / s->granularity, nb_chunks);
@@ -626,7 +624,7 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)if (base ==NULL && !bdrv_has_zero_init (target_bs)) {if (!bdrv_can_write_zeroes_with_unmap (target_bs)) { -bdrv_set_dirty_bitmap (s->dirty_bitmap,0 , end); +bdrv_set_dirty_bitmap (s->dirty_bitmap,0 , s->bdev_length);return 0 ; }
@@ -679,7 +677,9 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) n =DIV_ROUND_UP (count,BDRV_SECTOR_SIZE );assert (n >0 );if (ret ==1 ) { -bdrv_set_dirty_bitmap (s->dirty_bitmap, sector_num, n); +bdrv_set_dirty_bitmap (s->dirty_bitmap, + sector_num *BDRV_SECTOR_SIZE , + n *BDRV_SECTOR_SIZE ); } sector_num += n; }
migration/block.c +5 -2
@@ -326,7 +326,8 @@ static int mig_save_device_bulk(QEMUFile *f, BlkMigDevState *bmds) blk->aiocb =blk_aio_preadv (bb, cur_sector *BDRV_SECTOR_SIZE , &blk->qiov,0 , blk_mig_read_cb, blk); -bdrv_reset_dirty_bitmap (bmds->dirty_bitmap, cur_sector, nr_sectors); +bdrv_reset_dirty_bitmap (bmds->dirty_bitmap, cur_sector *BDRV_SECTOR_SIZE , + nr_sectors *BDRV_SECTOR_SIZE );aio_context_release (blk_get_aio_context (bmds->blk));qemu_mutex_unlock_iothread ();
@@ -534,7 +535,9 @@ static int mig_save_device_dirty(QEMUFile *f, BlkMigDevState *bmds, }else { nr_sectors =BDRV_SECTORS_PER_DIRTY_CHUNK ; } -bdrv_reset_dirty_bitmap_locked (bmds->dirty_bitmap, sector, nr_sectors); +bdrv_reset_dirty_bitmap_locked (bmds->dirty_bitmap, + sector *BDRV_SECTOR_SIZE , + nr_sectors *BDRV_SECTOR_SIZE );bdrv_dirty_bitmap_unlock (bmds->dirty_bitmap); blk =g_new (BlkMigBlock,1 );
Signature · 1 line
2.9.4
PATCH v3 9/11 mirror: Switch mirror_dirty_init() to byte-based iteration
In reply to Eric BlakeNow that we have adjusted the majority of the calls this function makes to be byte-based, it is easier to read the code if it makes passes over the image using bytes rather than sectors.
---
v2: no change
--- block/mirror.c | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-)
block/mirror.c +14 -21
@@ -613,15 +613,13 @@ static void mirror_throttle(MirrorBlockJob *s) static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) { -int64_t sector_num, end; +int64_t offset;BlockDriverState *base = s->base;BlockDriverState *bs = s->source;BlockDriverState *target_bs =blk_bs (s->target); -int ret, n; +int ret;int64_t count; - end = s->bdev_length /BDRV_SECTOR_SIZE ; -if (base ==NULL && !bdrv_has_zero_init (target_bs)) {if (!bdrv_can_write_zeroes_with_unmap (target_bs)) {bdrv_set_dirty_bitmap (s->dirty_bitmap,0 , s->bdev_length);
@@ -629,9 +627,9 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) } s->initial_zeroing_ongoing = true; -for (sector_num =0 ; sector_num < end; ) { -int nb_sectors =MIN (end - sector_num, -QEMU_ALIGN_DOWN (INT_MAX , s->granularity) >>BDRV_SECTOR_BITS ); +for (offset =0 ; offset < s->bdev_length; ) { +int bytes =MIN (s->bdev_length - offset, +QEMU_ALIGN_DOWN (INT_MAX , s->granularity));mirror_throttle (s);
@@ -647,9 +645,8 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)continue ; } -mirror_do_zero_or_discard (s, sector_num *BDRV_SECTOR_SIZE , - nb_sectors *BDRV_SECTOR_SIZE , false); - sector_num += nb_sectors; +mirror_do_zero_or_discard (s, offset, bytes, false); + offset += bytes; }mirror_wait_for_all_io (s);
@@ -657,10 +654,10 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s) }/* First part, loop on the sectors and initialize the dirty bitmap. */ -for (sector_num =0 ; sector_num < end; ) { +for (offset =0 ; offset < s->bdev_length; ) {/* Just to make sure we are not exceeding int limit. */ -int nb_sectors =MIN (INT_MAX >>BDRV_SECTOR_BITS , - end - sector_num); +int bytes =MIN (s->bdev_length - offset, +QEMU_ALIGN_DOWN (INT_MAX , s->granularity));mirror_throttle (s);
@@ -668,20 +665,16 @@ static int coroutine_fn mirror_dirty_init(MirrorBlockJob *s)return 0 ; } - ret =bdrv_is_allocated_above (bs, base, sector_num *BDRV_SECTOR_SIZE , - nb_sectors *BDRV_SECTOR_SIZE , &count); + ret =bdrv_is_allocated_above (bs, base, offset, bytes, &count);if (ret <0 ) {return ret; } - n =DIV_ROUND_UP (count,BDRV_SECTOR_SIZE ); -assert (n >0 ); + count =QEMU_ALIGN_UP (count,BDRV_SECTOR_SIZE );if (ret ==1 ) { -bdrv_set_dirty_bitmap (s->dirty_bitmap, - sector_num *BDRV_SECTOR_SIZE , - n *BDRV_SECTOR_SIZE ); +bdrv_set_dirty_bitmap (s->dirty_bitmap, offset, count); } - sector_num += n; + offset += count; }return 0 ; }
Signature · 1 line
2.9.4
PATCH v3 10/11 dirty-bitmap: Switch bdrv_set_dirty() to bytes
In reply to Eric BlakeBoth callers already had bytes available, but were scaling to sectors. Move the scaling to internal code. In the case of bdrv_aligned_pwritev(), we are now passing the exact offset rather than a rounded sector-aligned value, but that's okay as long as dirty bitmap widens start/bytes to granularity boundaries.
---
v3: rebase to lock context changes, R-b kept
v2: no change
--- include/block/block_int.h | 2 +- block/dirty-bitmap.c | 7 ++++--- block/io.c | 6 ++---- 3 files changed, 7 insertions(+), 8 deletions(-)
include/block/block_int.h +1 -1
@@ -953,7 +953,7 @@ void blk_dev_eject_request(BlockBackend *blk, bool force);bool blk_dev_is_tray_open (BlockBackend *blk);bool blk_dev_is_medium_locked (BlockBackend *blk); -void bdrv_set_dirty (BlockDriverState *bs,int64_t cur_sector,int64_t nr_sect); +void bdrv_set_dirty (BlockDriverState *bs,int64_t offset,int64_t bytes);bool bdrv_requests_pending (BlockDriverState *bs);void bdrv_clear_dirty_bitmap (BdrvDirtyBitmap *bitmap,HBitmap **out);
block/dirty-bitmap.c +4 -3
@@ -550,10 +550,10 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap)hbitmap_deserialize_finish (bitmap->bitmap); } -void bdrv_set_dirty (BlockDriverState *bs,int64_t cur_sector, -int64_t nr_sectors) +void bdrv_set_dirty (BlockDriverState *bs,int64_t offset,int64_t bytes) {BdrvDirtyBitmap *bitmap; +int64_t end_sector =DIV_ROUND_UP (offset + bytes,BDRV_SECTOR_SIZE );if (QLIST_EMPTY (&bs->dirty_bitmaps)) {return ;
@@ -564,7 +564,8 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t cur_sector,if (!bdrv_dirty_bitmap_enabled (bitmap)) {continue ; } -hbitmap_set (bitmap->bitmap, cur_sector, nr_sectors); +hbitmap_set (bitmap->bitmap, offset >>BDRV_SECTOR_BITS , + end_sector - (offset >>BDRV_SECTOR_BITS )); }bdrv_dirty_bitmaps_unlock (bs); }
block/io.c +2 -4
@@ -1310,7 +1310,6 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,bool waited;int ret; -int64_t start_sector = offset >>BDRV_SECTOR_BITS ;int64_t end_sector =DIV_ROUND_UP (offset + bytes,BDRV_SECTOR_SIZE );uint64_t bytes_remaining = bytes;int max_transfer;
@@ -1381,7 +1380,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,bdrv_debug_event (bs,BLKDBG_PWRITEV_DONE );atomic_inc (&bs->write_gen); -bdrv_set_dirty (bs, start_sector, end_sector - start_sector); +bdrv_set_dirty (bs, offset, bytes);stat64_max (&bs->wr_highest_offset, offset + bytes);
@@ -2362,8 +2361,7 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset, ret =0 ; out:atomic_inc (&bs->write_gen); -bdrv_set_dirty (bs, req.offset >>BDRV_SECTOR_BITS , - req.bytes >>BDRV_SECTOR_BITS ); +bdrv_set_dirty (bs, req.offset, req.bytes);tracked_request_end (&req);bdrv_dec_in_flight (bs);return ret;
Signature · 1 line
2.9.4
PATCH v3 11/11 dirty-bitmap: Convert internal hbitmap size/granularity
In reply to Eric BlakeNow that all callers are using byte-based interfaces, there's no reason for our internal hbitmap to remain with sector-based granularity. It also simplifies our internal scaling, since we already know that hbitmap widens requests out to granularity boundaries.
---
v2: no change
--- block/dirty-bitmap.c | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-)
block/dirty-bitmap.c +12 -25
@@ -38,7 +38,7 @@*/ struct BdrvDirtyBitmap {QemuMutex *mutex; -HBitmap *bitmap;/* Dirty sector bitmap implementation */ +HBitmap *bitmap;/* Dirty bitmap implementation */ HBitmap *meta;/* Meta dirty bitmap */ BdrvDirtyBitmap *successor;/* Anonymous child; implies frozen status */ char *name;/* Optional non-empty unique ID */
@@ -118,12 +118,7 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, } bitmap =g_new0 (BdrvDirtyBitmap,1 ); bitmap->mutex = &bs->dirty_bitmap_mutex; -/* -* TODO - let hbitmap track full granularity. For now, it is tracking -* only sector granularity, as a shortcut for our iterators. -*/ - bitmap->bitmap =hbitmap_alloc (bitmap_size >>BDRV_SECTOR_BITS , -ctz32 (granularity) -BDRV_SECTOR_BITS ); + bitmap->bitmap =hbitmap_alloc (bitmap_size,ctz32 (granularity)); bitmap->size = bitmap_size; bitmap->name =g_strdup (name); bitmap->disabled = false;
@@ -293,7 +288,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 >>BDRV_SECTOR_BITS ); +hbitmap_truncate (bitmap->bitmap, size); bitmap->size = size; }bdrv_dirty_bitmaps_unlock (bs);
@@ -388,7 +383,7 @@ bool bdrv_get_dirty_locked(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t offset) {if (bitmap) { -return hbitmap_get (bitmap->bitmap, offset >>BDRV_SECTOR_BITS ); +return hbitmap_get (bitmap->bitmap, offset); }else {return false; }
@@ -416,7 +411,7 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)uint32_t bdrv_dirty_bitmap_granularity (BdrvDirtyBitmap *bitmap) { -return BDRV_SECTOR_SIZE <<hbitmap_granularity (bitmap->bitmap); +return 1U <<hbitmap_granularity (bitmap->bitmap); }BdrvDirtyBitmapIter *bdrv_dirty_iter_new (BdrvDirtyBitmap *bitmap)
@@ -449,18 +444,15 @@ void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter)int64_t bdrv_dirty_iter_next (BdrvDirtyBitmapIter *iter) { -return hbitmap_iter_next (&iter->hbi) *BDRV_SECTOR_SIZE ; +return hbitmap_iter_next (&iter->hbi); }/* Called within bdrv_dirty_bitmap_lock..unlock */ void bdrv_set_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap,int64_t offset,int64_t bytes) { -int64_t end_sector =DIV_ROUND_UP (offset + bytes,BDRV_SECTOR_SIZE ); -assert (bdrv_dirty_bitmap_enabled (bitmap)); -hbitmap_set (bitmap->bitmap, offset >>BDRV_SECTOR_BITS , - end_sector - (offset >>BDRV_SECTOR_BITS )); +hbitmap_set (bitmap->bitmap, offset, bytes); } void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
@@ -475,11 +467,8 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,void bdrv_reset_dirty_bitmap_locked (BdrvDirtyBitmap *bitmap,int64_t offset,int64_t bytes) { -int64_t end_sector =DIV_ROUND_UP (offset + bytes,BDRV_SECTOR_SIZE ); -assert (bdrv_dirty_bitmap_enabled (bitmap)); -hbitmap_reset (bitmap->bitmap, offset >>BDRV_SECTOR_BITS , - end_sector - (offset >>BDRV_SECTOR_BITS )); +hbitmap_reset (bitmap->bitmap, offset, bytes); } void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
@@ -498,7 +487,7 @@ void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out)hbitmap_reset_all (bitmap->bitmap); }else {HBitmap *backup = bitmap->bitmap; - bitmap->bitmap =hbitmap_alloc (bitmap->size >>BDRV_SECTOR_BITS , + bitmap->bitmap =hbitmap_alloc (bitmap->size,hbitmap_granularity (backup)); *out = backup; }
@@ -553,7 +542,6 @@ void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap)void bdrv_set_dirty (BlockDriverState *bs,int64_t offset,int64_t bytes) {BdrvDirtyBitmap *bitmap; -int64_t end_sector =DIV_ROUND_UP (offset + bytes,BDRV_SECTOR_SIZE );if (QLIST_EMPTY (&bs->dirty_bitmaps)) {return ;
@@ -564,8 +552,7 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes)if (!bdrv_dirty_bitmap_enabled (bitmap)) {continue ; } -hbitmap_set (bitmap->bitmap, offset >>BDRV_SECTOR_BITS , - end_sector - (offset >>BDRV_SECTOR_BITS )); +hbitmap_set (bitmap->bitmap, offset, bytes); }bdrv_dirty_bitmaps_unlock (bs); }
@@ -575,12 +562,12 @@ void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes)*/ void bdrv_set_dirty_iter (BdrvDirtyBitmapIter *iter,int64_t offset) { -hbitmap_iter_init (&iter->hbi, iter->hbi.hb, offset >>BDRV_SECTOR_BITS ); +hbitmap_iter_init (&iter->hbi, iter->hbi.hb, offset); }int64_t bdrv_get_dirty_count (BdrvDirtyBitmap *bitmap) { -return hbitmap_count (bitmap->bitmap) <<BDRV_SECTOR_BITS ; +return hbitmap_count (bitmap->bitmap); }int64_t bdrv_get_meta_dirty_count (BdrvDirtyBitmap *bitmap)
Signature · 1 line
2.9.4
Hi!
Can we apply my "[PATCH v22 00/30] qcow2: persistent dirty bitmaps" first? It was already near to the victory a week ago, but I had to rebase it on new Paolo's patches.
Quoted 62 lines · Eric Blake
There are patches floating around to add
NBD_CMD_BLOCK_STATUS, but NBD wants to report status on byte granularity (even if the reporting will probably be naturally aligned to sectors or even much higher levels). I've therefore started the task of converting our block status code to report at a byte granularity rather than sectors.This is part two of that conversion: dirty-bitmap. Other parts include
bdrv_is_allocated(at v3 [1]) and replacingbdrv_get_block_statuswith a byte based callback in all the drivers (at v1, needs a rebase [3]).Available as a tag at:
git fetchgit://repo.or.cz/qemu/ericb.gitnbd-byte-dirty-v3Depends on Kevin's block branch and my v3
bdrv_is_allocated[1]Since v2 [2], I had to rebase on top of Paolo's locking fixes; patch v2 2/12 is gone, and many of the others had a lot of context conflicts. But I felt the resolution was simple enough that I kept R-b on all but patch 8.
[1] https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg06077.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03859.html
[3] https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02642.html(git backport-diff doesn't like the rename in 7/11)
001/11:[----] [--] 'dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented' 002/11:[0024] [FC] 'dirty-bitmap: Drop unused functions'
003/11:[----] [-C] 'dirty-bitmap: Track size in bytes'
004/11:[----] [-C] 'dirty-bitmap: Set iterator start by offset, not sector' 005/11:[----] [-C] 'dirty-bitmap: Changebdrv_dirty_iter_next()to report byte offset' 006/11:[----] [-C] 'dirty-bitmap: Changebdrv_get_dirty_count()to report bytes' 007/11:[down] 'dirty-bitmap: Changebdrv_get_dirty_locked()to take bytes' 008/11:[0036] [FC] 'dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes' 009/11:[----] [--] 'mirror: Switchmirror_dirty_init()to byte-based iteration' 010/11:[0001] [FC] 'dirty-bitmap: Switchbdrv_set_dirty()to bytes' 011/11:[----] [-C] 'dirty-bitmap: Convert internal hbitmap size/granularity'Eric Blake (11):
dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
dirty-bitmap: Drop unused functions
dirty-bitmap: Track size in bytes
dirty-bitmap: Set iterator start by offset, not sector
dirty-bitmap: Changebdrv_dirty_iter_next()to report byte offset
dirty-bitmap: Changebdrv_get_dirty_count()to report bytes
dirty-bitmap: Changebdrv_get_dirty_locked()to take bytes
dirty-bitmap: Change bdrv_[re]set_dirty_bitmap() to use bytes
mirror: Switchmirror_dirty_init()to byte-based iteration
dirty-bitmap: Switchbdrv_set_dirty()to bytes
dirty-bitmap: Convert internal hbitmap size/granularityinclude/block/block_int.h | 2 +- include/block/dirty-bitmap.h | 28 ++++-------- block/backup.c | 7 ++- block/dirty-bitmap.c | 105 +++++++++++-------------------------------- block/io.c | 6 +-- block/mirror.c | 73 +++++++++++++----------------- migration/block.c | 12 +++-- 7 files changed, 79 insertions(+), 154 deletions(-)
Signature · 2 lines
Best regards, Vladimir
Quoted 5 lines · Vladimir Sementsov-Ogievskiy
Hi!
Can we apply my "[PATCH v22 00/30] qcow2: persistent dirty bitmaps" first? It was already near to the victory a week ago, but I had to rebase it on new Paolo's patches.
Sure, I can rebase on top of yours (I've already rebased multiple times, as have you, but yours has been on the list longer).
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 10 lines · Eric Blake
Quoted 5 lines · Vladimir Sementsov-Ogievskiy
Hi!
Can we apply my "[PATCH v22 00/30] qcow2: persistent dirty bitmaps" first? It was already near to the victory a week ago, but I had to rebase it on new Paolo's patches.
Sure, I can rebase on top of yours (I've already rebased multiple times, as have you, but yours has been on the list longer).
I would normally like to feign impartiality, but in this case I'd really prefer if Vladimir's patches went through first, too.
--js