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

John Snow posted 2 patches 6 years, 8 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
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(-)
[Qemu-devel] [RFC PATCH 0/2] bitmaps: add inconsistent bit
Posted by John Snow 6 years, 8 months ago
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 chose "inconsistent" over "corrupt" to be more literal to the
     meaning of the "in-use" bit, but maybe this is overcautious.

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

     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.

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(-)

-- 
2.17.2


Re: [Qemu-devel] [RFC PATCH 0/2] bitmaps: add inconsistent bit
Posted by Vladimir Sementsov-Ogievskiy 6 years, 8 months ago
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