On Thu, 03/27 16:43, Stefan Hajnoczi wrote: > On Thu, Mar 27, 2014 at 05:09:41PM +0800, Fam Zheng wrote: > > @@ -1713,6 +1713,66 @@ void qmp_block_set_io_throttle(const char *device, > > int64_t bps, int64_t bps_rd, > > } > > } > > > > +void qmp_dirty_bitmap_add(const char *device, const char *name, > > + bool has_granularity, int64_t granularity, > > + Error **errp) > > +{ > > + BlockDriverState *bs; > > + BdrvDirtyBitmap *bitmap; > > + > > + bs = bdrv_find(device); > > + if (!bs) { > > + error_set(errp, QERR_DEVICE_NOT_FOUND, device); > > + return; > > + } > > + > > + if (!name || name[0] == '\0') { > > + error_setg(errp, "Bitmap name cannot be empty"); > > + return; > > + } > > + if (has_granularity) { > > + if (granularity & (granularity - 1)) { > > + error_setg(errp, "Granularity must be power of 2"); > > + return; > > + } > > granularity must be non-zero, otherwise bdrv_create_dirty_bitmap() hits > an assertion failure. > > It should probably also be at least 512.
Sure, adding a check. > > > + } else { > > + granularity = 65536; > > + } > > + > > + bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp); > > + if (!bitmap) { > > + return; > > + } > > Useless error return. Removing. > > > +} > > + > > +void qmp_dirty_bitmap_remove(const char *device, const char *name, > > + Error **errp) > > +{ > > + BlockDriverState *bs; > > + BdrvDirtyBitmap *bitmap; > > + > > + bs = bdrv_find(device); > > + if (!bs) { > > + error_set(errp, QERR_DEVICE_NOT_FOUND, device); > > + return; > > + } > > + > > + if (!name || name[0] == '\0') { > > + error_setg(errp, "Bitmap name cannot be empty"); > > + return; > > + } > > + bitmap = bdrv_find_dirty_bitmap(bs, name); > > + if (!bitmap) { > > + error_setg(errp, "Dirty bitmap not found: %s", name); > > + return; > > + } > > + > > + /* Make it invisible to user in case the following > > + * bdrv_release_dirty_bitmap doens't free it because of refcnt */ > > "doesn't" Thanks, Fam