On 06/02/2017 07:21 AM, Vladimir Sementsov-Ogievskiy wrote: > Add format driver handler, which should mark loaded read-only > bitmaps as 'IN_USE' in the image and unset read_only field in > corresponding BdrvDirtyBitmap's. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > block.c | 17 +++++++++++++++++ > include/block/block_int.h | 7 +++++++ > 2 files changed, 24 insertions(+) > > diff --git a/block.c b/block.c > index 04af7697dc..161db9e32a 100644 > --- a/block.c > +++ b/block.c > @@ -2946,12 +2946,16 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) > { > BlockDriver *drv; > BlockDriverState *bs; > + bool old_can_write, new_can_write; > > assert(reopen_state != NULL); > bs = reopen_state->bs; > drv = bs->drv; > assert(drv != NULL); > > + old_can_write = > + !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); > + > /* If there are any driver level actions to take */ > if (drv->bdrv_reopen_commit) { > drv->bdrv_reopen_commit(reopen_state); > @@ -2965,6 +2969,19 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state) > bs->read_only = !(reopen_state->flags & BDRV_O_RDWR); > > bdrv_refresh_limits(bs, NULL); > + > + new_can_write = > + !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE); > + if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) { > + Error *local_err = NULL; > + if (drv->bdrv_reopen_bitmaps_rw(bs, &local_err) < 0) { > + /* This is not fatal, bitmaps just left read-only, so all > following > + * writes will fail. User can remove read-only bitmaps to unblock > + * writes. > + */ > + error_report_err(local_err); > + } > + } > } > > /* > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 8d3724cce6..1dc6f2e90d 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -380,6 +380,13 @@ struct BlockDriver { > uint64_t parent_perm, uint64_t parent_shared, > uint64_t *nperm, uint64_t *nshared); > > + /** > + * Bitmaps should be marked as 'IN_USE' in the image on reopening image > + * as rw. This handler should realize it. It also should unset readonly > + * field of BlockDirtyBitmap's in case of success. > + */ > + int (*bdrv_reopen_bitmaps_rw)(BlockDriverState *bs, Error **errp); > +
Hmm, do we need a new top-level hook for this? We already have .bdrv_reopen_commit and .bdrv_reopen_prepare which inform the blockdriver that a reopen event is occurring. Can't we just amend qcow2_reopen_prepare and qcow2_reopen_commit to do the necessary tasks of either: (A) Flushing the bitmap in preparation for reopening as RO, or (B) Writing in_use and removing the RO flag in preparation for reopening as RW > QLIST_ENTRY(BlockDriver) list; > }; > > Well, design issues aside, I will at least confirm that I think this patch should work as designed, and I will leave the design critiques to Max and Kevin: Reviewed-by: John Snow <js...@redhat.com>