Re: [Qemu-devel] [PATCH v8 07/10] hbitmap: serialization

[Max Reitz]

On 02.08.2016 02:12, John Snow wrote:
> From: Vladimir Sementsov-Ogievskiy <address@hidden>
> 
> Functions to serialize / deserialize(restore) HBitmap. HBitmap should be
> saved to linear sequence of bits independently of endianness and bitmap
> array element (unsigned long) size. Therefore Little Endian is chosen.
> 
> These functions are appropriate for dirty bitmap migration, restoring
> the bitmap in several steps is available. To save performance, every
> step writes only the last level of the bitmap. All other levels are
> restored by hbitmap_deserialize_finish() as a last step of restoring.
> So, HBitmap is inconsistent while restoring.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> [Fix left shift operand to 1UL; add "finish" parameter. - Fam]
> Signed-off-by: Fam Zheng <address@hidden>
> Reviewed-by: Max Reitz <address@hidden>
> 
> Signed-off-by: John Snow <address@hidden>
> ---
>  include/qemu/hbitmap.h |  79 ++++++++++++++++++++++++++++
>  util/hbitmap.c         | 137 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 216 insertions(+)
> 

[...]

> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index f303975..6a13c12 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -397,6 +397,143 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item)
>      return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit) != 
> 0;
>  }
>  
> +uint64_t hbitmap_serialization_granularity(const HBitmap *hb)
> +{
> +    /* Require at least 64 bit granularity to be safe on both 64 bit and 32 
> bit
> +     * hosts. */
> +    return 64 << hb->granularity;
> +}
> +
> +/* Start should be aligned to serialization granularity, chunk size should be
> + * aligned to serialization granularity too, except for last chunk.
> + */
> +static void serialization_chunk(const HBitmap *hb,
> +                                uint64_t start, uint64_t count,
> +                                unsigned long **first_el, size_t *el_count)

As spotted by both Peter and the automatic build system, the last
parameter should be a uint64_t *.

Also, there's the issue Peter spotted on a BE machine:

http://lists.nongnu.org/archive/html/qemu-block/2016-09/msg00261.html

Because the latter is probably not as trivially fixable as the former, I
guess I'll have to drop this series from my queue for now.

Max

> +{
> +    uint64_t last = start + count - 1;
> +    uint64_t gran = hbitmap_serialization_granularity(hb);
> +
> +    assert((start & (gran - 1)) == 0);
> +    assert((last >> hb->granularity) < hb->size);
> +    if ((last >> hb->granularity) != hb->size - 1) {
> +        assert((count & (gran - 1)) == 0);
> +    }
> +
> +    start = (start >> hb->granularity) >> BITS_PER_LEVEL;
> +    last = (last >> hb->granularity) >> BITS_PER_LEVEL;
> +
> +    *first_el = &hb->levels[HBITMAP_LEVELS - 1][start];
> +    *el_count = last - start + 1;
> +}
> +

signature.asc
Description: OpenPGP digital signature