On 09/12/2017 04:31 PM, Eric Blake wrote:
> We've previously fixed several places where we failed to account
> for possible errors from bdrv_nb_sectors(). Fix another one by
> making bdrv_dirty_bitmap_truncate() report the error rather then
> silently resizing bitmaps to -1. Then adjust the sole caller
Nice failure mode. It was not immediately obvious to me that this could
fail, but here we all are.
> bdrv_truncate() to both reduce the likelihood of failure (blindly
> calling bdrv_dirty_bitmap_truncate() after refresh_total_sectors()
> fails was not nice) as well as propagate any actual failures.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v7: new patch [Kevin]
> ---
> include/block/dirty-bitmap.h | 2 +-
> block.c | 19 ++++++++++++++-----
> block/dirty-bitmap.c | 12 ++++++++----
> 3 files changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 8fd842eac9..15101b59d5 100644
> --- a/include/block/dirty-bitmap.h
> +++ b/include/block/dirty-bitmap.h
> @@ -83,7 +83,7 @@ int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
> void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
> int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
> int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
> +int bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
> bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
> bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
> bool bdrv_dirty_bitmap_get_autoload(const BdrvDirtyBitmap *bitmap);
> diff --git a/block.c b/block.c
> index ee6a48976e..790dcce360 100644
> --- a/block.c
> +++ b/block.c
> @@ -3450,12 +3450,21 @@ int bdrv_truncate(BdrvChild *child, int64_t offset,
> PreallocMode prealloc,
> assert(!(bs->open_flags & BDRV_O_INACTIVE));
>
> ret = drv->bdrv_truncate(bs, offset, prealloc, errp);
> - if (ret == 0) {
> - ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> - bdrv_dirty_bitmap_truncate(bs);
> - bdrv_parent_cb_resize(bs);
> - atomic_inc(&bs->write_gen);
> + if (ret < 0) {
> + return ret;
> }
> + ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not refresh total sector count");
> + return ret;
> + }
> + ret = bdrv_dirty_bitmap_truncate(bs);
> + if (ret < 0) {
> + error_setg_errno(errp, -ret, "Could not refresh total sector count");
You probably meant to write a different error message here.
Also, what happens if the actual truncate call works, but the bitmap
resizing fails? What state does that leave us in?
