* [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 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.