| Message ID | 20170602112158.232757-14-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
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 <vsementsov@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 <jsnow@redhat.com>
On 03.06.2017 01:17, John Snow wrote: > > 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 <vsementsov@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 (A) is done (see patch about reopen RO) (B) - We can't do it, as on this preparation the image is still RO, and I've created new handler, write 'in_use' _after_ the point when new flags was set. > >> 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 <jsnow@redhat.com> >
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 <vsementsov@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; > }; > >
09.06.2017 16:27, Max Reitz wrote: > 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 <vsementsov@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". Ok for prepending, but I don't want to add last note, as for the user it may better to retry an operation, leading reopening image rw.. > > 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; >> }; >> >> >
On 2017-06-13 12:25, Vladimir Sementsov-Ogievskiy wrote: > 09.06.2017 16:27, Max Reitz wrote: >> 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 <vsementsov@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". > > Ok for prepending, but I don't want to add last note, as for the user it > may better to retry an operation, leading reopening image rw.. Which operation do you mean? The reopening? Because that operation already "succeeded" at this point, so you can't retry it... Max
13.06.2017 18:28, Max Reitz wrote: > On 2017-06-13 12:25, Vladimir Sementsov-Ogievskiy wrote: >> 09.06.2017 16:27, Max Reitz wrote: >>> 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 <vsementsov@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". >> Ok for prepending, but I don't want to add last note, as for the user it >> may better to retry an operation, leading reopening image rw.. > Which operation do you mean? The reopening? Because that operation > already "succeeded" at this point, so you can't retry it... So, firstly, reopen r-o again and then reopen r-w? Is it possible? > > Max >
On 2017-06-14 11:03, Vladimir Sementsov-Ogievskiy wrote: > 13.06.2017 18:28, Max Reitz wrote: >> On 2017-06-13 12:25, Vladimir Sementsov-Ogievskiy wrote: >>> 09.06.2017 16:27, Max Reitz wrote: >>>> 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 <vsementsov@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". >>> Ok for prepending, but I don't want to add last note, as for the user it >>> may better to retry an operation, leading reopening image rw.. >> Which operation do you mean? The reopening? Because that operation >> already "succeeded" at this point, so you can't retry it... > > So, firstly, reopen r-o again and then reopen r-w? Is it possible? That should work, yes -- if it works then. :-) Max
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); + QLIST_ENTRY(BlockDriver) list; };
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 <vsementsov@virtuozzo.com> --- block.c | 17 +++++++++++++++++ include/block/block_int.h | 7 +++++++ 2 files changed, 24 insertions(+)