10.06.2019 12:42, Vladimir Sementsov-Ogievskiy wrote:
> 10.06.2019 12:33, Vladimir Sementsov-Ogievskiy wrote:
>> 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.
>
> I'm wrong, storing inside transaction is exactly what test does.. Hmm, so,
> seems you
> are right, we are storing stale IN_USE bitmaps. No serious damage, but bad
> design:
> creating inconsistent bitmaps in each snapshot. I need to fix it in v2
> somehow.
Ahaha, and I'm wrong again, as my series do right thing: remove bitmap from the
image
in .prepare, so there will be no stale entries..
>
>>
>>>
>>>> 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
