On 06/09/2015 12:01 PM, Stefan Hajnoczi wrote: > On Mon, Jun 08, 2015 at 06:21:22PM +0300, Vladimir Sementsov-Ogievskiy wrote: >> +BdrvDirtyBitmap *bdrv_load_dirty_bitmap(BlockDriverState *bs, >> + BlockDriverState *file, >> + int granularity, >> + const char *name, >> + Error **errp) >> +{ >> + BlockDriver *drv = file->drv; >> + if (!drv) { >> + return NULL; >> + } >> + if (drv->bdrv_dirty_bitmap_load) { >> + BdrvDirtyBitmap *bitmap; >> + uint64_t bitmap_size = bdrv_nb_sectors(bs); >> + uint8_t *buf = drv->bdrv_dirty_bitmap_load(file, name, bitmap_size, >> + granularity); >> + if (buf == NULL) { >> + return NULL; >> + } >> + >> + bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp); >> + if (bitmap == NULL) { >> + g_free(buf); >> + return NULL; >> + } >> + >> + hbitmap_deserialize_part(bitmap->bitmap, buf, 0, bitmap_size); >> + hbitmap_deserialize_finish(bitmap->bitmap); > > How about passing bitmap and errp into drv->bdrv_dirty_bitmap_load? > That way bdrv_dirty_bitmap_load() can stream using > hbitmap_deserialize_part() and does not need to allocate the full > bitmap. It can also report errors properly. >
My hunch is that this was avoided because BdrvDirtyBitmap is currently a structure local only to block.c, but I would be fine with shifting the header to block_int.h and giving the BdrvDirtyBitmap some limited exposure outside of the block core file to facilitate some cleaner function prototypes here. OR, you could have the qcow2 layer rely on serialization functions that are written back here in block.c that supports feeding it out chunk-by-chunk. Whatever happens to feel cleaner is (probably) fine by me. --js