[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/6] bloc/qcow2: drop dirty_bitmaps_loaded state
From: |
John Snow |
Subject: |
Re: [Qemu-devel] [PATCH 3/6] bloc/qcow2: drop dirty_bitmaps_loaded state variable |
Date: |
Mon, 9 Jul 2018 19:25:06 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 06/26/2018 09:50 AM, Vladimir Sementsov-Ogievskiy wrote:
> This variable doesn't work as it should, because it is actually cleared
> in qcow2_co_invalidate_cache() by memset(). Drop it, as the following
> patch will introduce new behavior.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> block/qcow2.h | 1 -
> block/qcow2.c | 19 ++-----------------
> 2 files changed, 2 insertions(+), 18 deletions(-)
>
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 01b5250415..c9df19b6e6 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -294,7 +294,6 @@ typedef struct BDRVQcow2State {
> uint32_t nb_bitmaps;
> uint64_t bitmap_directory_size;
> uint64_t bitmap_directory_offset;
> - bool dirty_bitmaps_loaded;
>
> int flags;
> int qcow_version;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 46194a33ca..0044ff58e7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1149,7 +1149,6 @@ static int coroutine_fn qcow2_do_open(BlockDriverState
> *bs, QDict *options,
> uint64_t ext_end;
> uint64_t l1_vm_state_index;
> bool update_header = false;
> - bool header_updated = false;
>
> ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
> if (ret < 0) {
> @@ -1488,23 +1487,9 @@ static int coroutine_fn qcow2_do_open(BlockDriverState
> *bs, QDict *options,
> s->autoclear_features &= QCOW2_AUTOCLEAR_MASK;
> }
>
> - if (s->dirty_bitmaps_loaded) {
> - /* It's some kind of reopen. There are no known cases where we need
> to
> - * reload bitmaps in such a situation, so it's safer to skip them.
> - *
> - * Moreover, if we have some readonly bitmaps and we are reopening
> for
> - * rw we should reopen bitmaps correspondingly.
> - */
> - if (bdrv_has_readonly_bitmaps(bs) &&
> - !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) &
> BDRV_O_INACTIVE))
> - {
> - qcow2_reopen_bitmaps_rw_hint(bs, &header_updated, &local_err);
> - }
> - } else {
> - header_updated = qcow2_load_dirty_bitmaps(bs, &local_err);
> - s->dirty_bitmaps_loaded = true;
> + if (qcow2_load_dirty_bitmaps(bs, &local_err)) {
> + update_header = false;
> }
> - update_header = update_header && !header_updated;
> if (local_err != NULL) {
> error_propagate(errp, local_err);
> ret = -EINVAL;
>
Usually I'd like to see the new behavior introduced before removing the
old, imperfect solution... (it doesn't make any tests fail!? ...)
At this point in the series I just have to trust that you will fix it to
be better :)
--js
- Re: [Qemu-devel] [PATCH 3/6] bloc/qcow2: drop dirty_bitmaps_loaded state variable,
John Snow <=