[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 05/20] dirty-bitmap: Avoid size query failur
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v10 05/20] dirty-bitmap: Avoid size query failure during truncate |
Date: |
Tue, 26 Sep 2017 09:25:38 +0800 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Mon, 09/25 09:55, 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() take the new size from the
> caller instead of querying itself; then adjust the sole caller
> bdrv_truncate() to pass the size just determined by a successful
> resize, or to skip the bitmap resize on failure, thus avoiding
> sizing the bitmaps to -1. This also fixes a bug where not all
> failure paths in bdrv_truncate() would set errp.
>
> Note that bdrv_truncate() is still a bit awkward. We may want
> to revisit it later and clean up things to better guarantee that
> a resize attempt either fails cleanly up front, or cannot fail
> after guest-visible changes have been made (if temporary changes
> are made, then they need to be cleanly rolled back). But that
> is a task for another day; for now, the goal is the bare minimum
> fix to ensure that just bdrv_dirty_bitmap_truncate() cannot fail.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v10: always resize bitmap as before [John], enhance commit message to
> point out errp bugfix [Vladimir]
> v9: skip only bdrv_dirty_bitmap_truncate on error [Fam]
> v8: retitle and rework to avoid possibility of secondary failure [John]
> v7: new patch [Kevin]
> ---
> include/block/dirty-bitmap.h | 2 +-
> block.c | 16 +++++++++++-----
> block/dirty-bitmap.c | 6 +++---
> 3 files changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
> index 8fd842eac9..7a27590047 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);
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes);
> 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 528cda7b2c..ef5af81f66 100644
> --- a/block.c
> +++ b/block.c
> @@ -3545,12 +3545,18 @@ 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");
> + } else {
> + offset = bs->total_sectors * BDRV_SECTOR_SIZE;
> + }
> + bdrv_dirty_bitmap_truncate(bs, offset);
> + bdrv_parent_cb_resize(bs);
> + atomic_inc(&bs->write_gen);
> return ret;
> }
>
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 42a55e4a4b..ee164fb518 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -1,7 +1,7 @@
> /*
> * Block Dirty Bitmap
> *
> - * Copyright (c) 2016 Red Hat. Inc
> + * Copyright (c) 2016-2017 Red Hat. Inc
> *
> * Permission is hereby granted, free of charge, to any person obtaining a
> copy
> * of this software and associated documentation files (the "Software"), to
> deal
> @@ -302,10 +302,10 @@ BdrvDirtyBitmap
> *bdrv_reclaim_dirty_bitmap(BlockDriverState *bs,
> * Truncates _all_ bitmaps attached to a BDS.
> * Called with BQL taken.
> */
> -void bdrv_dirty_bitmap_truncate(BlockDriverState *bs)
> +void bdrv_dirty_bitmap_truncate(BlockDriverState *bs, int64_t bytes)
> {
> BdrvDirtyBitmap *bitmap;
> - uint64_t size = bdrv_nb_sectors(bs);
> + int64_t size = DIV_ROUND_UP(bytes, BDRV_SECTOR_SIZE);
>
> bdrv_dirty_bitmaps_lock(bs);
> QLIST_FOREACH(bitmap, &bs->dirty_bitmaps, list) {
> --
> 2.13.5
>
Reviewed-by: Fam Zheng <address@hidden>
- [Qemu-devel] [PATCH v10 00/20] make dirty-bitmap byte-based, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 01/20] block: Make bdrv_img_create() size selection easier to read, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 03/20] qcow2: Ensure bitmap serialization is aligned, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 02/20] hbitmap: Rename serialization_granularity to serialization_align, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 05/20] dirty-bitmap: Avoid size query failure during truncate, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 04/20] dirty-bitmap: Drop unused functions, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 06/20] dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 07/20] dirty-bitmap: Track bitmap size by bytes, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 08/20] dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to take bytes, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 09/20] qcow2: Switch sectors_covered_by_bitmap_cluster() to byte-based, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 10/20] dirty-bitmap: Set iterator start by offset, not sector, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 11/20] dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 12/20] dirty-bitmap: Change bdrv_get_dirty_count() to report bytes, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 13/20] dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes, Eric Blake, 2017/09/25
- [Qemu-devel] [PATCH v10 15/20] mirror: Switch mirror_dirty_init() to byte-based iteration, Eric Blake, 2017/09/25