Re: [Qemu-devel] [PATCH 4/8] block: add bdrv_load_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

On 11.06.2015 01:33, John Snow wrote:
> 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
I'll just use bdrv_dirty_bitmap_deserialize_part, etc, which are already 
in the code.

--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.