[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 01/12] hbitmap: serialization
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 01/12] hbitmap: serialization |
Date: |
Thu, 20 Aug 2015 07:53:26 -0700 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
On Fri, Aug 07, 2015 at 12:32:33PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> +/**
> + * hbitmap_serialize_part
> + * @hb: HBitmap to oprate on.
s/oprate/operate/
> + * @buf: Buffer to store serialized bitmap.
> + * @start: First bit to store.
> + * @count: Number of bits to store.
> + *
> + * Stores HBitmap data corresponding to given region. The format of saved
> data
> + * is linear sequence of bits, so it can be used by hbitmap_deserialize_part
> + * independently of endianness and size of HBitmap level array elements
These functions *are* dependent of HBitmap level array element size.
They always assign full array elements (unsigned long). If count <
BITS_PER_LONG at some point before the end, the bitmap will be corrupt
because leading bits will be zeroed when the next
hbitmap_deserialize_part() call is made!
> + */
> +void hbitmap_serialize_part(const HBitmap *hb, uint8_t *buf,
> + uint64_t start, uint64_t count);
> +
> +/**
> + * hbitmap_deserialize_part
> + * @hb: HBitmap to operate on.
> + * @buf: Buffer to restore bitmap data from.
> + * @start: First bit to restore.
> + * @count: Number of bits to restore.
> + *
> + * Retores HBitmap data corresponding to given region. The format is the same
s/Retores/Restores/
> + * as for hbitmap_serialize_part.
> + *
> + * ! The bitmap becomes inconsistent after this operation.
> + * hbitmap_serialize_finish should be called before using the bitmap after
> + * data restoring.
> + */
> +void hbitmap_deserialize_part(HBitmap *hb, uint8_t *buf,
> + uint64_t start, uint64_t count);
> +
> +/**
> + * hbitmap_deserialize_zeroes
> + * @hb: HBitmap to operate on.
> + * @start: First bit to restore.
> + * @count: Number of bits to restore.
> + *
> + * Same as hbitmap_serialize_part, but fills the bitmap with zeroes.
> + */
> +void hbitmap_deserialize_zeroes(HBitmap *hb, uint64_t start, uint64_t count);
> +
> +/**
> + * hbitmap_deserialize_finish
> + * @hb: HBitmap to operate on.
> + *
> + * Repair HBitmap after calling hbitmap_deserialize_data. Actually, all
> HBitmap
> + * layers are restored here.
> + */
> +void hbitmap_deserialize_finish(HBitmap *hb);
> +
> +/**
> * hbitmap_free:
> * @hb: HBitmap to operate on.
> *
> diff --git a/util/hbitmap.c b/util/hbitmap.c
> index 50b888f..c7c21fe 100644
> --- a/util/hbitmap.c
> +++ b/util/hbitmap.c
> @@ -378,6 +378,104 @@ bool hbitmap_get(const HBitmap *hb, uint64_t item)
> return (hb->levels[HBITMAP_LEVELS - 1][pos >> BITS_PER_LEVEL] & bit) !=
> 0;
> }
>
> +uint64_t hbitmap_data_size(const HBitmap *hb, uint64_t count)
> +{
> + uint64_t size, gran;
> +
> + if (count == 0) {
> + return 0;
> + }
> +
> + gran = 1ll << hb->granularity;
> + size = (((gran + count - 2) >> hb->granularity) >> BITS_PER_LEVEL) + 1;
> +
> + return size * sizeof(unsigned long);
> +}
> +
> +void hbitmap_serialize_part(const HBitmap *hb, uint8_t *buf,
> + uint64_t start, uint64_t count)
> +{
> + uint64_t i;
> + uint64_t last = start + count - 1;
> + unsigned long *out = (unsigned long *)buf;
I'm not sure if we care but this can lead to unaligned stores if buf
isn't aligned to sizeof(unsigned long). Unaligned stores are best
avoided:
https://www.linux-mips.org/wiki/Alignment
If you replace out[i - start] = ... with a memcpy() call then the
alignment problem is solved. If you are worried that all these memcpy()
calls are slow, check the compiler output since gcc probably optimizes
away the memcpy().
> +
> + if (count == 0) {
> + return;
> + }
> +
> + start = (start >> hb->granularity) >> BITS_PER_LEVEL;
> + last = (last >> hb->granularity) >> BITS_PER_LEVEL;
> + count = last - start + 1;
> +
> + for (i = start; i <= last; ++i) {
> + unsigned long el = hb->levels[HBITMAP_LEVELS - 1][i];
> + out[i - start] =
> + (BITS_PER_LONG == 32 ? cpu_to_le32(el) : cpu_to_le64(el));
> + }
> +}
> +
> +void hbitmap_deserialize_part(HBitmap *hb, uint8_t *buf,
> + uint64_t start, uint64_t count)
> +{
> + uint64_t i;
> + uint64_t last = start + count - 1;
> + unsigned long *in = (unsigned long *)buf;
Same here.
pgpqj7WmEiPN5.pgp
Description: PGP signature
- [Qemu-devel] [PATCH v6 00/12] Dirty bitmaps migration, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 02/12] block: BdrvDirtyBitmap serialization interface, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 01/12] hbitmap: serialization, Vladimir Sementsov-Ogievskiy, 2015/08/07
- Re: [Qemu-devel] [PATCH 01/12] hbitmap: serialization,
Stefan Hajnoczi <=
- [Qemu-devel] [PATCH 04/12] block: add meta bitmaps, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 03/12] block: tiny refactoring: minimize hbitmap_(set/reset) usage, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 05/12] block: add bdrv_next_dirty_bitmap(), Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 06/12] qapi: add dirty-bitmaps migration capability, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 07/12] migration/qemu-file: add qemu_put_counted_string(), Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 09/12] iotests: maintain several vms in test, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 10/12] iotests: add add_incoming_migration to VM class, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 08/12] migration: add migration/block-dirty-bitmap.c, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 11/12] qapi: add md5 checksum of last dirty bitmap level to query-block, Vladimir Sementsov-Ogievskiy, 2015/08/07
- [Qemu-devel] [PATCH 12/12] iotests: add dirty bitmap migration test, Vladimir Sementsov-Ogievskiy, 2015/08/07