[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 05/20] dirty-bitmap: Check for size query fai
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH v7 05/20] dirty-bitmap: Check for size query failure during truncate |
Date: |
Wed, 13 Sep 2017 19:27:30 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
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 <address@hidden>
>
> ---
> 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?
- [Qemu-devel] [PATCH v7 00/20] make dirty-bitmap byte-based, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 01/20] block: Make bdrv_img_create() size selection easier to read, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 02/20] hbitmap: Rename serialization_granularity to serialization_align, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 03/20] qcow2: Ensure bitmap serialization is aligned, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 04/20] dirty-bitmap: Drop unused functions, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 05/20] dirty-bitmap: Check for size query failure during truncate, Eric Blake, 2017/09/12
- Re: [Qemu-devel] [PATCH v7 05/20] dirty-bitmap: Check for size query failure during truncate,
John Snow <=
- [Qemu-devel] [PATCH v7 06/20] dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 07/20] dirty-bitmap: Track bitmap size by bytes, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 08/20] dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to take bytes, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 09/20] qcow2: Switch sectors_covered_by_bitmap_cluster() to byte-based, Eric Blake, 2017/09/12
- [Qemu-devel] [PATCH v7 11/20] dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset, Eric Blake, 2017/09/12