08.06.2019 1:39, John Snow wrote: > > > On 6/3/19 8:00 AM, Vladimir Sementsov-Ogievskiy wrote: >> Add functionality to make bitmap temporary anonymous. It will be used >> to implement bitmap remove transaction action. We need hide bitmap >> persistence too, as there are should not be unnamed persistent bitmaps. >> > > Ah, so this effectively ... "hides" a bitmap from any further > transaction actions. It also "hides" it from getting flushed to disk... > sort of? > > The outer loop in store works with bdrv_dirty_bitmap_next, and we'll > skip this bitmap because it's anonymous/not persistent. > > There's a second loop where we iterate bm_list, and we'll skip storing > this bitmap because that entry won't have an in-memory bitmap associated > with it in bm_list. > > ...But then we'll call update_ext_header_and_dir with the stale entries > in bm_list?
Hidden is a temprory state, so, we should not go to close() in this state. It's a state inside a transaction. Stale entries in list will be marked IN_USE anyway, so there is no damage anyway. > >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> --- >> include/block/dirty-bitmap.h | 2 ++ >> block/dirty-bitmap.c | 26 ++++++++++++++++++++++++++ >> 2 files changed, 28 insertions(+) >> >> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h >> index 8044ace63e..542e437123 100644 >> --- a/include/block/dirty-bitmap.h >> +++ b/include/block/dirty-bitmap.h >> @@ -116,5 +116,7 @@ bool bdrv_dirty_bitmap_next_dirty_area(BdrvDirtyBitmap >> *bitmap, >> BdrvDirtyBitmap *bdrv_reclaim_dirty_bitmap_locked(BlockDriverState *bs, >> BdrvDirtyBitmap *bitmap, >> Error **errp); >> +void bdrv_dirty_bitmap_hide(BdrvDirtyBitmap *bitmap); >> +void bdrv_dirty_bitmap_unhide(BdrvDirtyBitmap *bitmap); >> >> #endif >> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >> index 49646a30e6..592964635e 100644 >> --- a/block/dirty-bitmap.c >> +++ b/block/dirty-bitmap.c >> @@ -35,6 +35,10 @@ struct BdrvDirtyBitmap { >> bool busy; /* Bitmap is busy, it can't be used via >> QMP */ >> BdrvDirtyBitmap *successor; /* Anonymous child, if any. */ >> char *name; /* Optional non-empty unique ID */ >> + char *hidden_name; /* Backup of @name for removal transaction >> + action. Used for hide/unhide API. */ >> + bool hidden_persistent; /* Backup of @persistent for removal >> transaction >> + action. */ >> int64_t size; /* Size of the bitmap, in bytes */ >> bool disabled; /* Bitmap is disabled. It ignores all >> writes to >> the device */ >> @@ -849,3 +853,25 @@ out: >> qemu_mutex_unlock(src->mutex); >> } >> } >> + >> +void bdrv_dirty_bitmap_hide(BdrvDirtyBitmap *bitmap) >> +{ >> + qemu_mutex_lock(bitmap->mutex); >> + assert(!bitmap->hidden_name); >> + bitmap->hidden_name = bitmap->name; >> + bitmap->hidden_persistent = bitmap->persistent; >> + bitmap->name = NULL; >> + bitmap->persistent = false; >> + qemu_mutex_unlock(bitmap->mutex); >> +} >> + >> +void bdrv_dirty_bitmap_unhide(BdrvDirtyBitmap *bitmap) >> +{ >> + qemu_mutex_lock(bitmap->mutex); >> + assert(!bitmap->name); >> + bitmap->name = bitmap->hidden_name; >> + bitmap->persistent = bitmap->hidden_persistent; >> + bitmap->hidden_name = NULL; >> + bitmap->hidden_persistent = false; >> + qemu_mutex_unlock(bitmap->mutex); >> +} >> -- Best regards, Vladimir