Il 04/11/2013 07:59, Fam Zheng ha scritto: >>> >> I think callers outside block.c should only call >> hbitmap_set/hbitmap_reset; resetting is typically done before processing >> sectors and setting after an error (both of which happen privately to >> each task). >> >> Thus you probably should add a fourth patch which makes >> bdrv_(re)set_dirty static and remove >> bdrv_get_dirty/bdrv_dirty_iter_init/bdrv_get_dirty_count. > I like the idea of adding a wrapper struct (will be BdrvDirtyBitmap) to > HBitmap so that patch 1 is not needed, and HBitmap becomes (almost) > internal to block.c. > > But I'm not sure removing > bdrv_get_dirty/bdrv_dirty_iter_init/bdrv_get_dirty_count is good, as we > are exposing BdrvDirtyBitmap, we should also provide operations on it, > instead of let callers to handle HBitmap pointer inside. In other words, > I prefer to define BdrvDirtyBitmap structure in block.c and only put a > type declaration in header.
If you want to expose BdrvDirtyBitmap, having wrappers is fine. I was thinking of exposing HBitmap instead and keeping BdrvDirtyBitmap internal. Either way is fine, and as the person who writes the code you have the privilege of making the choice. :) Paolo
