* [Qemu-devel] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory
@ 2017-11-30 16:47 Vladimir Sementsov-Ogievskiy
2017-11-30 16:47 ` [Qemu-devel] [PATCH 1/2] " Vladimir Sementsov-Ogievskiy
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-11-30 16:47 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: mreitz, kwolf
Add simple constant overlap check.
Vladimir Sementsov-Ogievskiy (2):
qcow2: add overlap check for bitmap directory
qcow2: fix indentation after previous patch
block/qcow2.h | 45 ++++++++++++++++++++++++---------------------
block/qcow2-refcount.c | 12 ++++++++++++
block/qcow2.c | 22 ++++++++++++++--------
3 files changed, 50 insertions(+), 29 deletions(-)
--
2.11.1
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2017-11-30 16:47 [Qemu-devel] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory Vladimir Sementsov-Ogievskiy
@ 2017-11-30 16:47 ` Vladimir Sementsov-Ogievskiy
2017-12-01 17:59 ` Eric Blake
` (2 more replies)
2017-11-30 16:47 ` [Qemu-devel] [PATCH 2/2] qcow2: fix indentation after previous patch Vladimir Sementsov-Ogievskiy
2018-03-16 22:21 ` [Qemu-devel] [Qemu-block] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory John Snow
2 siblings, 3 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-11-30 16:47 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: mreitz, kwolf
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
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 */
+ if (overlaps_with(s->bitmap_directory_offset,
+ s->bitmap_directory_size))
+ {
+ return QCOW2_OL_BITMAP_DIRECTORY;
+ }
+ }
+
return 0;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 1914a940e5..8278c0e124 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -655,6 +655,11 @@ static QemuOptsList qcow2_runtime_opts = {
.help = "Check for unintended writes into an inactive L2 table",
},
{
+ .name = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
+ .type = QEMU_OPT_BOOL,
+ .help = "Check for unintended writes into the bitmap directory",
+ },
+ {
.name = QCOW2_OPT_CACHE_SIZE,
.type = QEMU_OPT_SIZE,
.help = "Maximum combined metadata (L2 tables and refcount blocks) "
@@ -690,6 +695,7 @@ static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
[QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
[QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
[QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
+ [QCOW2_OL_BITMAP_DIRECTORY_BITNR] = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
};
static void cache_clean_timer_cb(void *opaque)
--
2.11.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH 2/2] qcow2: fix indentation after previous patch
2017-11-30 16:47 [Qemu-devel] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory Vladimir Sementsov-Ogievskiy
2017-11-30 16:47 ` [Qemu-devel] [PATCH 1/2] " Vladimir Sementsov-Ogievskiy
@ 2017-11-30 16:47 ` Vladimir Sementsov-Ogievskiy
2017-12-01 17:56 ` Eric Blake
2017-12-08 18:53 ` [Qemu-devel] [Qemu-block] " John Snow
2018-03-16 22:21 ` [Qemu-devel] [Qemu-block] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory John Snow
2 siblings, 2 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2017-11-30 16:47 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: mreitz, kwolf
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
block/qcow2.h | 34 +++++++++++++++++-----------------
block/qcow2.c | 16 ++++++++--------
2 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/block/qcow2.h b/block/qcow2.h
index 8f226a3609..896ad08e5b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -399,29 +399,29 @@ typedef enum QCow2ClusterType {
} QCow2ClusterType;
typedef enum QCow2MetadataOverlap {
- QCOW2_OL_MAIN_HEADER_BITNR = 0,
- QCOW2_OL_ACTIVE_L1_BITNR = 1,
- QCOW2_OL_ACTIVE_L2_BITNR = 2,
- QCOW2_OL_REFCOUNT_TABLE_BITNR = 3,
- QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4,
- QCOW2_OL_SNAPSHOT_TABLE_BITNR = 5,
- QCOW2_OL_INACTIVE_L1_BITNR = 6,
- QCOW2_OL_INACTIVE_L2_BITNR = 7,
+ QCOW2_OL_MAIN_HEADER_BITNR = 0,
+ QCOW2_OL_ACTIVE_L1_BITNR = 1,
+ QCOW2_OL_ACTIVE_L2_BITNR = 2,
+ QCOW2_OL_REFCOUNT_TABLE_BITNR = 3,
+ QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4,
+ 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 = 9,
- QCOW2_OL_NONE = 0,
- QCOW2_OL_MAIN_HEADER = (1 << QCOW2_OL_MAIN_HEADER_BITNR),
- QCOW2_OL_ACTIVE_L1 = (1 << QCOW2_OL_ACTIVE_L1_BITNR),
- QCOW2_OL_ACTIVE_L2 = (1 << QCOW2_OL_ACTIVE_L2_BITNR),
- QCOW2_OL_REFCOUNT_TABLE = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR),
- QCOW2_OL_REFCOUNT_BLOCK = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR),
- QCOW2_OL_SNAPSHOT_TABLE = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR),
- QCOW2_OL_INACTIVE_L1 = (1 << QCOW2_OL_INACTIVE_L1_BITNR),
+ QCOW2_OL_NONE = 0,
+ QCOW2_OL_MAIN_HEADER = (1 << QCOW2_OL_MAIN_HEADER_BITNR),
+ QCOW2_OL_ACTIVE_L1 = (1 << QCOW2_OL_ACTIVE_L1_BITNR),
+ QCOW2_OL_ACTIVE_L2 = (1 << QCOW2_OL_ACTIVE_L2_BITNR),
+ QCOW2_OL_REFCOUNT_TABLE = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR),
+ QCOW2_OL_REFCOUNT_BLOCK = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR),
+ QCOW2_OL_SNAPSHOT_TABLE = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR),
+ QCOW2_OL_INACTIVE_L1 = (1 << QCOW2_OL_INACTIVE_L1_BITNR),
/* NOTE: Checking overlaps with inactive L2 tables will result in bdrv
* reads. */
- QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_BITNR),
+ QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_BITNR),
QCOW2_OL_BITMAP_DIRECTORY = (1 << QCOW2_OL_BITMAP_DIRECTORY_BITNR),
} QCow2MetadataOverlap;
diff --git a/block/qcow2.c b/block/qcow2.c
index 8278c0e124..23f36a67c7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -687,14 +687,14 @@ static QemuOptsList qcow2_runtime_opts = {
};
static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
- [QCOW2_OL_MAIN_HEADER_BITNR] = QCOW2_OPT_OVERLAP_MAIN_HEADER,
- [QCOW2_OL_ACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L1,
- [QCOW2_OL_ACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L2,
- [QCOW2_OL_REFCOUNT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE,
- [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK,
- [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
- [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
- [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
+ [QCOW2_OL_MAIN_HEADER_BITNR] = QCOW2_OPT_OVERLAP_MAIN_HEADER,
+ [QCOW2_OL_ACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L1,
+ [QCOW2_OL_ACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L2,
+ [QCOW2_OL_REFCOUNT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE,
+ [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK,
+ [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
+ [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
+ [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
[QCOW2_OL_BITMAP_DIRECTORY_BITNR] = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
};
--
2.11.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] qcow2: fix indentation after previous patch
2017-11-30 16:47 ` [Qemu-devel] [PATCH 2/2] qcow2: fix indentation after previous patch Vladimir Sementsov-Ogievskiy
@ 2017-12-01 17:56 ` Eric Blake
2017-12-08 18:53 ` [Qemu-devel] [Qemu-block] " John Snow
1 sibling, 0 replies; 16+ messages in thread
From: Eric Blake @ 2017-12-01 17:56 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf, mreitz
On 11/30/2017 10:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2.h | 34 +++++++++++++++++-----------------
> block/qcow2.c | 16 ++++++++--------
> 2 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 8f226a3609..896ad08e5b 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -399,29 +399,29 @@ typedef enum QCow2ClusterType {
> } QCow2ClusterType;
>
> typedef enum QCow2MetadataOverlap {
> - QCOW2_OL_MAIN_HEADER_BITNR = 0,
> - QCOW2_OL_ACTIVE_L1_BITNR = 1,
> - QCOW2_OL_ACTIVE_L2_BITNR = 2,
> - QCOW2_OL_REFCOUNT_TABLE_BITNR = 3,
> - QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4,
> - QCOW2_OL_SNAPSHOT_TABLE_BITNR = 5,
> - QCOW2_OL_INACTIVE_L1_BITNR = 6,
> - QCOW2_OL_INACTIVE_L2_BITNR = 7,
> + QCOW2_OL_MAIN_HEADER_BITNR = 0,
> + QCOW2_OL_ACTIVE_L1_BITNR = 1,
This demonstrates why I'm a fan of single space before =:
foo = 1,
blah = 2,
because it is needless churn to realign everything if you add a new
longer line that doesn't fit with the earlier alignment of =. But we
don't have a clear preference for either code style, and I'm also okay
if you just squash this in with the previous patch or omit it altogether.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2017-11-30 16:47 ` [Qemu-devel] [PATCH 1/2] " Vladimir Sementsov-Ogievskiy
@ 2017-12-01 17:59 ` Eric Blake
2017-12-06 22:56 ` [Qemu-devel] [Qemu-block] " John Snow
2018-01-29 15:34 ` [Qemu-devel] " Max Reitz
2 siblings, 0 replies; 16+ messages in thread
From: Eric Blake @ 2017-12-01 17:59 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
Cc: kwolf, mreitz, Daniel P. Berrange
[adding Dan in cc]
On 11/30/2017 10:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2.h | 7 +++++--
> block/qcow2-refcount.c | 12 ++++++++++++
> block/qcow2.c | 6 ++++++
> 3 files changed, 23 insertions(+), 2 deletions(-)
>
Does the LUKS encryption header need similar overlap protection checks?
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 6f0ff15dd0..8f226a3609 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> +++ 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 */
Wording is awkward, how about:
The autoclear flag was previously dropped by
update_ext_header_and_dir_in_place, so this will not fail
but I'm not sure if that is the intended meaning.
> + if (overlaps_with(s->bitmap_directory_offset,
> + s->bitmap_directory_size))
> + {
> + return QCOW2_OL_BITMAP_DIRECTORY;
> + }
> + }
> +
> return 0;
> }
Do we need to add/update any iotests to cover this?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2017-11-30 16:47 ` [Qemu-devel] [PATCH 1/2] " Vladimir Sementsov-Ogievskiy
2017-12-01 17:59 ` Eric Blake
@ 2017-12-06 22:56 ` John Snow
[not found] ` <2af21d5f-3ad9-3492-03ea-c720929c4e36@virtuozzo.com>
2018-01-29 15:34 ` [Qemu-devel] " Max Reitz
2 siblings, 1 reply; 16+ messages in thread
From: John Snow @ 2017-12-06 22:56 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf, mreitz
On 11/30/2017 11:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 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 */
> + if (overlaps_with(s->bitmap_directory_offset,
> + s->bitmap_directory_size))
> + {
> + return QCOW2_OL_BITMAP_DIRECTORY;
> + }
> + }
> +
Isn't the purpose of this function to test if a given offset conflicts
with known regions of the file? I don't see you actually utilize the
'offset' parameter here, but maybe I don't understand what you're trying
to accomplish.
> return 0;
> }
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 1914a940e5..8278c0e124 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -655,6 +655,11 @@ static QemuOptsList qcow2_runtime_opts = {
> .help = "Check for unintended writes into an inactive L2 table",
> },
> {
> + .name = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
> + .type = QEMU_OPT_BOOL,
> + .help = "Check for unintended writes into the bitmap directory",
> + },
> + {
> .name = QCOW2_OPT_CACHE_SIZE,
> .type = QEMU_OPT_SIZE,
> .help = "Maximum combined metadata (L2 tables and refcount blocks) "
> @@ -690,6 +695,7 @@ static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
> [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
> [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
> [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
> + [QCOW2_OL_BITMAP_DIRECTORY_BITNR] = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
> };
>
> static void cache_clean_timer_cb(void *opaque)
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 1/2] qcow2: add overlap check for bitmap directory
[not found] ` <2af21d5f-3ad9-3492-03ea-c720929c4e36@virtuozzo.com>
@ 2017-12-07 17:03 ` John Snow
0 siblings, 0 replies; 16+ messages in thread
From: John Snow @ 2017-12-07 17:03 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf, mreitz
On 12/07/2017 04:43 AM, Vladimir Sementsov-Ogievskiy wrote:
> 07.12.2017 01:56, John Snow wrote:
>>
>> On 11/30/2017 11:47 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>> ---
>>> 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 */
>>> + if (overlaps_with(s->bitmap_directory_offset,
>>> + s->bitmap_directory_size))
>>> + {
>>> + return QCOW2_OL_BITMAP_DIRECTORY;
>>> + }
>>> + }
>>> +
>> Isn't the purpose of this function to test if a given offset conflicts
>> with known regions of the file? I don't see you actually utilize the
>> 'offset' parameter here, but maybe I don't understand what you're trying
>> to accomplish.
>
> #define overlaps_with(ofs, sz) \
> ranges_overlap(offset, size, ofs, sz)
>
> I've just copied one of similar blocks in qcow2_check_metadata_overlap()
>
Sigh, I didn't realize that was a macro. I don't really like lowercase
macros, but you didn't add it.
Reviewed-by: John Snow <jsnow@redhat.com>
Carry on...
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH 2/2] qcow2: fix indentation after previous patch
2017-11-30 16:47 ` [Qemu-devel] [PATCH 2/2] qcow2: fix indentation after previous patch Vladimir Sementsov-Ogievskiy
2017-12-01 17:56 ` Eric Blake
@ 2017-12-08 18:53 ` John Snow
1 sibling, 0 replies; 16+ messages in thread
From: John Snow @ 2017-12-08 18:53 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf, mreitz
On 11/30/2017 11:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2.h | 34 +++++++++++++++++-----------------
> block/qcow2.c | 16 ++++++++--------
> 2 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 8f226a3609..896ad08e5b 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -399,29 +399,29 @@ typedef enum QCow2ClusterType {
> } QCow2ClusterType;
>
> typedef enum QCow2MetadataOverlap {
> - QCOW2_OL_MAIN_HEADER_BITNR = 0,
> - QCOW2_OL_ACTIVE_L1_BITNR = 1,
> - QCOW2_OL_ACTIVE_L2_BITNR = 2,
> - QCOW2_OL_REFCOUNT_TABLE_BITNR = 3,
> - QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4,
> - QCOW2_OL_SNAPSHOT_TABLE_BITNR = 5,
> - QCOW2_OL_INACTIVE_L1_BITNR = 6,
> - QCOW2_OL_INACTIVE_L2_BITNR = 7,
> + QCOW2_OL_MAIN_HEADER_BITNR = 0,
> + QCOW2_OL_ACTIVE_L1_BITNR = 1,
> + QCOW2_OL_ACTIVE_L2_BITNR = 2,
> + QCOW2_OL_REFCOUNT_TABLE_BITNR = 3,
> + QCOW2_OL_REFCOUNT_BLOCK_BITNR = 4,
> + 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 = 9,
>
> - QCOW2_OL_NONE = 0,
> - QCOW2_OL_MAIN_HEADER = (1 << QCOW2_OL_MAIN_HEADER_BITNR),
> - QCOW2_OL_ACTIVE_L1 = (1 << QCOW2_OL_ACTIVE_L1_BITNR),
> - QCOW2_OL_ACTIVE_L2 = (1 << QCOW2_OL_ACTIVE_L2_BITNR),
> - QCOW2_OL_REFCOUNT_TABLE = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR),
> - QCOW2_OL_REFCOUNT_BLOCK = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR),
> - QCOW2_OL_SNAPSHOT_TABLE = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR),
> - QCOW2_OL_INACTIVE_L1 = (1 << QCOW2_OL_INACTIVE_L1_BITNR),
> + QCOW2_OL_NONE = 0,
> + QCOW2_OL_MAIN_HEADER = (1 << QCOW2_OL_MAIN_HEADER_BITNR),
> + QCOW2_OL_ACTIVE_L1 = (1 << QCOW2_OL_ACTIVE_L1_BITNR),
> + QCOW2_OL_ACTIVE_L2 = (1 << QCOW2_OL_ACTIVE_L2_BITNR),
> + QCOW2_OL_REFCOUNT_TABLE = (1 << QCOW2_OL_REFCOUNT_TABLE_BITNR),
> + QCOW2_OL_REFCOUNT_BLOCK = (1 << QCOW2_OL_REFCOUNT_BLOCK_BITNR),
> + QCOW2_OL_SNAPSHOT_TABLE = (1 << QCOW2_OL_SNAPSHOT_TABLE_BITNR),
> + QCOW2_OL_INACTIVE_L1 = (1 << QCOW2_OL_INACTIVE_L1_BITNR),
> /* NOTE: Checking overlaps with inactive L2 tables will result in bdrv
> * reads. */
> - QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_BITNR),
> + QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_BITNR),
> QCOW2_OL_BITMAP_DIRECTORY = (1 << QCOW2_OL_BITMAP_DIRECTORY_BITNR),
> } QCow2MetadataOverlap;
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8278c0e124..23f36a67c7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -687,14 +687,14 @@ static QemuOptsList qcow2_runtime_opts = {
> };
>
> static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
> - [QCOW2_OL_MAIN_HEADER_BITNR] = QCOW2_OPT_OVERLAP_MAIN_HEADER,
> - [QCOW2_OL_ACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L1,
> - [QCOW2_OL_ACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L2,
> - [QCOW2_OL_REFCOUNT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE,
> - [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK,
> - [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
> - [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
> - [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
> + [QCOW2_OL_MAIN_HEADER_BITNR] = QCOW2_OPT_OVERLAP_MAIN_HEADER,
> + [QCOW2_OL_ACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L1,
> + [QCOW2_OL_ACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_ACTIVE_L2,
> + [QCOW2_OL_REFCOUNT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE,
> + [QCOW2_OL_REFCOUNT_BLOCK_BITNR] = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK,
> + [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
> + [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
> + [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
> [QCOW2_OL_BITMAP_DIRECTORY_BITNR] = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
> };
>
>
Squash it in, or don't, either way:
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2017-11-30 16:47 ` [Qemu-devel] [PATCH 1/2] " Vladimir Sementsov-Ogievskiy
2017-12-01 17:59 ` Eric Blake
2017-12-06 22:56 ` [Qemu-devel] [Qemu-block] " John Snow
@ 2018-01-29 15:34 ` Max Reitz
2018-02-02 12:07 ` Vladimir Sementsov-Ogievskiy
2 siblings, 1 reply; 16+ messages in thread
From: Max Reitz @ 2018-01-29 15:34 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf
[-- Attachment #1: Type: text/plain, Size: 4180 bytes --]
On 2017-11-30 17:47, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> 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.)
Max
> + if (overlaps_with(s->bitmap_directory_offset,
> + s->bitmap_directory_size))
> + {
> + return QCOW2_OL_BITMAP_DIRECTORY;
> + }
> + }
> +
> return 0;
> }
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 1914a940e5..8278c0e124 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -655,6 +655,11 @@ static QemuOptsList qcow2_runtime_opts = {
> .help = "Check for unintended writes into an inactive L2 table",
> },
> {
> + .name = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
> + .type = QEMU_OPT_BOOL,
> + .help = "Check for unintended writes into the bitmap directory",
> + },
> + {
> .name = QCOW2_OPT_CACHE_SIZE,
> .type = QEMU_OPT_SIZE,
> .help = "Maximum combined metadata (L2 tables and refcount blocks) "
> @@ -690,6 +695,7 @@ static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
> [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
> [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
> [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
> + [QCOW2_OL_BITMAP_DIRECTORY_BITNR] = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
> };
>
> static void cache_clean_timer_cb(void *opaque)
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2018-01-29 15:34 ` [Qemu-devel] " Max Reitz
@ 2018-02-02 12:07 ` Vladimir Sementsov-Ogievskiy
2018-02-02 13:00 ` Max Reitz
0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-02-02 12:07 UTC (permalink / raw)
To: Max Reitz, qemu-devel, qemu-block; +Cc: kwolf
29.01.2018 18:34, Max Reitz wrote:
> On 2017-11-30 17:47, Vladimir Sementsov-Ogievskiy wrote:
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> 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.
But this is done with cleared autoclear flag (to make it safe), so we
will skip this check and will not
fail.
>
> Max
>
>> + if (overlaps_with(s->bitmap_directory_offset,
>> + s->bitmap_directory_size))
>> + {
>> + return QCOW2_OL_BITMAP_DIRECTORY;
>> + }
>> + }
>> +
>> return 0;
>> }
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 1914a940e5..8278c0e124 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -655,6 +655,11 @@ static QemuOptsList qcow2_runtime_opts = {
>> .help = "Check for unintended writes into an inactive L2 table",
>> },
>> {
>> + .name = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
>> + .type = QEMU_OPT_BOOL,
>> + .help = "Check for unintended writes into the bitmap directory",
>> + },
>> + {
>> .name = QCOW2_OPT_CACHE_SIZE,
>> .type = QEMU_OPT_SIZE,
>> .help = "Maximum combined metadata (L2 tables and refcount blocks) "
>> @@ -690,6 +695,7 @@ static const char *overlap_bool_option_names[QCOW2_OL_MAX_BITNR] = {
>> [QCOW2_OL_SNAPSHOT_TABLE_BITNR] = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE,
>> [QCOW2_OL_INACTIVE_L1_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L1,
>> [QCOW2_OL_INACTIVE_L2_BITNR] = QCOW2_OPT_OVERLAP_INACTIVE_L2,
>> + [QCOW2_OL_BITMAP_DIRECTORY_BITNR] = QCOW2_OPT_OVERLAP_BITMAP_DIRECTORY,
>> };
>>
>> static void cache_clean_timer_cb(void *opaque)
>>
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2018-02-02 12:07 ` Vladimir Sementsov-Ogievskiy
@ 2018-02-02 13:00 ` Max Reitz
2018-02-02 13:48 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 16+ messages in thread
From: Max Reitz @ 2018-02-02 13:00 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf
[-- Attachment #1: Type: text/plain, Size: 3582 bytes --]
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 <vsementsov@virtuozzo.com>
>>> ---
>>> 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? :-)
Max
> But this is done with cleared autoclear flag (to make it safe), so we
> will skip this check and will not
> fail.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2018-02-02 13:00 ` Max Reitz
@ 2018-02-02 13:48 ` Vladimir Sementsov-Ogievskiy
2018-02-02 13:53 ` Max Reitz
0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-02-02 13:48 UTC (permalink / raw)
To: Max Reitz, qemu-devel, qemu-block; +Cc: kwolf
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 <vsementsov@virtuozzo.com>
>>>> ---
>>>> 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?
>
> Max
>
>> But this is done with cleared autoclear flag (to make it safe), so we
>> will skip this check and will not
>> fail.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2018-02-02 13:48 ` Vladimir Sementsov-Ogievskiy
@ 2018-02-02 13:53 ` Max Reitz
2018-02-02 14:28 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 16+ messages in thread
From: Max Reitz @ 2018-02-02 13:53 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf
[-- Attachment #1: Type: text/plain, Size: 4067 bytes --]
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 <vsementsov@virtuozzo.com>
>>>>> ---
>>>>> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory
2018-02-02 13:53 ` Max Reitz
@ 2018-02-02 14:28 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-02-02 14:28 UTC (permalink / raw)
To: Max Reitz, qemu-devel, qemu-block; +Cc: kwolf
02.02.2018 16:53, Max Reitz wrote:
> 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 <vsementsov@virtuozzo.com>
>>>>>> ---
>>>>>> 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
>
Ok, reasonable. I'll respin with that fixed.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory
2017-11-30 16:47 [Qemu-devel] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory Vladimir Sementsov-Ogievskiy
2017-11-30 16:47 ` [Qemu-devel] [PATCH 1/2] " Vladimir Sementsov-Ogievskiy
2017-11-30 16:47 ` [Qemu-devel] [PATCH 2/2] qcow2: fix indentation after previous patch Vladimir Sementsov-Ogievskiy
@ 2018-03-16 22:21 ` John Snow
2018-03-19 6:49 ` Vladimir Sementsov-Ogievskiy
2 siblings, 1 reply; 16+ messages in thread
From: John Snow @ 2018-03-16 22:21 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block; +Cc: kwolf, mreitz
On 11/30/2017 11:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add simple constant overlap check.
>
> Vladimir Sementsov-Ogievskiy (2):
> qcow2: add overlap check for bitmap directory
> qcow2: fix indentation after previous patch
>
> block/qcow2.h | 45 ++++++++++++++++++++++++---------------------
> block/qcow2-refcount.c | 12 ++++++++++++
> block/qcow2.c | 22 ++++++++++++++--------
> 3 files changed, 50 insertions(+), 29 deletions(-)
>
Vladimir, do we need this for 2.12 still?
How about "fix bitmaps migration through shared storage"?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory
2018-03-16 22:21 ` [Qemu-devel] [Qemu-block] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory John Snow
@ 2018-03-19 6:49 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-03-19 6:49 UTC (permalink / raw)
To: John Snow, qemu-devel, qemu-block; +Cc: kwolf, mreitz
17.03.2018 01:21, John Snow wrote:
>
> On 11/30/2017 11:47 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Add simple constant overlap check.
>>
>> Vladimir Sementsov-Ogievskiy (2):
>> qcow2: add overlap check for bitmap directory
>> qcow2: fix indentation after previous patch
>>
>> block/qcow2.h | 45 ++++++++++++++++++++++++---------------------
>> block/qcow2-refcount.c | 12 ++++++++++++
>> block/qcow2.c | 22 ++++++++++++++--------
>> 3 files changed, 50 insertions(+), 29 deletions(-)
>>
> Vladimir, do we need this for 2.12 still?
> How about "fix bitmaps migration through shared storage"?
Ohh, my fault. Will look at them today.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-03-19 6:50 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-30 16:47 [Qemu-devel] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory Vladimir Sementsov-Ogievskiy
2017-11-30 16:47 ` [Qemu-devel] [PATCH 1/2] " Vladimir Sementsov-Ogievskiy
2017-12-01 17:59 ` Eric Blake
2017-12-06 22:56 ` [Qemu-devel] [Qemu-block] " John Snow
[not found] ` <2af21d5f-3ad9-3492-03ea-c720929c4e36@virtuozzo.com>
2017-12-07 17:03 ` John Snow
2018-01-29 15:34 ` [Qemu-devel] " Max Reitz
2018-02-02 12:07 ` Vladimir Sementsov-Ogievskiy
2018-02-02 13:00 ` Max Reitz
2018-02-02 13:48 ` Vladimir Sementsov-Ogievskiy
2018-02-02 13:53 ` Max Reitz
2018-02-02 14:28 ` Vladimir Sementsov-Ogievskiy
2017-11-30 16:47 ` [Qemu-devel] [PATCH 2/2] qcow2: fix indentation after previous patch Vladimir Sementsov-Ogievskiy
2017-12-01 17:56 ` Eric Blake
2017-12-08 18:53 ` [Qemu-devel] [Qemu-block] " John Snow
2018-03-16 22:21 ` [Qemu-devel] [Qemu-block] [PATCH for-2.12 0/2] qcow2: add overlap check for bitmap directory John Snow
2018-03-19 6:49 ` Vladimir Sementsov-Ogievskiy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.