20.06.2019 19:36, John Snow wrote: > > > On 6/20/19 12:02 PM, Max Reitz wrote: >> On 20.06.19 03:03, John Snow wrote: >>> This function can claim an hbitmap to replace its own current hbitmap. >>> In the case that the granularities do not match, it will use >>> hbitmap_merge to copy the bit data instead. >> >> I really do not like this name because to me it implies a relationship >> to bdrv_reclaim_dirty_bitmap(). Maybe just bdrv_dirty_bitmap_take()? >> Or, if you want to get more fancy, bdrv_dirty_dirty_bitmap_steal(). >> (Because references are taken or stolen.) >> > > take or steal is good. I just wanted to highlight that it truly takes > ownership. The double-pointer and erasing the caller's reference for > emphasis of this idea.
Didn't you consider bdrv_dirty_bitmap_set_hbitmap? Hmm, but your function actually eats pointer, so 'set' is not enough.. bdrv_dirty_bitmap_eat_hbitmap? And to be serious: it is the point where we definitely should drop HBitmap:meta field, as it in bad relation with parent hbitmap stealing. > >> The latter might fit in nicely with the abdication theme. We’d just >> need to rename bdrv_reclaim_dirty_bitmap() to >> bdrv_dirty_bitmap_backstab(), and it’d be perfect. >> > > Don't tempt me; I do like my silly function names. You are lucky I don't > call > >> (On another note: bdrv_restore_dirty_bitmap() vs. >> bdrv_dirty_bitmap_restore() – really? :-/) >> > > I have done a terrible job at enforcing any kind of consistency here, > and it gets me all the time too. I had a big series that re-arranged and > re-named a ton of stuff just to make things a little more nicer, but I > let it bitrot because I didn't want to deal with the bike-shedding. > > I do agree I am in desperate need of a spring cleaning in here. > > One thing that does upset me quite often is that the canonical name for > the structure is "bdrv dirty bitmap", which makes the function names all > quite long. > >>> Signed-off-by: John Snow <js...@redhat.com> >>> --- >>> include/block/block_int.h | 1 + >>> include/qemu/hbitmap.h | 8 ++++++++ >>> block/dirty-bitmap.c | 14 ++++++++++++++ >>> util/hbitmap.c | 5 +++++ >>> 4 files changed, 28 insertions(+) >> >> The implementation looks good to me. >> >> Max >> > -- Best regards, Vladimir
