Re: [Qemu-devel] [RFC PATCH 0/2] bitmaps: add inconsistent bit

[Vladimir Sementsov-Ogievskiy]

14.02.2019 2:36, John Snow wrote:
> Allow QEMU to read in bitmaps that have the in-use bit set, for the
> purposes of allowing users to clear or reset these bitmaps.
> 
> This is chosen in preference to a hard error on load to minimize
> impact for a non-critical error, but to force the user or management
> utility to acknowledge that the bitmap is no longer viable.
> 
> Requires: [PATCH v2 0/6] dirty-bitmaps: deprecate @status field
>            (Which in turn requires my bitmaps staging branch.)
> 
> RFC: This is just for general approach, naming, and API.
>       I chose NOT to overload the busy predicate so that it would be distinct,
>       which unfortunately means many more manual checks across blockdev.

I think we just need a helper predicate with errp parameter, which will call 
both
_busy and _inconsistent.

> 
>       - I chose "inconsistent" over "corrupt" to be more literal to the
>       meaning of the "in-use" bit, but maybe this is overcautious.

I'm ok with both

> 
>       - I chose to make the field optional so that it disappears in
>       normative cases, as the information is only really relevant when
>       inconsistent=true.

Agree

> 
>       I also have NOT tested this and I didn't verify the saving logic
>       for what happens if you don't delete or clear the bitmap,
>       but I'm on PTO the next two days and I wanted this to see daylight.

I think, we should not change clear() logic to make bitmaps consistent. Is
there real usage of it?

1. bitmap (and its name) is related to some checkpoint, and clearing don't make 
it consistent
2. it is extra difficulty
3. we can add this logic separately, later, on demand, if needed
4. without it, we can use the following simple interface and easily save RAM:

no bdrv_dirty_bitmap_set_inconsistent(), but just 
bdrv_create_inconsistent_dirty_bitmap()
(ahaha cool function name), and call it in qcow2 code, when found in-use 
bitmap. And, in
this bdrv_create_inconsitent_dirty_bitmap() do not allocate HBitmap, which may 
save a
lot of memory.

> 
> John Snow (2):
>    block/dirty-bitmaps: add inconsistent bit
>    block/dirty-bitmap: implement inconsistent bit
> 
>   block/dirty-bitmap.c         | 34 ++++++++++++++++++++++++++++
>   block/qcow2-bitmap.c         | 42 ++++++++++++++++++-----------------
>   blockdev.c                   | 43 ++++++++++++++++++++++++++++++++++++
>   include/block/dirty-bitmap.h |  3 +++
>   qapi/block-core.json         |  9 ++++++--
>   5 files changed, 109 insertions(+), 22 deletions(-)
> 


-- 
Best regards,
Vladimir