Re: [Qemu-block] [Qemu-devel] [PATCH v22 06/30] block/dirty-bitmap: add deserialize_ones func
On 06/29/2017 10:01 PM, Eric Blake wrote: > On 06/29/2017 08:55 PM, Eric Blake wrote: >> On 06/28/2017 07:05 AM, Vladimir Sementsov-Ogievskiy wrote: >>> Add bdrv_dirty_bitmap_deserialize_ones() function, which is needed for >>> qcow2 bitmap loading, to handle unallocated bitmap parts, marked as >>> all-ones. >>> > >>> + * hbitmap_deserialize_ones >>> + * @hb: HBitmap to operate on. >>> + * @start: First bit to restore. >>> + * @count: Number of bits to restore. >> >> This part is accurate (the dirty-bitmap is using an underlying bitmap >> with "one bit per sector" before my series, afterwards it will be "one >> bit per byte", remembering that hbitmap really stores only one bit per >> granularity multiple of the underlying unit), if incomplete (the code >> asserts that things are aligned, but doesn't document that the caller >> must pass in aligned values); but again, that's matching the >> pre-existing deserialize_zeroes code. > > Okay, I looked again; the documentation for > hbitmap_serialization_granularity() has a blanket statement that all > other hbitmap_serialization_* functions taking a start and count must be > aligned. Indirect, but at least documented, so I retract my statement > about the docs being incomplete. > If the docs are confusing, please send patches to amend them; the bitmap code is definitely very confusing at times and I appreciate any and all insight to make the names, variable and documentation read better to be more intuitive. Thanks!
Re: [Qemu-block] [Qemu-devel] [PATCH v22 06/30] block/dirty-bitmap: add deserialize_ones func
On 06/29/2017 08:55 PM, Eric Blake wrote: > On 06/28/2017 07:05 AM, Vladimir Sementsov-Ogievskiy wrote: >> Add bdrv_dirty_bitmap_deserialize_ones() function, which is needed for >> qcow2 bitmap loading, to handle unallocated bitmap parts, marked as >> all-ones. >> >> + * hbitmap_deserialize_ones >> + * @hb: HBitmap to operate on. >> + * @start: First bit to restore. >> + * @count: Number of bits to restore. > > This part is accurate (the dirty-bitmap is using an underlying bitmap > with "one bit per sector" before my series, afterwards it will be "one > bit per byte", remembering that hbitmap really stores only one bit per > granularity multiple of the underlying unit), if incomplete (the code > asserts that things are aligned, but doesn't document that the caller > must pass in aligned values); but again, that's matching the > pre-existing deserialize_zeroes code. Okay, I looked again; the documentation for hbitmap_serialization_granularity() has a blanket statement that all other hbitmap_serialization_* functions taking a start and count must be aligned. Indirect, but at least documented, so I retract my statement about the docs being incomplete. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
