* [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime
@ 2013-09-20 8:37 Max Reitz
2013-09-20 8:37 ` [Qemu-devel] [PATCH 1/6] qcow2: Use negated overflow check mask Max Reitz
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Max Reitz @ 2013-09-20 8:37 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
This series changes the way of selecting what metadata overlap checks to
perform from (currently) using a macro to using a variable contained in
BDRVQcowState which can be configured at runtime through several command
line options.
Although this does not seem necessary anymore regarding performance
(because I could not find any performance hits using cached overlap
checks), it seems cleaner to me this way.
This series depends on:
- qcow2: Correct snapshots size for overlap check
Max Reitz (6):
qcow2: Use negated overflow check mask
qcow2: Make overlap check mask variable
qcow2: Add overlap-check options
qcow2: Array assigning options to OL check bits
qcow2: Add more overlap check bitmask macros
qcow2: Evaluate overlap check options
block/qcow2-cache.c | 8 ++---
block/qcow2-cluster.c | 16 ++++-----
block/qcow2-refcount.c | 22 ++++++------
block/qcow2-snapshot.c | 12 +++----
block/qcow2.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++---
block/qcow2.h | 30 ++++++++++++----
6 files changed, 137 insertions(+), 44 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH 1/6] qcow2: Use negated overflow check mask 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz @ 2013-09-20 8:37 ` Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 2/6] qcow2: Make overlap check mask variable Max Reitz ` (5 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2013-09-20 8:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz In qcow2_check_metadata_overlap and qcow2_pre_write_overlap_check, change the parameter signifying the checks to perform from its current positive form to a negative one, i.e., it will no longer explicitly specify every check to perform but rather a mask of checks not to perform. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2-cache.c | 8 +++----- block/qcow2-cluster.c | 16 +++++++--------- block/qcow2-refcount.c | 22 ++++++++++------------ block/qcow2-snapshot.c | 12 +++++------- block/qcow2.c | 7 +++---- block/qcow2.h | 4 ++-- 6 files changed, 30 insertions(+), 39 deletions(-) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 40a5a3f..8ecbb5b 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -115,15 +115,13 @@ static int qcow2_cache_entry_flush(BlockDriverState *bs, Qcow2Cache *c, int i) } if (c == s->refcount_block_cache) { - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_REFCOUNT_BLOCK, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_REFCOUNT_BLOCK, c->entries[i].offset, s->cluster_size); } else if (c == s->l2_table_cache) { - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L2, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2, c->entries[i].offset, s->cluster_size); } else { - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, + ret = qcow2_pre_write_overlap_check(bs, 0, c->entries[i].offset, s->cluster_size); } diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 738ff73..6577de1 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -82,8 +82,8 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, /* the L1 position has not yet been updated, so these clusters must * indeed be completely free */ - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - new_l1_table_offset, new_l1_size2); + ret = qcow2_pre_write_overlap_check(bs, 0, new_l1_table_offset, + new_l1_size2); if (ret < 0) { goto fail; } @@ -157,8 +157,7 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index) buf[i] = cpu_to_be64(s->l1_table[l1_start_index + i]); } - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L1, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, s->l1_table_offset + 8 * l1_start_index, sizeof(buf)); if (ret < 0) { return ret; @@ -383,7 +382,7 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs, &s->aes_encrypt_key); } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, + ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset + n_start * BDRV_SECTOR_SIZE, n * BDRV_SECTOR_SIZE); if (ret < 0) { goto out; @@ -1592,8 +1591,7 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, } } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - offset, s->cluster_size); + ret = qcow2_pre_write_overlap_check(bs, 0, offset, s->cluster_size); if (ret < 0) { qcow2_free_clusters(bs, offset, s->cluster_size, QCOW2_DISCARD_ALWAYS); @@ -1628,8 +1626,8 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, } } else { if (l2_dirty) { - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT & - ~(QCOW2_OL_INACTIVE_L2 | QCOW2_OL_ACTIVE_L2), l2_offset, + ret = qcow2_pre_write_overlap_check(bs, + QCOW2_OL_INACTIVE_L2 | QCOW2_OL_ACTIVE_L2, l2_offset, s->cluster_size); if (ret < 0) { goto fail; diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 4264148..2fa6acb 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1311,9 +1311,8 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, } if (l2_dirty) { - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L2, l2_offset, - s->cluster_size); + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2, + l2_offset, s->cluster_size); if (ret < 0) { fprintf(stderr, "ERROR: Could not write L2 table; metadata " "overlap check failed: %s\n", strerror(-ret)); @@ -1354,8 +1353,7 @@ static int write_reftable_entry(BlockDriverState *bs, int rt_index) buf[i] = cpu_to_be64(s->refcount_table[rt_start_index + i]); } - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_REFCOUNT_TABLE, + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_REFCOUNT_TABLE, s->refcount_table_offset + rt_start_index * sizeof(uint64_t), sizeof(buf)); if (ret < 0) { @@ -1406,8 +1404,7 @@ static int64_t realloc_refcount_block(BlockDriverState *bs, int reftable_index, /* new block has not yet been entered into refcount table, therefore it is * no refcount block yet (regarding this check) */ - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, new_offset, - s->cluster_size); + ret = qcow2_pre_write_overlap_check(bs, 0, new_offset, s->cluster_size); if (ret < 0) { fprintf(stderr, "Could not write refcount block; metadata overlap " "check failed: %s\n", strerror(-ret)); @@ -1640,8 +1637,8 @@ fail: * looking for overlaps with important metadata sections (L1/L2 tables etc.), * i.e. a sanity check without relying on the refcount tables. * - * The chk parameter specifies exactly what checks to perform (being a bitmask - * of QCow2MetadataOverlap values). + * The ign parameter specifies what checks not to perform (being a bitmask of + * QCow2MetadataOverlap values), i.e., what sections to ignore. * * Returns: * - 0 if writing to this offset will not affect the mentioned metadata @@ -1649,10 +1646,11 @@ fail: * - a negative value (-errno) indicating an error while performing a check, * e.g. when bdrv_read failed on QCOW2_OL_INACTIVE_L2 */ -int qcow2_check_metadata_overlap(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, int64_t size) { BDRVQcowState *s = bs->opaque; + int chk = QCOW2_OL_DEFAULT & ~ign; int i, j; if (!size) { @@ -1769,10 +1767,10 @@ static const char *metadata_ol_names[] = { * Returns 0 if there were neither overlaps nor errors while checking for * overlaps; or a negative value (-errno) on error. */ -int qcow2_pre_write_overlap_check(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size) { - int ret = qcow2_check_metadata_overlap(bs, chk, offset, size); + int ret = qcow2_check_metadata_overlap(bs, ign, offset, size); if (ret < 0) { return ret; diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 5e8a779..f4b3eac 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -191,8 +191,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs) /* The snapshot list position has not yet been updated, so these clusters * must indeed be completely free */ - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, offset, - snapshots_size); + ret = qcow2_pre_write_overlap_check(bs, 0, offset, snapshots_size); if (ret < 0) { return ret; } @@ -388,8 +387,8 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info) l1_table[i] = cpu_to_be64(s->l1_table[i]); } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - sn->l1_table_offset, s->l1_size * sizeof(uint64_t)); + ret = qcow2_pre_write_overlap_check(bs, 0, sn->l1_table_offset, + s->l1_size * sizeof(uint64_t)); if (ret < 0) { goto fail; } @@ -513,9 +512,8 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) goto fail; } - ret = qcow2_pre_write_overlap_check(bs, - QCOW2_OL_DEFAULT & ~QCOW2_OL_ACTIVE_L1, - s->l1_table_offset, cur_l1_bytes); + ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L1, + s->l1_table_offset, cur_l1_bytes); if (ret < 0) { goto fail; } diff --git a/block/qcow2.c b/block/qcow2.c index 318d95d..13c9d5e 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -965,7 +965,7 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs, cur_nr_sectors * 512); } - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, + ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset + index_in_cluster * BDRV_SECTOR_SIZE, cur_nr_sectors * BDRV_SECTOR_SIZE); if (ret < 0) { @@ -1739,7 +1739,7 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num, if (ret != Z_STREAM_END || out_len >= s->cluster_size) { /* could not compress: write normal cluster */ - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, + ret = qcow2_pre_write_overlap_check(bs, 0, sector_num * BDRV_SECTOR_SIZE, s->cluster_sectors * BDRV_SECTOR_SIZE); if (ret < 0) { @@ -1759,8 +1759,7 @@ static int qcow2_write_compressed(BlockDriverState *bs, int64_t sector_num, } cluster_offset &= s->cluster_offset_mask; - ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_DEFAULT, - cluster_offset, out_len); + ret = qcow2_pre_write_overlap_check(bs, 0, cluster_offset, out_len); if (ret < 0) { goto fail; } diff --git a/block/qcow2.h b/block/qcow2.h index c90e5d6..b5a158f 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -433,9 +433,9 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res, void qcow2_process_discards(BlockDriverState *bs, int ret); -int qcow2_check_metadata_overlap(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, int64_t size); -int qcow2_pre_write_overlap_check(BlockDriverState *bs, int chk, int64_t offset, +int qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset, int64_t size); /* qcow2-cluster.c functions */ -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 2/6] qcow2: Make overlap check mask variable 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 1/6] qcow2: Use negated overflow check mask Max Reitz @ 2013-09-20 8:37 ` Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 3/6] qcow2: Add overlap-check options Max Reitz ` (4 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2013-09-20 8:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz Replace the QCOW2_OL_DEFAULT macro by a variable overlap_check in BDRVQcowState. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2-refcount.c | 2 +- block/qcow2.c | 2 ++ block/qcow2.h | 5 ++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 2fa6acb..3edd558 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1650,7 +1650,7 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset, int64_t size) { BDRVQcowState *s = bs->opaque; - int chk = QCOW2_OL_DEFAULT & ~ign; + int chk = s->overlap_check & ~ign; int i, j; if (!size) { diff --git a/block/qcow2.c b/block/qcow2.c index 13c9d5e..a1b854c 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -631,6 +631,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, s->discard_passthrough[QCOW2_DISCARD_OTHER] = qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false); + s->overlap_check = QCOW2_OL_CACHED; + qemu_opts_del(opts); if (s->use_lazy_refcounts && s->qcow_version < 3) { diff --git a/block/qcow2.h b/block/qcow2.h index b5a158f..7dd787d 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -203,6 +203,8 @@ typedef struct BDRVQcowState { bool discard_passthrough[QCOW2_DISCARD_MAX]; + int overlap_check; /* bitmask of Qcow2MetadataOverlap values */ + uint64_t incompatible_features; uint64_t compatible_features; uint64_t autoclear_features; @@ -321,9 +323,6 @@ typedef enum QCow2MetadataOverlap { QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \ QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1) -/* The default checks to perform */ -#define QCOW2_OL_DEFAULT QCOW2_OL_CACHED - #define L1E_OFFSET_MASK 0x00ffffffffffff00ULL #define L2E_OFFSET_MASK 0x00ffffffffffff00ULL #define L2E_COMPRESSED_OFFSET_SIZE_MASK 0x3fffffffffffffffULL -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 3/6] qcow2: Add overlap-check options 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 1/6] qcow2: Use negated overflow check mask Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 2/6] qcow2: Make overlap check mask variable Max Reitz @ 2013-09-20 8:37 ` Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 4/6] qcow2: Array assigning options to OL check bits Max Reitz ` (3 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2013-09-20 8:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz Add runtime options to tune the overlap checks to be performed before write accesses. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ block/qcow2.h | 9 +++++++++ 2 files changed, 55 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index a1b854c..e74f269 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -354,6 +354,52 @@ static QemuOptsList qcow2_runtime_opts = { .type = QEMU_OPT_BOOL, .help = "Generate discard requests when other clusters are freed", }, + { + .name = QCOW2_OPT_OVERLAP, + .type = QEMU_OPT_STRING, + .help = "Selects which overlap checks to perform from a range of " + "templates (none, constant, cached, all)", + }, + { + .name = QCOW2_OPT_OVERLAP_MAIN_HEADER, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the main qcow2 header", + }, + { + .name = QCOW2_OPT_OVERLAP_ACTIVE_L1, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the active L1 table", + }, + { + .name = QCOW2_OPT_OVERLAP_ACTIVE_L2, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into an active L2 table", + }, + { + .name = QCOW2_OPT_OVERLAP_REFCOUNT_TABLE, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the refcount table", + }, + { + .name = QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into a refcount block", + }, + { + .name = QCOW2_OPT_OVERLAP_SNAPSHOT_TABLE, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into the snapshot table", + }, + { + .name = QCOW2_OPT_OVERLAP_INACTIVE_L1, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into an inactive L1 table", + }, + { + .name = QCOW2_OPT_OVERLAP_INACTIVE_L2, + .type = QEMU_OPT_BOOL, + .help = "Check for unintended writes into an inactive L2 table", + }, { /* end of list */ } }, }; diff --git a/block/qcow2.h b/block/qcow2.h index 7dd787d..067ed2f 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -63,6 +63,15 @@ #define QCOW2_OPT_DISCARD_REQUEST "pass-discard-request" #define QCOW2_OPT_DISCARD_SNAPSHOT "pass-discard-snapshot" #define QCOW2_OPT_DISCARD_OTHER "pass-discard-other" +#define QCOW2_OPT_OVERLAP "overlap-check" +#define QCOW2_OPT_OVERLAP_MAIN_HEADER "overlap-check.main-header" +#define QCOW2_OPT_OVERLAP_ACTIVE_L1 "overlap-check.active-l1" +#define QCOW2_OPT_OVERLAP_ACTIVE_L2 "overlap-check.active-l2" +#define QCOW2_OPT_OVERLAP_REFCOUNT_TABLE "overlap-check.refcount-table" +#define QCOW2_OPT_OVERLAP_REFCOUNT_BLOCK "overlap-check.refcount-block" +#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" typedef struct QCowHeader { uint32_t magic; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 4/6] qcow2: Array assigning options to OL check bits 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz ` (2 preceding siblings ...) 2013-09-20 8:37 ` [Qemu-devel] [PATCH 3/6] qcow2: Add overlap-check options Max Reitz @ 2013-09-20 8:37 ` Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros Max Reitz ` (2 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2013-09-20 8:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz Add an array which assigns the option string to its corresponding overlap check bit. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/block/qcow2.c b/block/qcow2.c index e74f269..66afeca 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -404,6 +404,17 @@ 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, +}; + static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz ` (3 preceding siblings ...) 2013-09-20 8:37 ` [Qemu-devel] [PATCH 4/6] qcow2: Array assigning options to OL check bits Max Reitz @ 2013-09-20 8:37 ` Max Reitz 2013-10-09 13:07 ` Kevin Wolf 2013-09-20 8:37 ` [Qemu-devel] [PATCH 6/6] qcow2: Evaluate overlap check options Max Reitz 2013-10-09 10:17 ` [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz 6 siblings, 1 reply; 10+ messages in thread From: Max Reitz @ 2013-09-20 8:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz Introduces the macros QCOW2_OL_CONSTANT and QCOW2_OL_ALL in addition to the already existing QCOW2_OL_CACHED, signifying all metadata overlap checks that can be performed in constant time (regardless of image size etc.) and truly all available overlap checks, respectively. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 067ed2f..098c14f 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -326,11 +326,19 @@ typedef enum QCow2MetadataOverlap { QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_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) + /* Perform all overlap checks which don't require disk access */ #define QCOW2_OL_CACHED \ - (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_ACTIVE_L2 | \ - QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \ - QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1) + (QCOW2_OL_CONSTANT | QCOW2_OL_ACTIVE_L2 | QCOW2_OL_REFCOUNT_BLOCK | \ + QCOW2_OL_SNAPSHOT_TABLE) + +/* Perform all overlap checks */ +#define QCOW2_OL_ALL \ + (QCOW2_OL_CACHED | QCOW2_OL_INACTIVE_L2) #define L1E_OFFSET_MASK 0x00ffffffffffff00ULL #define L2E_OFFSET_MASK 0x00ffffffffffff00ULL -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros 2013-09-20 8:37 ` [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros Max Reitz @ 2013-10-09 13:07 ` Kevin Wolf 2013-10-09 13:22 ` Max Reitz 0 siblings, 1 reply; 10+ messages in thread From: Kevin Wolf @ 2013-10-09 13:07 UTC (permalink / raw) To: Max Reitz; +Cc: qemu-devel, Stefan Hajnoczi Am 20.09.2013 um 10:37 hat Max Reitz geschrieben: > Introduces the macros QCOW2_OL_CONSTANT and QCOW2_OL_ALL in addition to > the already existing QCOW2_OL_CACHED, signifying all metadata overlap > checks that can be performed in constant time (regardless of image size > etc.) and truly all available overlap checks, respectively. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/qcow2.h | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/block/qcow2.h b/block/qcow2.h > index 067ed2f..098c14f 100644 > --- a/block/qcow2.h > +++ b/block/qcow2.h > @@ -326,11 +326,19 @@ typedef enum QCow2MetadataOverlap { > QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_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) > + > /* Perform all overlap checks which don't require disk access */ > #define QCOW2_OL_CACHED \ > - (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_ACTIVE_L2 | \ > - QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \ > - QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1) > + (QCOW2_OL_CONSTANT | QCOW2_OL_ACTIVE_L2 | QCOW2_OL_REFCOUNT_BLOCK | \ > + QCOW2_OL_SNAPSHOT_TABLE) QCOW2_OL_INACTIVE_L1 is lost here. > +/* Perform all overlap checks */ > +#define QCOW2_OL_ALL \ > + (QCOW2_OL_CACHED | QCOW2_OL_INACTIVE_L2) Kevin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros 2013-10-09 13:07 ` Kevin Wolf @ 2013-10-09 13:22 ` Max Reitz 0 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2013-10-09 13:22 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, Stefan Hajnoczi On 2013-10-09 15:07, Kevin Wolf wrote: > Am 20.09.2013 um 10:37 hat Max Reitz geschrieben: >> Introduces the macros QCOW2_OL_CONSTANT and QCOW2_OL_ALL in addition to >> the already existing QCOW2_OL_CACHED, signifying all metadata overlap >> checks that can be performed in constant time (regardless of image size >> etc.) and truly all available overlap checks, respectively. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> block/qcow2.h | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/block/qcow2.h b/block/qcow2.h >> index 067ed2f..098c14f 100644 >> --- a/block/qcow2.h >> +++ b/block/qcow2.h >> @@ -326,11 +326,19 @@ typedef enum QCow2MetadataOverlap { >> QCOW2_OL_INACTIVE_L2 = (1 << QCOW2_OL_INACTIVE_L2_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) >> + >> /* Perform all overlap checks which don't require disk access */ >> #define QCOW2_OL_CACHED \ >> - (QCOW2_OL_MAIN_HEADER | QCOW2_OL_ACTIVE_L1 | QCOW2_OL_ACTIVE_L2 | \ >> - QCOW2_OL_REFCOUNT_TABLE | QCOW2_OL_REFCOUNT_BLOCK | \ >> - QCOW2_OL_SNAPSHOT_TABLE | QCOW2_OL_INACTIVE_L1) >> + (QCOW2_OL_CONSTANT | QCOW2_OL_ACTIVE_L2 | QCOW2_OL_REFCOUNT_BLOCK | \ >> + QCOW2_OL_SNAPSHOT_TABLE) > QCOW2_OL_INACTIVE_L1 is lost here. Oh, right, I'll fix it. Thanks for reviewing, by the way. :) Max >> +/* Perform all overlap checks */ >> +#define QCOW2_OL_ALL \ >> + (QCOW2_OL_CACHED | QCOW2_OL_INACTIVE_L2) > Kevin ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH 6/6] qcow2: Evaluate overlap check options 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz ` (4 preceding siblings ...) 2013-09-20 8:37 ` [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros Max Reitz @ 2013-09-20 8:37 ` Max Reitz 2013-10-09 10:17 ` [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz 6 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2013-09-20 8:37 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz Evaluate the runtime overlap check options and set BDRVQcowState.overlap_check appropriately. Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/block/qcow2.c b/block/qcow2.c index 66afeca..df3dc9d 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -425,6 +425,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, Error *local_err = NULL; uint64_t ext_end; uint64_t l1_vm_state_index; + const char *opt_overlap_check; + int overlap_check_template = 0; ret = bdrv_pread(bs->file, 0, &header, sizeof(header)); if (ret < 0) { @@ -688,7 +690,32 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags, s->discard_passthrough[QCOW2_DISCARD_OTHER] = qemu_opt_get_bool(opts, QCOW2_OPT_DISCARD_OTHER, false); - s->overlap_check = QCOW2_OL_CACHED; + opt_overlap_check = qemu_opt_get(opts, "overlap-check") ?: "cached"; + if (!strcmp(opt_overlap_check, "none")) { + overlap_check_template = 0; + } else if (!strcmp(opt_overlap_check, "constant")) { + overlap_check_template = QCOW2_OL_CONSTANT; + } else if (!strcmp(opt_overlap_check, "cached")) { + overlap_check_template = QCOW2_OL_CACHED; + } else if (!strcmp(opt_overlap_check, "all")) { + overlap_check_template = QCOW2_OL_ALL; + } else { + error_setg(errp, "Unsupported value '%s' for qcow2 option " + "'overlap-check'. Allowed are either of the following: " + "none, constant, cached, all", opt_overlap_check); + qemu_opts_del(opts); + ret = -EINVAL; + goto fail; + } + + s->overlap_check = 0; + for (i = 0; i < QCOW2_OL_MAX_BITNR; i++) { + /* overlap-check defines a template bitmask, but every flag may be + * overwritten through the associated boolean option */ + s->overlap_check |= + qemu_opt_get_bool(opts, overlap_bool_option_names[i], + overlap_check_template & (1 << i)) << i; + } qemu_opts_del(opts); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz ` (5 preceding siblings ...) 2013-09-20 8:37 ` [Qemu-devel] [PATCH 6/6] qcow2: Evaluate overlap check options Max Reitz @ 2013-10-09 10:17 ` Max Reitz 6 siblings, 0 replies; 10+ messages in thread From: Max Reitz @ 2013-10-09 10:17 UTC (permalink / raw) To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi On 2013-09-20 10:37, Max Reitz wrote: > This series changes the way of selecting what metadata overlap checks to > perform from (currently) using a macro to using a variable contained in > BDRVQcowState which can be configured at runtime through several command > line options. > > Although this does not seem necessary anymore regarding performance > (because I could not find any performance hits using cached overlap > checks), it seems cleaner to me this way. > > This series depends on: > - qcow2: Correct snapshots size for overlap check > > Max Reitz (6): > qcow2: Use negated overflow check mask > qcow2: Make overlap check mask variable > qcow2: Add overlap-check options > qcow2: Array assigning options to OL check bits > qcow2: Add more overlap check bitmask macros > qcow2: Evaluate overlap check options > > block/qcow2-cache.c | 8 ++--- > block/qcow2-cluster.c | 16 ++++----- > block/qcow2-refcount.c | 22 ++++++------ > block/qcow2-snapshot.c | 12 +++---- > block/qcow2.c | 93 +++++++++++++++++++++++++++++++++++++++++++++++--- > block/qcow2.h | 30 ++++++++++++---- > 6 files changed, 137 insertions(+), 44 deletions(-) Ping – does anyone have comments on this series? It doesn't really seem to be required, since the current default setting is fast enough (although I still have to do a performance test regarding the host CPU usage). However, this series introduces a cleaner interface to the overlap checks besides the option to change the overlap checks at startup (which isn't just useful to users, but also a requirement for I/O tests on overlap checks on inactive L1 tables). Max ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-10-09 13:22 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-09-20 8:37 [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 1/6] qcow2: Use negated overflow check mask Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 2/6] qcow2: Make overlap check mask variable Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 3/6] qcow2: Add overlap-check options Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 4/6] qcow2: Array assigning options to OL check bits Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 5/6] qcow2: Add more overlap check bitmask macros Max Reitz 2013-10-09 13:07 ` Kevin Wolf 2013-10-09 13:22 ` Max Reitz 2013-09-20 8:37 ` [Qemu-devel] [PATCH 6/6] qcow2: Evaluate overlap check options Max Reitz 2013-10-09 10:17 ` [Qemu-devel] [PATCH 0/6] Configure metadata overlap checks at runtime Max Reitz
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).