On Mon, Dec 01, 2014 at 03:30:08PM -0500, John Snow wrote: > diff --git a/block.c b/block.c > index e5c6ccf..3f27519 100644 > --- a/block.c > +++ b/block.c > @@ -5420,6 +5420,25 @@ int bdrv_get_dirty(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap, int64_t sector > } > } > > +#define BDB_MIN_DEF_GRANULARITY 4096 > +#define BDB_MAX_DEF_GRANULARITY 65536 > +#define BDB_DEFAULT_GRANULARITY BDB_MAX_DEF_GRANULARITY > + > +uint64_t bdrv_dbm_calc_def_granularity(BlockDriverState *bs)
Long names are unwieldy but introducing multiple abbreviations is not a
good solution, it makes the code more confusing (BDB vs dbm).
I would call the function bdrv_get_default_bitmap_granularity().
The constants weren't necessary since the point of this function is to
capture the default value. No one else should use the constants -
otherwise there is a high probability that they are reimplementing this
logic. I would just leave them as literals in the code.
> diff --git a/blockdev.c b/blockdev.c
> index 5651a8e..4d30b09 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1894,6 +1894,60 @@ void qmp_block_set_io_throttle(const char *device,
> int64_t bps, int64_t bps_rd,
> aio_context_release(aio_context);
> }
>
> +void qmp_block_dirty_bitmap_add(const char *device, const char *name,
> + bool has_granularity, int64_t granularity,
> + Error **errp)
> +{
> + BlockDriverState *bs;
> +
> + bs = bdrv_lookup_bs(device, NULL, errp);
Markus: I think we need to support node-name here so dirty bitmaps can
be applied at any node in the graph?
> + if (!bs) {
> + return;
> + }
These new monitor commands need to acquire/release the
BlockDriverState's AioContext like the other monitor commands. This
ensures that BDS accesses in other threads are synchronized (stopped
while we run).
AioContext *aio_context;
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
...
out:
aio_context_release(aio_context);
pgp__INvPK1Kj.pgp
Description: PGP signature
