[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v10 05/20] dirty-bitmap: Avoid size query failur
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH v10 05/20] dirty-bitmap: Avoid size query failure during truncate |
Date: |
Mon, 25 Sep 2017 10:46:05 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 09/25/2017 10:23 AM, Vladimir Sementsov-Ogievskiy wrote:
> 25.09.2017 17: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
>
> nothing is skipped in this version
So much for me squashing in a change. If the maintainer is willing
(Kevin, is this going through your tree?), we can reword that to:
"...pass the size just determined by a successful resize, or to reuse
the size given to the original truncate operation when
refresh_total_sectors() was not able to confirm the actual size (the two
sizes can potentially differ according to rounding constraints), 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>
>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <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) {
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-block] [PATCH v10 00/20] make dirty-bitmap byte-based, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 01/20] block: Make bdrv_img_create() size selection easier to read, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 03/20] qcow2: Ensure bitmap serialization is aligned, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 02/20] hbitmap: Rename serialization_granularity to serialization_align, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 04/20] dirty-bitmap: Drop unused functions, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 05/20] dirty-bitmap: Avoid size query failure during truncate, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 06/20] dirty-bitmap: Change bdrv_dirty_bitmap_size() to report bytes, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 07/20] dirty-bitmap: Track bitmap size by bytes, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 08/20] dirty-bitmap: Change bdrv_dirty_bitmap_*serialize*() to take bytes, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 09/20] qcow2: Switch sectors_covered_by_bitmap_cluster() to byte-based, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 10/20] dirty-bitmap: Set iterator start by offset, not sector, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 11/20] dirty-bitmap: Change bdrv_dirty_iter_next() to report byte offset, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 12/20] dirty-bitmap: Change bdrv_get_dirty_count() to report bytes, Eric Blake, 2017/09/25
- [Qemu-block] [PATCH v10 13/20] dirty-bitmap: Change bdrv_get_dirty_locked() to take bytes, Eric Blake, 2017/09/25