* [Qemu-devel] [PATCH v3] qcow2: add overlap check for bitmap directory
@ 2018-07-05 9:21 Vladimir Sementsov-Ogievskiy
2018-07-05 10:46 ` Kevin Wolf
0 siblings, 1 reply; 3+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-07-05 9:21 UTC (permalink / raw)
To: qemu-devel, qemu-block; +Cc: armbru, eblake, mreitz, kwolf, vsementsov, den
If it appropriate for 3.0, let's push it. If not - then for 3.1
with fixed "since". Should I cc stable?
v3: - update Qcow2OverlapCheckFlags in qapi/block-core.json [Max]
v2: - squash 02 (indentation fix) to 01
- drop comment from qcow2_check_metadata_overlap()
- set @ign to QCOW2_OL_BITMAP_DIRECTORY for in-place case in
bitmap_list_store. I don't think non-inplace case should be changed,
as it don't touch active bitmap directory.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
qapi/block-core.json | 21 ++++++++++++---------
block/qcow2.h | 45 ++++++++++++++++++++++++---------------------
block/qcow2-bitmap.c | 7 ++++++-
block/qcow2-refcount.c | 10 ++++++++++
block/qcow2.c | 22 ++++++++++++++--------
5 files changed, 66 insertions(+), 39 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 90e554ed0f..e7393e0313 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2695,18 +2695,21 @@
# @template: Specifies a template mode which can be adjusted using the other
# flags, defaults to 'cached'
#
+# @bitmap-directory: since 3.0
+#
# Since: 2.9
##
{ 'struct': 'Qcow2OverlapCheckFlags',
- 'data': { '*template': 'Qcow2OverlapCheckMode',
- '*main-header': 'bool',
- '*active-l1': 'bool',
- '*active-l2': 'bool',
- '*refcount-table': 'bool',
- '*refcount-block': 'bool',
- '*snapshot-table': 'bool',
- '*inactive-l1': 'bool',
- '*inactive-l2': 'bool' } }
+ 'data': { '*template': 'Qcow2OverlapCheckMode',
+ '*main-header': 'bool',
+ '*active-l1': 'bool',
+ '*active-l2': 'bool',
+ '*refcount-table': 'bool',
+ '*refcount-block': 'bool',
+ '*snapshot-table': 'bool',
+ '*inactive-l1': 'bool',
+ '*inactive-l2': 'bool',
+ '*bitmap-directory': 'bool' } }
##
# @Qcow2OverlapChecks:
diff --git a/block/qcow2.h b/block/qcow2.h
index 1c9c0d3631..f90e6e2de2 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -94,6 +94,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_L2_CACHE_ENTRY_SIZE "l2-cache-entry-size"
@@ -397,34 +398,36 @@ 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_MAX_BITNR = 8,
-
- 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_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),
/* 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;
/* 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-bitmap.c b/block/qcow2-bitmap.c
index 69485aa1de..ba978ad2aa 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -775,7 +775,12 @@ static int bitmap_list_store(BlockDriverState *bs, Qcow2BitmapList *bm_list,
}
}
- ret = qcow2_pre_write_overlap_check(bs, 0, dir_offset, dir_size);
+ /* Actually, even in in-place case ignoring QCOW2_OL_BITMAP_DIRECTORY is not
+ * necessary, because we drop QCOW2_AUTOCLEAR_BITMAPS when updating bitmap
+ * directory in-place (actually, turn-off the extension), which is checked
+ * in qcow2_check_metadata_overlap() */
+ ret = qcow2_pre_write_overlap_check(
+ bs, in_place ? QCOW2_OL_BITMAP_DIRECTORY : 0, dir_offset, dir_size);
if (ret < 0) {
goto fail;
}
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 18c729aa27..1b9ecb1ca0 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -2705,6 +2705,16 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
}
}
+ if ((chk & QCOW2_OL_BITMAP_DIRECTORY) &&
+ (s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS))
+ {
+ 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 2f9e58e0c4..af51220499 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -676,6 +676,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) "
@@ -708,14 +713,15 @@ 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,
};
static void cache_clean_timer_cb(void *opaque)
--
2.11.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] qcow2: add overlap check for bitmap directory
2018-07-05 9:21 [Qemu-devel] [PATCH v3] qcow2: add overlap check for bitmap directory Vladimir Sementsov-Ogievskiy
@ 2018-07-05 10:46 ` Kevin Wolf
2018-07-05 14:48 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 3+ messages in thread
From: Kevin Wolf @ 2018-07-05 10:46 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy
Cc: qemu-devel, qemu-block, armbru, eblake, mreitz, den
Am 05.07.2018 um 11:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
> If it appropriate for 3.0, let's push it. If not - then for 3.1
> with fixed "since". Should I cc stable?
>
> v3: - update Qcow2OverlapCheckFlags in qapi/block-core.json [Max]
>
> v2: - squash 02 (indentation fix) to 01
> - drop comment from qcow2_check_metadata_overlap()
> - set @ign to QCOW2_OL_BITMAP_DIRECTORY for in-place case in
> bitmap_list_store. I don't think non-inplace case should be changed,
> as it don't touch active bitmap directory.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> qapi/block-core.json | 21 ++++++++++++---------
> block/qcow2.h | 45 ++++++++++++++++++++++++---------------------
> block/qcow2-bitmap.c | 7 ++++++-
> block/qcow2-refcount.c | 10 ++++++++++
> block/qcow2.c | 22 ++++++++++++++--------
> 5 files changed, 66 insertions(+), 39 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 90e554ed0f..e7393e0313 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2695,18 +2695,21 @@
> # @template: Specifies a template mode which can be adjusted using the other
> # flags, defaults to 'cached'
> #
> +# @bitmap-directory: since 3.0
> +#
> # Since: 2.9
> ##
> { 'struct': 'Qcow2OverlapCheckFlags',
> - 'data': { '*template': 'Qcow2OverlapCheckMode',
> - '*main-header': 'bool',
> - '*active-l1': 'bool',
> - '*active-l2': 'bool',
> - '*refcount-table': 'bool',
> - '*refcount-block': 'bool',
> - '*snapshot-table': 'bool',
> - '*inactive-l1': 'bool',
> - '*inactive-l2': 'bool' } }
> + 'data': { '*template': 'Qcow2OverlapCheckMode',
> + '*main-header': 'bool',
> + '*active-l1': 'bool',
> + '*active-l2': 'bool',
> + '*refcount-table': 'bool',
> + '*refcount-block': 'bool',
> + '*snapshot-table': 'bool',
> + '*inactive-l1': 'bool',
> + '*inactive-l2': 'bool',
> + '*bitmap-directory': 'bool' } }
Did you intend to add two spaces to each line? Because this still isn't
aligned to the same column.
Kevin
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v3] qcow2: add overlap check for bitmap directory
2018-07-05 10:46 ` Kevin Wolf
@ 2018-07-05 14:48 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 3+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2018-07-05 14:48 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, armbru, eblake, mreitz, den
05.07.2018 13:46, Kevin Wolf wrote:
> Am 05.07.2018 um 11:21 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> If it appropriate for 3.0, let's push it. If not - then for 3.1
>> with fixed "since". Should I cc stable?
>>
>> v3: - update Qcow2OverlapCheckFlags in qapi/block-core.json [Max]
>>
>> v2: - squash 02 (indentation fix) to 01
>> - drop comment from qcow2_check_metadata_overlap()
>> - set @ign to QCOW2_OL_BITMAP_DIRECTORY for in-place case in
>> bitmap_list_store. I don't think non-inplace case should be changed,
>> as it don't touch active bitmap directory.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>> qapi/block-core.json | 21 ++++++++++++---------
>> block/qcow2.h | 45 ++++++++++++++++++++++++---------------------
>> block/qcow2-bitmap.c | 7 ++++++-
>> block/qcow2-refcount.c | 10 ++++++++++
>> block/qcow2.c | 22 ++++++++++++++--------
>> 5 files changed, 66 insertions(+), 39 deletions(-)
>>
>> diff --git a/qapi/block-core.json b/qapi/block-core.json
>> index 90e554ed0f..e7393e0313 100644
>> --- a/qapi/block-core.json
>> +++ b/qapi/block-core.json
>> @@ -2695,18 +2695,21 @@
>> # @template: Specifies a template mode which can be adjusted using the other
>> # flags, defaults to 'cached'
>> #
>> +# @bitmap-directory: since 3.0
>> +#
>> # Since: 2.9
>> ##
>> { 'struct': 'Qcow2OverlapCheckFlags',
>> - 'data': { '*template': 'Qcow2OverlapCheckMode',
>> - '*main-header': 'bool',
>> - '*active-l1': 'bool',
>> - '*active-l2': 'bool',
>> - '*refcount-table': 'bool',
>> - '*refcount-block': 'bool',
>> - '*snapshot-table': 'bool',
>> - '*inactive-l1': 'bool',
>> - '*inactive-l2': 'bool' } }
>> + 'data': { '*template': 'Qcow2OverlapCheckMode',
>> + '*main-header': 'bool',
>> + '*active-l1': 'bool',
>> + '*active-l2': 'bool',
>> + '*refcount-table': 'bool',
>> + '*refcount-block': 'bool',
>> + '*snapshot-table': 'bool',
>> + '*inactive-l1': 'bool',
>> + '*inactive-l2': 'bool',
>> + '*bitmap-directory': 'bool' } }
> Did you intend to add two spaces to each line? Because this still isn't
> aligned to the same column.
>
> Kevin
Aha, I forget to add '*', and added it late, after indentation.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-07-05 14:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-05 9:21 [Qemu-devel] [PATCH v3] qcow2: add overlap check for bitmap directory Vladimir Sementsov-Ogievskiy
2018-07-05 10:46 ` Kevin Wolf
2018-07-05 14:48 ` Vladimir Sementsov-Ogievskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).