| Message ID | 20190606184159.979-4-jsnow@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block/dirty-bitmap: check number and size constraints against queued bitmaps | expand |
On 6/6/19 1:41 PM, John Snow wrote: > Allow propagating error code information from > bdrv_remove_persistent_dirty_bitmap as well. > > Give it an interface that matches the newly revised > bdrv_add_persistent_dirty_bitmap, including removing the persistent flag > when the operation succeeds and refusing to operate on bitmaps that are > not persistent. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > +++ b/include/block/block_int.h > @@ -540,9 +540,9 @@ struct BlockDriver { > int (*bdrv_add_persistent_dirty_bitmap)(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap, > Error **errp); > - void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, > - const char *name, > - Error **errp); > + int (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Error **errp); Would it hurt us (in patch 2 and again here) to add a comment about what each callback is supposed to do? Just because we've been lousy at callback interfaces in the past does not mean that we should continue to omit them. Reviewed-by: Eric Blake <eblake@redhat.com>
06.06.2019 21:41, John Snow wrote: > Allow propagating error code information from > bdrv_remove_persistent_dirty_bitmap as well. > > Give it an interface that matches the newly revised > bdrv_add_persistent_dirty_bitmap, including removing the persistent flag > when the operation succeeds and refusing to operate on bitmaps that are > not persistent. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > block/qcow2.h | 6 +++--- > include/block/block_int.h | 6 +++--- > include/block/dirty-bitmap.h | 6 +++--- > block/dirty-bitmap.c | 25 ++++++++++++++++++++----- > block/qcow2-bitmap.c | 16 +++++++++------- > blockdev.c | 15 ++++++--------- > 6 files changed, 44 insertions(+), 30 deletions(-) > > diff --git a/block/qcow2.h b/block/qcow2.h > index 95d723d3c0..ce07f003f7 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -745,9 +745,9 @@ int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); > int qcow2_add_persistent_dirty_bitmap(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap, > Error **errp); > -void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, > - const char *name, > - Error **errp); > +int qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Error **errp); > > ssize_t coroutine_fn > qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size, > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 93bbb66cd0..59f8cb9c12 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -540,9 +540,9 @@ struct BlockDriver { > int (*bdrv_add_persistent_dirty_bitmap)(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap, > Error **errp); > - void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, > - const char *name, > - Error **errp); > + int (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Error **errp); > > /** > * Register/unregister a buffer for I/O. For example, when the driver is > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h > index c37edbe05f..88a9832ddc 100644 > --- a/include/block/dirty-bitmap.h > +++ b/include/block/dirty-bitmap.h > @@ -41,9 +41,9 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs); > int bdrv_add_persistent_dirty_bitmap(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap, > Error **errp); > -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, > - const char *name, > - Error **errp); > +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Error **errp); > void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap); > void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap); > void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap); > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index 615f8480b2..4667f9e65a 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -455,15 +455,30 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs) > * BdrvDirtyBitmap can have .persistent = true but not yet saved and have no > * stored version. For such bitmap bdrv_remove_persistent_dirty_bitmap() should > * not fail. > - * This function doesn't release corresponding BdrvDirtyBitmap. > + * This function doesn't release the corresponding BdrvDirtyBitmap. > */ > -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, > - const char *name, > - Error **errp) > +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Error **errp) > { > + int ret = 0; > + > + if (!bdrv_dirty_bitmap_get_persistence(bitmap)) { > + error_setg(errp, "Bitmap '%s' is not persistent, " > + "so it cannot be removed from node '%s'", > + bdrv_dirty_bitmap_name(bitmap), > + bdrv_get_device_or_node_name(bs)); > + return -EINVAL; > + } > + > if (bs->drv && bs->drv->bdrv_remove_persistent_dirty_bitmap) { > - bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, name, errp); > + ret = bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, bitmap, errp); > } > + if (!ret) { > + bdrv_dirty_bitmap_set_persistence(bitmap, false); > + } > + > + return ret; > } > > int bdrv_add_persistent_dirty_bitmap(BlockDriverState *bs, > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index c3a72ca782..930a6c91ff 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -1402,23 +1402,24 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list, > return NULL; > } > > -void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, > - const char *name, > - Error **errp) > +int qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, > + BdrvDirtyBitmap *bitmap, > + Error **errp) > { > - int ret; > + int ret = 0; dead assignment > BDRVQcow2State *s = bs->opaque; > Qcow2Bitmap *bm; > Qcow2BitmapList *bm_list; > + const char *name = bdrv_dirty_bitmap_name(bitmap); > > if (s->nb_bitmaps == 0) { > /* Absence of the bitmap is not an error: see explanation above > * bdrv_remove_persistent_dirty_bitmap() definition. */ > - return; > + return 0; > } > > - if (bitmap_list_load(bs, &bm_list, errp)) { > - return; > + if ((ret = bitmap_list_load(bs, &bm_list, errp))) { > + return ret; > } > > bm = find_bitmap_by_name(bm_list, name); if (bm == NULL) { goto fail; } You don't set ret, as it's considered success. Worth s/fail/out then (preexisting, of course)? > @@ -1439,6 +1440,7 @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, > fail: > bitmap_free(bm); > bitmap_list_free(bm_list); > + return ret; > } > > void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) > diff --git a/blockdev.c b/blockdev.c > index 169a8da831..82f02d8e72 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2875,7 +2875,6 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, > { > BlockDriverState *bs; > BdrvDirtyBitmap *bitmap; > - Error *local_err = NULL; Oh, that's cool. And it was my mistake to create interfaces provoking endless local_errors.. > AioContext *aio_context = NULL; > > bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp); > @@ -2889,20 +2888,18 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, > } > > if (bdrv_dirty_bitmap_get_persistence(bitmap)) { > + int ret; > + > aio_context = bdrv_get_aio_context(bs); and here, could you define aio_context in this block? > aio_context_acquire(aio_context); > - bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err); > - if (local_err != NULL) { > - error_propagate(errp, local_err); > - goto out; > + ret = bdrv_remove_persistent_dirty_bitmap(bs, bitmap, errp); > + aio_context_release(aio_context); > + if (ret) { > + return; > } > } > > bdrv_release_dirty_bitmap(bs, bitmap); > - out: > - if (aio_context) { > - aio_context_release(aio_context); > - } > } > > /** > With some my suggestions or without: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
On 6/7/19 10:41 AM, Vladimir Sementsov-Ogievskiy wrote: > 06.06.2019 21:41, John Snow wrote: >> Allow propagating error code information from >> bdrv_remove_persistent_dirty_bitmap as well. >> >> Give it an interface that matches the newly revised >> bdrv_add_persistent_dirty_bitmap, including removing the persistent flag >> when the operation succeeds and refusing to operate on bitmaps that are >> not persistent. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> block/qcow2.h | 6 +++--- >> include/block/block_int.h | 6 +++--- >> include/block/dirty-bitmap.h | 6 +++--- >> block/dirty-bitmap.c | 25 ++++++++++++++++++++----- >> block/qcow2-bitmap.c | 16 +++++++++------- >> blockdev.c | 15 ++++++--------- >> 6 files changed, 44 insertions(+), 30 deletions(-) >> >> diff --git a/block/qcow2.h b/block/qcow2.h >> index 95d723d3c0..ce07f003f7 100644 >> --- a/block/qcow2.h >> +++ b/block/qcow2.h >> @@ -745,9 +745,9 @@ int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); >> int qcow2_add_persistent_dirty_bitmap(BlockDriverState *bs, >> BdrvDirtyBitmap *bitmap, >> Error **errp); >> -void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> - const char *name, >> - Error **errp); >> +int qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> + BdrvDirtyBitmap *bitmap, >> + Error **errp); >> >> ssize_t coroutine_fn >> qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size, >> diff --git a/include/block/block_int.h b/include/block/block_int.h >> index 93bbb66cd0..59f8cb9c12 100644 >> --- a/include/block/block_int.h >> +++ b/include/block/block_int.h >> @@ -540,9 +540,9 @@ struct BlockDriver { >> int (*bdrv_add_persistent_dirty_bitmap)(BlockDriverState *bs, >> BdrvDirtyBitmap *bitmap, >> Error **errp); >> - void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, >> - const char *name, >> - Error **errp); >> + int (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, >> + BdrvDirtyBitmap *bitmap, >> + Error **errp); >> >> /** >> * Register/unregister a buffer for I/O. For example, when the driver is >> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h >> index c37edbe05f..88a9832ddc 100644 >> --- a/include/block/dirty-bitmap.h >> +++ b/include/block/dirty-bitmap.h >> @@ -41,9 +41,9 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs); >> int bdrv_add_persistent_dirty_bitmap(BlockDriverState *bs, >> BdrvDirtyBitmap *bitmap, >> Error **errp); >> -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> - const char *name, >> - Error **errp); >> +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> + BdrvDirtyBitmap *bitmap, >> + Error **errp); >> void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap); >> void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap); >> void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap); >> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c >> index 615f8480b2..4667f9e65a 100644 >> --- a/block/dirty-bitmap.c >> +++ b/block/dirty-bitmap.c >> @@ -455,15 +455,30 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs) >> * BdrvDirtyBitmap can have .persistent = true but not yet saved and have no >> * stored version. For such bitmap bdrv_remove_persistent_dirty_bitmap() should >> * not fail. >> - * This function doesn't release corresponding BdrvDirtyBitmap. >> + * This function doesn't release the corresponding BdrvDirtyBitmap. >> */ >> -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> - const char *name, >> - Error **errp) >> +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> + BdrvDirtyBitmap *bitmap, >> + Error **errp) >> { >> + int ret = 0; >> + >> + if (!bdrv_dirty_bitmap_get_persistence(bitmap)) { >> + error_setg(errp, "Bitmap '%s' is not persistent, " >> + "so it cannot be removed from node '%s'", >> + bdrv_dirty_bitmap_name(bitmap), >> + bdrv_get_device_or_node_name(bs)); >> + return -EINVAL; >> + } >> + >> if (bs->drv && bs->drv->bdrv_remove_persistent_dirty_bitmap) { >> - bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, name, errp); >> + ret = bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, bitmap, errp); >> } >> + if (!ret) { >> + bdrv_dirty_bitmap_set_persistence(bitmap, false); >> + } >> + >> + return ret; >> } >> >> int bdrv_add_persistent_dirty_bitmap(BlockDriverState *bs, >> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c >> index c3a72ca782..930a6c91ff 100644 >> --- a/block/qcow2-bitmap.c >> +++ b/block/qcow2-bitmap.c >> @@ -1402,23 +1402,24 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list, >> return NULL; >> } >> >> -void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> - const char *name, >> - Error **errp) >> +int qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> + BdrvDirtyBitmap *bitmap, >> + Error **errp) >> { >> - int ret; >> + int ret = 0; > > dead assignment > >> BDRVQcow2State *s = bs->opaque; >> Qcow2Bitmap *bm; >> Qcow2BitmapList *bm_list; >> + const char *name = bdrv_dirty_bitmap_name(bitmap); >> >> if (s->nb_bitmaps == 0) { >> /* Absence of the bitmap is not an error: see explanation above >> * bdrv_remove_persistent_dirty_bitmap() definition. */ >> - return; >> + return 0; >> } >> >> - if (bitmap_list_load(bs, &bm_list, errp)) { >> - return; >> + if ((ret = bitmap_list_load(bs, &bm_list, errp))) { >> + return ret; >> } >> >> bm = find_bitmap_by_name(bm_list, name); > if (bm == NULL) { > goto fail; > } > > You don't set ret, as it's considered success. Worth s/fail/out then (preexisting, of course)? > This becomes an error in the next patch, so I'll omit this suggestion because the problem goes away soon anyway. >> @@ -1439,6 +1440,7 @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, >> fail: >> bitmap_free(bm); >> bitmap_list_free(bm_list); >> + return ret; >> } >> >> void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) >> diff --git a/blockdev.c b/blockdev.c >> index 169a8da831..82f02d8e72 100644 >> --- a/blockdev.c >> +++ b/blockdev.c >> @@ -2875,7 +2875,6 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, >> { >> BlockDriverState *bs; >> BdrvDirtyBitmap *bitmap; >> - Error *local_err = NULL; > > Oh, that's cool. And it was my mistake to create interfaces provoking endless local_errors.. > I was worried these first three patches might look useless, but I'm glad you like the idea of doing it this way. >> AioContext *aio_context = NULL; >> >> bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp); >> @@ -2889,20 +2888,18 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, >> } >> >> if (bdrv_dirty_bitmap_get_persistence(bitmap)) { >> + int ret; >> + >> aio_context = bdrv_get_aio_context(bs); > > and here, could you define aio_context in this block? > Indeed. >> aio_context_acquire(aio_context); >> - bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err); >> - if (local_err != NULL) { >> - error_propagate(errp, local_err); >> - goto out; >> + ret = bdrv_remove_persistent_dirty_bitmap(bs, bitmap, errp); >> + aio_context_release(aio_context); >> + if (ret) { >> + return; >> } >> } >> >> bdrv_release_dirty_bitmap(bs, bitmap); >> - out: >> - if (aio_context) { >> - aio_context_release(aio_context); >> - } >> } >> >> /** >> > > With some my suggestions or without: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Will take them, thanks for the review!
diff --git a/block/qcow2.h b/block/qcow2.h index 95d723d3c0..ce07f003f7 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -745,9 +745,9 @@ int qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp); int qcow2_add_persistent_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, Error **errp); -void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp); +int qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + Error **errp); ssize_t coroutine_fn qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size, diff --git a/include/block/block_int.h b/include/block/block_int.h index 93bbb66cd0..59f8cb9c12 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -540,9 +540,9 @@ struct BlockDriver { int (*bdrv_add_persistent_dirty_bitmap)(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, Error **errp); - void (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, - const char *name, - Error **errp); + int (*bdrv_remove_persistent_dirty_bitmap)(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + Error **errp); /** * Register/unregister a buffer for I/O. For example, when the driver is diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h index c37edbe05f..88a9832ddc 100644 --- a/include/block/dirty-bitmap.h +++ b/include/block/dirty-bitmap.h @@ -41,9 +41,9 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs); int bdrv_add_persistent_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, Error **errp); -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp); +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + Error **errp); void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap); void bdrv_enable_dirty_bitmap(BdrvDirtyBitmap *bitmap); void bdrv_enable_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap); diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c index 615f8480b2..4667f9e65a 100644 --- a/block/dirty-bitmap.c +++ b/block/dirty-bitmap.c @@ -455,15 +455,30 @@ void bdrv_release_named_dirty_bitmaps(BlockDriverState *bs) * BdrvDirtyBitmap can have .persistent = true but not yet saved and have no * stored version. For such bitmap bdrv_remove_persistent_dirty_bitmap() should * not fail. - * This function doesn't release corresponding BdrvDirtyBitmap. + * This function doesn't release the corresponding BdrvDirtyBitmap. */ -void bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp) +int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + Error **errp) { + int ret = 0; + + if (!bdrv_dirty_bitmap_get_persistence(bitmap)) { + error_setg(errp, "Bitmap '%s' is not persistent, " + "so it cannot be removed from node '%s'", + bdrv_dirty_bitmap_name(bitmap), + bdrv_get_device_or_node_name(bs)); + return -EINVAL; + } + if (bs->drv && bs->drv->bdrv_remove_persistent_dirty_bitmap) { - bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, name, errp); + ret = bs->drv->bdrv_remove_persistent_dirty_bitmap(bs, bitmap, errp); } + if (!ret) { + bdrv_dirty_bitmap_set_persistence(bitmap, false); + } + + return ret; } int bdrv_add_persistent_dirty_bitmap(BlockDriverState *bs, diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index c3a72ca782..930a6c91ff 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1402,23 +1402,24 @@ static Qcow2Bitmap *find_bitmap_by_name(Qcow2BitmapList *bm_list, return NULL; } -void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, - const char *name, - Error **errp) +int qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, + BdrvDirtyBitmap *bitmap, + Error **errp) { - int ret; + int ret = 0; BDRVQcow2State *s = bs->opaque; Qcow2Bitmap *bm; Qcow2BitmapList *bm_list; + const char *name = bdrv_dirty_bitmap_name(bitmap); if (s->nb_bitmaps == 0) { /* Absence of the bitmap is not an error: see explanation above * bdrv_remove_persistent_dirty_bitmap() definition. */ - return; + return 0; } - if (bitmap_list_load(bs, &bm_list, errp)) { - return; + if ((ret = bitmap_list_load(bs, &bm_list, errp))) { + return ret; } bm = find_bitmap_by_name(bm_list, name); @@ -1439,6 +1440,7 @@ void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, fail: bitmap_free(bm); bitmap_list_free(bm_list); + return ret; } void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) diff --git a/blockdev.c b/blockdev.c index 169a8da831..82f02d8e72 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2875,7 +2875,6 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, { BlockDriverState *bs; BdrvDirtyBitmap *bitmap; - Error *local_err = NULL; AioContext *aio_context = NULL; bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp); @@ -2889,20 +2888,18 @@ void qmp_block_dirty_bitmap_remove(const char *node, const char *name, } if (bdrv_dirty_bitmap_get_persistence(bitmap)) { + int ret; + aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); - bdrv_remove_persistent_dirty_bitmap(bs, name, &local_err); - if (local_err != NULL) { - error_propagate(errp, local_err); - goto out; + ret = bdrv_remove_persistent_dirty_bitmap(bs, bitmap, errp); + aio_context_release(aio_context); + if (ret) { + return; } } bdrv_release_dirty_bitmap(bs, bitmap); - out: - if (aio_context) { - aio_context_release(aio_context); - } } /**
Allow propagating error code information from bdrv_remove_persistent_dirty_bitmap as well. Give it an interface that matches the newly revised bdrv_add_persistent_dirty_bitmap, including removing the persistent flag when the operation succeeds and refusing to operate on bitmaps that are not persistent. Signed-off-by: John Snow <jsnow@redhat.com> --- block/qcow2.h | 6 +++--- include/block/block_int.h | 6 +++--- include/block/dirty-bitmap.h | 6 +++--- block/dirty-bitmap.c | 25 ++++++++++++++++++++----- block/qcow2-bitmap.c | 16 +++++++++------- blockdev.c | 15 ++++++--------- 6 files changed, 44 insertions(+), 30 deletions(-)