| Message ID | 20170711160814.20941-51-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11 July 2017 at 17:07, Max Reitz <mreitz@redhat.com> wrote: > From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > Realize .bdrv_remove_persistent_dirty_bitmap interface. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Reviewed-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: John Snow <jsnow@redhat.com> > Message-id: 20170628120530.31251-29-vsementsov@virtuozzo.com > Signed-off-by: Max Reitz <mreitz@redhat.com> > +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, > + const char *name, > + Error **errp) > +{ > + int ret; > + BDRVQcow2State *s = bs->opaque; > + Qcow2Bitmap *bm; > + Qcow2BitmapList *bm_list; > + > + if (s->nb_bitmaps == 0) { > + /* Absence of the bitmap is not an error: see explanation above > + * bdrv_remove_persistent_dirty_bitmap() definition. */ > + return; > + } > + > + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset, > + s->bitmap_directory_size, errp); > + if (bm_list == NULL) { > + return; > + } > + > + bm = find_bitmap_by_name(bm_list, name); > + if (bm == NULL) { > + goto fail; > + } > + > + QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry); > + > + ret = update_ext_header_and_dir(bs, bm_list); > + if (ret < 0) { > + error_setg_errno(errp, -ret, "Failed to update bitmap extension"); > + goto fail; > + } > + > + free_bitmap_clusters(bs, &bm->table); > + > +fail: > + bitmap_free(bm); > + bitmap_list_free(bm_list); > +} Coverity points out that this can crash in the error-exit paths, because bitmap_free() doesn't handle being passed a NULL pointer. (CID 1377700). Probably the best fix for this is to make bitmap_free() do nothing when handed NULL. thanks -- PMM
diff --git a/block/qcow2.h b/block/qcow2.h index 084a298..2df05ac 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -653,5 +653,8 @@ bool qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, uint32_t granularity, Error **errp); +void qcow2_remove_persistent_dirty_bitmap(BlockDriverState *bs, + const char *name, + Error **errp); #endif diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index a8c7e1d..3e8735a 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1237,6 +1237,47 @@ 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 ret; + BDRVQcow2State *s = bs->opaque; + Qcow2Bitmap *bm; + Qcow2BitmapList *bm_list; + + if (s->nb_bitmaps == 0) { + /* Absence of the bitmap is not an error: see explanation above + * bdrv_remove_persistent_dirty_bitmap() definition. */ + return; + } + + bm_list = bitmap_list_load(bs, s->bitmap_directory_offset, + s->bitmap_directory_size, errp); + if (bm_list == NULL) { + return; + } + + bm = find_bitmap_by_name(bm_list, name); + if (bm == NULL) { + goto fail; + } + + QSIMPLEQ_REMOVE(bm_list, bm, Qcow2Bitmap, entry); + + ret = update_ext_header_and_dir(bs, bm_list); + if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to update bitmap extension"); + goto fail; + } + + free_bitmap_clusters(bs, &bm->table); + +fail: + bitmap_free(bm); + bitmap_list_free(bm_list); +} + void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp) { BdrvDirtyBitmap *bitmap; diff --git a/block/qcow2.c b/block/qcow2.c index d7046a3..9d3c70e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -3990,6 +3990,7 @@ BlockDriver bdrv_qcow2 = { .bdrv_reopen_bitmaps_rw = qcow2_reopen_bitmaps_rw, .bdrv_can_store_new_dirty_bitmap = qcow2_can_store_new_dirty_bitmap, + .bdrv_remove_persistent_dirty_bitmap = qcow2_remove_persistent_dirty_bitmap, }; static void bdrv_qcow2_init(void)