[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directo
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory |
Date: |
Fri, 2 Feb 2018 14:53:51 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 2018-02-02 14:48, Vladimir Sementsov-Ogievskiy wrote:
> 02.02.2018 16:00, Max Reitz wrote:
>> On 2018-02-02 13:07, Vladimir Sementsov-Ogievskiy wrote:
>>> 29.01.2018 18:34, Max Reitz wrote:
>>>> On 2017-11-30 17:47, Vladimir Sementsov-Ogievskiy wrote:
>>>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>>>>> ---
>>>>> block/qcow2.h | 7 +++++--
>>>>> block/qcow2-refcount.c | 12 ++++++++++++
>>>>> block/qcow2.c | 6 ++++++
>>>>> 3 files changed, 23 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/block/qcow2.h b/block/qcow2.h
>>>>> index 6f0ff15dd0..8f226a3609 100644
>>>>> --- a/block/qcow2.h
>>>>> +++ b/block/qcow2.h
>>>>> @@ -98,6 +98,7 @@
>>>>> #define QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE
>>>>> "overlap-check.snapshot-table"
>>>>> #define QCOW2_OPT_OVERLAP_INACTIVE_L1 "overlap-check.inactive-l1"
>>>>> #define QCOW2_OPT_OVERLAP_INACTIVE_L2 "overlap-check.inactive-l2"
>>>>> +#define QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY
>>>>> "overlap-check.bitmap-directory"
>>>>> #define QCOW2_OPT_CACHE_SIZE "cache-size"
>>>>> #define QCOW2_OPT_L2_CACHE_SIZE "l2-cache-size"
>>>>> #define QCOW2_OPT_REFCOUNT_CACHE_SIZE "refcount-cache-size"
>>>>> @@ -406,8 +407,9 @@ typedef enum QCow2MetadataOverlap {
>>>>> QCOW2_OL_SNAPSHOT_TABLE_BITNR = 5,
>>>>> QCOW2_OL_INACTIVE_L1_BITNR = 6,
>>>>> QCOW2_OL_INACTIVE_L2_BITNR = 7,
>>>>> + QCOW2_OL_BITMAP_DIRECTORY_BITNR = 8,
>>>>> - QCOW2_OL_MAX_BITNR = 8,
>>>>> + QCOW2_OL_MAX_BITNR = 9,
>>>>> QCOW2_OL_NONE = 0,
>>>>> QCOW2_OL_MAIN_HEADER = (1 << QCOW2_OL_MAIN_HEADER_BITNR),
>>>>> @@ -420,12 +422,13 @@ typedef enum QCow2MetadataOverlap {
>>>>> /* NOTE: Checking overlaps with inactive L2 tables will result
>>>>> in bdrv
>>>>> * reads. */
>>>>> QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_BITNR),
>>>>> + QCOW2_OL_BITMAP_DIRECTORY = (1 <<
>>>>> QCOW2_OL_BITMAP_DIRECTORY_BITNR),
>>>>> } QCow2MetadataOverlap;
>>>>> /* Perform all overlap checks which can be done in constant
>>>>> time */
>>>>> #define QCOW2_OL_CONSTANT \
>>>>> (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 |
>>>>> QCOW2_OL_REFCOUNT_TABLE | \
>>>>> - QCOW2_OL_SNAPSHOT_TABLE)
>>>>> + QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_BITMAP_DIRECTORY)
>>>>> /* Perform all overlap checks which don't require disk access */
>>>>> #define QCOW2_OL_CACHED \
>>>>> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
>>>>> index 3de1ab51ba..a7a2703f26 100644
>>>>> --- a/block/qcow2-refcount.c
>>>>> +++ b/block/qcow2-refcount.c
>>>>> @@ -2585,6 +2585,18 @@ int
>>>>> qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t
>>>>> offset,
>>>>> }
>>>>> }
>>>>> + if ((chk & QCOW2_OL_BITMAP_DIRECTORY) &&
>>>>> + (s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS))
>>>>> + {
>>>>> + /* update_ext_header_and_dir_in_place firstly drop autoclear
>>>>> flag,
>>>>> + * so it will not fail */
>>>> That's really not an argument. bitmap_list_store() has to pass
>>>> QCOW2_OL_BITMAP_DIRECTORY to @ign anyway. (Because there is no reason
>>>> not to.)
>>> in_place is a reason. When we store directory in_place, it definitely
>>> overlaps with current directory.
>> Well, then you just pass QCOW2_OL_BITMAP_DIRECTORY to @ign, which is
>> what that argument is for? :-)
>
> hmm. but actually, I should not, because of zeroed autoclear flag. So,
> do you think, it is better to pass it, anyway?
Yes. That flag describes what kind of metadata structures you are
planning to overwrite, and you *are* planning to overwrite the bitmap
directory, so you should set it.
Max
signature.asc
Description: OpenPGP digital signature