On 2017-06-02 13:21, 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.
> + */
In a sense, it pretty much is fatal. We were asked to make the image
non-read-only but we failed because it effectively still is read-only.
But I can't think of anything better, and you're right, removing the
bitmaps would resolve the situation. This would require the user to know
that updating the bitmaps was the issue, and local_err may not actually
reflect that.
> + error_report_err(local_err);
So I'd prepend this with something like "$node_name: Failed to make
dirty bitmaps writable", and maybe append a hint like "Removing all
persistent dirty bitmaps from this node will allow writing to it".
Max
> + }
> + }
> }
>
> /*
> 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);
> +
> QLIST_ENTRY(BlockDriver) list;
> };
>
>
signature.asc
Description: OpenPGP digital signature
