* [Qemu-devel] [PATCH v9 1/4] quorum: Add the rewrite-corrupted parameter to quorum
2014-06-11 13:24 [Qemu-devel] [PATCH v9 0/4] Quorum maintainance operations Benoît Canet
@ 2014-06-11 13:24 ` Benoît Canet
2014-06-13 23:07 ` Max Reitz
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 2/4] block: Add node-name argument to drive-mirror Benoît Canet
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Benoît Canet @ 2014-06-11 13:24 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Benoît Canet, Benoit Canet, mreitz, stefanha
On read operations when this parameter is set and some replicas are corrupted
while quorum can be reached quorum will proceed to rewrite the correct version
of the data to fix the corrupted replicas.
This will shine with SSD where the FTL will remap the same block at another
place on rewrite.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
block/quorum.c | 97 +++++++++++++++++++++++++++++++++++++++++++---
qapi/block-core.json | 5 ++-
tests/qemu-iotests/081 | 15 ++++++-
tests/qemu-iotests/081.out | 10 +++++
4 files changed, 119 insertions(+), 8 deletions(-)
diff --git a/block/quorum.c b/block/quorum.c
index 426077a..1a7a01e 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -22,6 +22,7 @@
#define QUORUM_OPT_VOTE_THRESHOLD "vote-threshold"
#define QUORUM_OPT_BLKVERIFY "blkverify"
+#define QUORUM_OPT_REWRITE "rewrite-corrupted"
/* This union holds a vote hash value */
typedef union QuorumVoteValue {
@@ -69,6 +70,9 @@ typedef struct BDRVQuorumState {
* It is useful to debug other block drivers by
* comparing them with a reference one.
*/
+ bool rewrite_corrupted;/* true if the driver must rewrite-on-read corrupted
+ * block if Quorum is reached.
+ */
} BDRVQuorumState;
typedef struct QuorumAIOCB QuorumAIOCB;
@@ -104,13 +108,17 @@ struct QuorumAIOCB {
int count; /* number of completed AIOCB */
int success_count; /* number of successfully completed AIOCB */
+ int rewrite_count; /* number of replica to rewrite: count down to
+ * zero once writes are fired
+ */
+
QuorumVotes votes;
bool is_read;
int vote_ret;
};
-static void quorum_vote(QuorumAIOCB *acb);
+static bool quorum_vote(QuorumAIOCB *acb);
static void quorum_aio_cancel(BlockDriverAIOCB *blockacb)
{
@@ -182,6 +190,7 @@ static QuorumAIOCB *quorum_aio_get(BDRVQuorumState *s,
acb->qcrs = g_new0(QuorumChildRequest, s->num_children);
acb->count = 0;
acb->success_count = 0;
+ acb->rewrite_count = 0;
acb->votes.compare = quorum_sha256_compare;
QLIST_INIT(&acb->votes.vote_list);
acb->is_read = false;
@@ -241,11 +250,27 @@ static bool quorum_has_too_much_io_failed(QuorumAIOCB *acb)
return false;
}
+static void quorum_rewrite_aio_cb(void *opaque, int ret)
+{
+ QuorumAIOCB *acb = opaque;
+
+ /* one less rewrite to do */
+ acb->rewrite_count--;
+
+ /* wait until all rewrite callbacks have completed */
+ if (acb->rewrite_count) {
+ return;
+ }
+
+ quorum_aio_finalize(acb);
+}
+
static void quorum_aio_cb(void *opaque, int ret)
{
QuorumChildRequest *sacb = opaque;
QuorumAIOCB *acb = sacb->parent;
BDRVQuorumState *s = acb->common.bs->opaque;
+ bool rewrite = false;
sacb->ret = ret;
acb->count++;
@@ -262,12 +287,15 @@ static void quorum_aio_cb(void *opaque, int ret)
/* Do the vote on read */
if (acb->is_read) {
- quorum_vote(acb);
+ rewrite = quorum_vote(acb);
} else {
quorum_has_too_much_io_failed(acb);
}
- quorum_aio_finalize(acb);
+ /* if no rewrite is done the code will finish right away */
+ if (!rewrite) {
+ quorum_aio_finalize(acb);
+ }
}
static void quorum_report_bad_versions(BDRVQuorumState *s,
@@ -287,6 +315,43 @@ static void quorum_report_bad_versions(BDRVQuorumState *s,
}
}
+static bool quorum_rewrite_bad_versions(BDRVQuorumState *s, QuorumAIOCB *acb,
+ QuorumVoteValue *value)
+{
+ QuorumVoteVersion *version;
+ QuorumVoteItem *item;
+ int count = 0;
+
+ /* first count the number of bad versions: done first to avoid concurrency
+ * issues.
+ */
+ QLIST_FOREACH(version, &acb->votes.vote_list, next) {
+ if (acb->votes.compare(&version->value, value)) {
+ continue;
+ }
+ QLIST_FOREACH(item, &version->items, next) {
+ count++;
+ }
+ }
+
+ /* quorum_rewrite_aio_cb will count down this to zero */
+ acb->rewrite_count = count;
+
+ /* now fire the correcting rewrites */
+ QLIST_FOREACH(version, &acb->votes.vote_list, next) {
+ if (acb->votes.compare(&version->value, value)) {
+ continue;
+ }
+ QLIST_FOREACH(item, &version->items, next) {
+ bdrv_aio_writev(s->bs[item->index], acb->sector_num, acb->qiov,
+ acb->nb_sectors, quorum_rewrite_aio_cb, acb);
+ }
+ }
+
+ /* return true if any rewrite is done else false */
+ return count;
+}
+
static void quorum_copy_qiov(QEMUIOVector *dest, QEMUIOVector *source)
{
int i;
@@ -477,16 +542,17 @@ static int quorum_vote_error(QuorumAIOCB *acb)
return ret;
}
-static void quorum_vote(QuorumAIOCB *acb)
+static bool quorum_vote(QuorumAIOCB *acb)
{
bool quorum = true;
+ bool rewrite = false;
int i, j, ret;
QuorumVoteValue hash;
BDRVQuorumState *s = acb->common.bs->opaque;
QuorumVoteVersion *winner;
if (quorum_has_too_much_io_failed(acb)) {
- return;
+ return false;
}
/* get the index of the first successful read */
@@ -514,7 +580,7 @@ static void quorum_vote(QuorumAIOCB *acb)
/* Every successful read agrees */
if (quorum) {
quorum_copy_qiov(acb->qiov, &acb->qcrs[i].qiov);
- return;
+ return false;
}
/* compute hashes for each successful read, also store indexes */
@@ -547,9 +613,15 @@ static void quorum_vote(QuorumAIOCB *acb)
/* some versions are bad print them */
quorum_report_bad_versions(s, acb, &winner->value);
+ /* corruption correction is enabled */
+ if (s->rewrite_corrupted) {
+ rewrite = quorum_rewrite_bad_versions(s, acb, &winner->value);
+ }
+
free_exit:
/* free lists */
quorum_free_vote_list(&acb->votes);
+ return rewrite;
}
static BlockDriverAIOCB *quorum_aio_readv(BlockDriverState *bs,
@@ -714,6 +786,11 @@ static QemuOptsList quorum_runtime_opts = {
.type = QEMU_OPT_BOOL,
.help = "Trigger block verify mode if set",
},
+ {
+ .name = QUORUM_OPT_REWRITE,
+ .type = QEMU_OPT_BOOL,
+ .help = "Rewrite corrupted block on read quorum",
+ },
{ /* end of list */ }
},
};
@@ -775,6 +852,14 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
"and using two files with vote_threshold=2\n");
}
+ s->rewrite_corrupted = qemu_opt_get_bool(opts, QUORUM_OPT_REWRITE, false);
+ if (s->rewrite_corrupted && s->is_blkverify) {
+ error_setg(&local_err,
+ "rewrite-corrupted=on cannot be used with blkverify=on");
+ ret = -EINVAL;
+ goto exit;
+ }
+
/* allocate the children BlockDriverState array */
s->bs = g_new0(BlockDriverState *, s->num_children);
opened = g_new0(bool, s->num_children);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 33bd93f..eec881b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1333,12 +1333,15 @@
#
# @vote-threshold: the vote limit under which a read will fail
#
+# @rewrite-corrupted: #optional rewrite corrupted data when quorum is reached
+# (Since 2.1)
+#
# Since: 2.0
##
{ 'type': 'BlockdevOptionsQuorum',
'data': { '*blkverify': 'bool',
'children': [ 'BlockdevRef' ],
- 'vote-threshold': 'int' } }
+ 'vote-threshold': 'int', '*rewrite-corrupted': 'bool' } }
##
# @BlockdevOptions
diff --git a/tests/qemu-iotests/081 b/tests/qemu-iotests/081
index b512d00..7ae4be2 100755
--- a/tests/qemu-iotests/081
+++ b/tests/qemu-iotests/081
@@ -134,15 +134,28 @@ run_qemu -drive "file=$TEST_DIR/2.raw,format=$IMGFMT,if=none,id=drive2" <<EOF
EOF
echo
+echo "== using quorum rewrite corrupted mode =="
+
+quorum="$quorum,file.rewrite-corrupted=on"
+
+$QEMU_IO -c "open -o $quorum" -c "read -P 0x32 0 $size" | _filter_qemu_io
+
+echo
+echo "== checking that quorum has corrected the corrupted file =="
+
+$QEMU_IO -c "read -P 0x32 0 $size" "$TEST_DIR/2.raw" | _filter_qemu_io
+
+echo
echo "== breaking quorum =="
$QEMU_IO -c "write -P 0x41 0 $size" "$TEST_DIR/1.raw" | _filter_qemu_io
+$QEMU_IO -c "write -P 0x42 0 $size" "$TEST_DIR/2.raw" | _filter_qemu_io
+
echo
echo "== checking that quorum is broken =="
$QEMU_IO -c "open -o $quorum" -c "read -P 0x32 0 $size" | _filter_qemu_io
-
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/081.out b/tests/qemu-iotests/081.out
index 84aeb0c..1629d2a 100644
--- a/tests/qemu-iotests/081.out
+++ b/tests/qemu-iotests/081.out
@@ -40,9 +40,19 @@ read 10485760/10485760 bytes at offset 0
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+== using quorum rewrite corrupted mode ==
+read 10485760/10485760 bytes at offset 0
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== checking that quorum has corrected the corrupted file ==
+read 10485760/10485760 bytes at offset 0
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
== breaking quorum ==
wrote 10485760/10485760 bytes at offset 0
10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 10485760/10485760 bytes at offset 0
+10 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
== checking that quorum is broken ==
qemu-io: can't open device (null): Could not read image for determining its format: Input/output error
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v9 1/4] quorum: Add the rewrite-corrupted parameter to quorum
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 1/4] quorum: Add the rewrite-corrupted parameter to quorum Benoît Canet
@ 2014-06-13 23:07 ` Max Reitz
0 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-06-13 23:07 UTC (permalink / raw)
To: Benoît Canet, qemu-devel; +Cc: kwolf, Benoit Canet, stefanha
On 11.06.2014 15:24, Benoît Canet wrote:
> On read operations when this parameter is set and some replicas are corrupted
> while quorum can be reached quorum will proceed to rewrite the correct version
> of the data to fix the corrupted replicas.
>
> This will shine with SSD where the FTL will remap the same block at another
> place on rewrite.
>
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
> block/quorum.c | 97 +++++++++++++++++++++++++++++++++++++++++++---
> qapi/block-core.json | 5 ++-
> tests/qemu-iotests/081 | 15 ++++++-
> tests/qemu-iotests/081.out | 10 +++++
> 4 files changed, 119 insertions(+), 8 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v9 2/4] block: Add node-name argument to drive-mirror
2014-06-11 13:24 [Qemu-devel] [PATCH v9 0/4] Quorum maintainance operations Benoît Canet
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 1/4] quorum: Add the rewrite-corrupted parameter to quorum Benoît Canet
@ 2014-06-11 13:24 ` Benoît Canet
2014-06-13 23:15 ` Max Reitz
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 3/4] block: Add replaces " Benoît Canet
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 4/4] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode Benoît Canet
3 siblings, 1 reply; 10+ messages in thread
From: Benoît Canet @ 2014-06-11 13:24 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Benoît Canet, Benoit Canet, mreitz, stefanha
This new argument can be used to specify the node-name of the new mirrored BDS.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
blockdev.c | 11 +++++++++--
hmp.c | 1 +
qapi/block-core.json | 4 ++++
qmp-commands.hx | 3 +++
4 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 4cbcc56..06b14f2 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2106,6 +2106,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
void qmp_drive_mirror(const char *device, const char *target,
bool has_format, const char *format,
+ bool has_node_name, const char *node_name,
enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
@@ -2119,6 +2120,7 @@ void qmp_drive_mirror(const char *device, const char *target,
BlockDriverState *source, *target_bs;
BlockDriver *drv = NULL;
Error *local_err = NULL;
+ QDict *options = NULL;
int flags;
int64_t size;
int ret;
@@ -2220,12 +2222,17 @@ void qmp_drive_mirror(const char *device, const char *target,
return;
}
+ if (has_node_name) {
+ options = qdict_new();
+ qdict_put(options, "node-name", qstring_from_str(node_name));
+ }
+
/* Mirroring takes care of copy-on-write using the source's backing
* file.
*/
target_bs = NULL;
- ret = bdrv_open(&target_bs, target, NULL, NULL, flags | BDRV_O_NO_BACKING,
- drv, &local_err);
+ ret = bdrv_open(&target_bs, target, NULL, options,
+ flags | BDRV_O_NO_BACKING, drv, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
return;
diff --git a/hmp.c b/hmp.c
index ccc35d4..ef0d583 100644
--- a/hmp.c
+++ b/hmp.c
@@ -930,6 +930,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
}
qmp_drive_mirror(device, filename, !!format, format,
+ false, NULL,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
true, mode, false, 0, false, 0, false, 0,
false, 0, false, 0, &err);
diff --git a/qapi/block-core.json b/qapi/block-core.json
index eec881b..4c28d7b 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -769,6 +769,9 @@
# @format: #optional the format of the new destination, default is to
# probe if @mode is 'existing', else the format of the source
#
+# @node-name: #optional the new block driver state node name in the graph
+# (Since 2.1)
+#
# @mode: #optional whether and how QEMU should create a new image, default is
# 'absolute-paths'.
#
@@ -801,6 +804,7 @@
##
{ 'command': 'drive-mirror',
'data': { 'device': 'str', 'target': 'str', '*format': 'str',
+ '*node-name': 'str',
'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
'*speed': 'int', '*granularity': 'uint32',
'*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d8aa4ed..98c28f5 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1293,6 +1293,7 @@ EQMP
{
.name = "drive-mirror",
.args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
+ "node-name:s?,"
"on-source-error:s?,on-target-error:s?,"
"granularity:i?,buf-size:i?",
.mhandler.cmd_new = qmp_marshal_input_drive_mirror,
@@ -1314,6 +1315,8 @@ Arguments:
- "device": device name to operate on (json-string)
- "target": name of new image file (json-string)
- "format": format of new image (json-string, optional)
+- "node-name": the name of the new block driver state in the node graph
+ (json-string, optional)
- "mode": how an image file should be created into the target
file/device (NewImageMode, optional, default 'absolute-paths')
- "speed": maximum speed of the streaming job, in bytes per second
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH v9 3/4] block: Add replaces argument to drive-mirror
2014-06-11 13:24 [Qemu-devel] [PATCH v9 0/4] Quorum maintainance operations Benoît Canet
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 1/4] quorum: Add the rewrite-corrupted parameter to quorum Benoît Canet
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 2/4] block: Add node-name argument to drive-mirror Benoît Canet
@ 2014-06-11 13:24 ` Benoît Canet
2014-06-13 23:59 ` Max Reitz
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 4/4] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode Benoît Canet
3 siblings, 1 reply; 10+ messages in thread
From: Benoît Canet @ 2014-06-11 13:24 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Benoît Canet, Benoit Canet, mreitz, stefanha
drive-mirror will bdrv_swap the new BDS named node-name with the one
pointed by replaces when the mirroring is finished.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
block.c | 17 ++++++++++++++
block/mirror.c | 60 +++++++++++++++++++++++++++++++++++++----------
blockdev.c | 30 +++++++++++++++++++++++-
hmp.c | 2 +-
include/block/block.h | 4 ++++
include/block/block_int.h | 4 +++-
qapi/block-core.json | 6 ++++-
qmp-commands.hx | 4 +++-
8 files changed, 109 insertions(+), 18 deletions(-)
diff --git a/block.c b/block.c
index 17f763d..318f1e6 100644
--- a/block.c
+++ b/block.c
@@ -5795,3 +5795,20 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate)
return false;
}
+
+BlockDriverState *check_to_replace_node(const char *node_name, Error **errp)
+{
+ BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
+ if (!to_replace_bs) {
+ error_setg(errp, "Node name '%s' not found",
+ node_name);
+ return NULL;
+ }
+
+ if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) {
+ return NULL;
+ }
+
+ return to_replace_bs;
+}
+
diff --git a/block/mirror.c b/block/mirror.c
index 94c8661..b00365f 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -32,6 +32,12 @@ typedef struct MirrorBlockJob {
RateLimit limit;
BlockDriverState *target;
BlockDriverState *base;
+ /* The name of the graph node to replace */
+ char *replaces;
+ /* The block BDS to replace */
+ BlockDriverState *to_replace;
+ /* Used to block operations on the drive-mirror-replace target */
+ Error *replace_blocker;
bool is_none_mode;
BlockdevOnError on_source_error, on_target_error;
bool synced;
@@ -490,10 +496,14 @@ immediate_exit:
bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
bdrv_iostatus_disable(s->target);
if (s->should_complete && ret == 0) {
- if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) {
- bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
+ BlockDriverState *to_replace = s->common.bs;
+ if (s->to_replace) {
+ to_replace = s->to_replace;
}
- bdrv_swap(s->target, s->common.bs);
+ if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
+ bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
+ }
+ bdrv_swap(s->target, to_replace);
if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
/* drop the bs loop chain formed by the swap: break the loop then
* trigger the unref from the top one */
@@ -502,6 +512,12 @@ immediate_exit:
bdrv_unref(p);
}
}
+ if (s->to_replace) {
+ bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
+ error_free(s->replace_blocker);
+ bdrv_unref(s->to_replace);
+ g_free(s->replaces);
+ }
bdrv_unref(s->target);
block_job_completed(&s->common, ret);
}
@@ -540,6 +556,20 @@ static void mirror_complete(BlockJob *job, Error **errp)
return;
}
+ /* check the target bs is not block and block all operations on it */
+ if (s->replaces) {
+ s->to_replace = check_to_replace_node(s->replaces, errp);
+
+ if (!s->to_replace) {
+ return;
+ }
+
+ error_setg(&s->replace_blocker,
+ "block device is in use by block-job-complete");
+ bdrv_op_block_all(s->to_replace, s->replace_blocker);
+ bdrv_ref(s->to_replace);
+ }
+
s->should_complete = true;
block_job_resume(job);
}
@@ -562,14 +592,15 @@ static const BlockJobDriver commit_active_job_driver = {
};
static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
- int64_t speed, int64_t granularity,
- int64_t buf_size,
- BlockdevOnError on_source_error,
- BlockdevOnError on_target_error,
- BlockDriverCompletionFunc *cb,
- void *opaque, Error **errp,
- const BlockJobDriver *driver,
- bool is_none_mode, BlockDriverState *base)
+ const char *replaces,
+ int64_t speed, int64_t granularity,
+ int64_t buf_size,
+ BlockdevOnError on_source_error,
+ BlockdevOnError on_target_error,
+ BlockDriverCompletionFunc *cb,
+ void *opaque, Error **errp,
+ const BlockJobDriver *driver,
+ bool is_none_mode, BlockDriverState *base)
{
MirrorBlockJob *s;
@@ -600,6 +631,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
return;
}
+ s->replaces = g_strdup(replaces);
s->on_source_error = on_source_error;
s->on_target_error = on_target_error;
s->target = target;
@@ -621,6 +653,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
}
void mirror_start(BlockDriverState *bs, BlockDriverState *target,
+ const char *replaces,
int64_t speed, int64_t granularity, int64_t buf_size,
MirrorSyncMode mode, BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
@@ -632,7 +665,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
- mirror_start_job(bs, target, speed, granularity, buf_size,
+ mirror_start_job(bs, target, replaces,
+ speed, granularity, buf_size,
on_source_error, on_target_error, cb, opaque, errp,
&mirror_job_driver, is_none_mode, base);
}
@@ -680,7 +714,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
}
bdrv_ref(base);
- mirror_start_job(bs, base, speed, 0, 0,
+ mirror_start_job(bs, base, NULL, speed, 0, 0,
on_error, on_error, cb, opaque, &local_err,
&commit_active_job_driver, false, base);
if (local_err) {
diff --git a/blockdev.c b/blockdev.c
index 06b14f2..237a548 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2107,6 +2107,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
void qmp_drive_mirror(const char *device, const char *target,
bool has_format, const char *format,
bool has_node_name, const char *node_name,
+ bool has_replaces, const char *replaces,
enum MirrorSyncMode sync,
bool has_mode, enum NewImageMode mode,
bool has_speed, int64_t speed,
@@ -2194,6 +2195,28 @@ void qmp_drive_mirror(const char *device, const char *target,
return;
}
+ if (has_replaces) {
+ BlockDriverState *to_replace_bs;
+
+ if (!has_node_name) {
+ error_setg(errp, "a node-name must be provided when replacing a"
+ " named node of the graph");
+ return;
+ }
+
+ to_replace_bs = check_to_replace_node(replaces, errp);
+
+ if (!to_replace_bs) {
+ return;
+ }
+
+ if (size != bdrv_getlength(to_replace_bs)) {
+ error_setg(errp, "cannot replace image with a mirror image of "
+ "different size");
+ return;
+ }
+ }
+
if ((sync == MIRROR_SYNC_MODE_FULL || !source)
&& mode != NEW_IMAGE_MODE_EXISTING)
{
@@ -2238,7 +2261,12 @@ void qmp_drive_mirror(const char *device, const char *target,
return;
}
- mirror_start(bs, target_bs, speed, granularity, buf_size, sync,
+ /* pass the node name to replace to mirror start since it's loose coupling
+ * and will allow to check whether the node still exist at mirror completion
+ */
+ mirror_start(bs, target_bs,
+ has_replaces ? replaces : NULL,
+ speed, granularity, buf_size, sync,
on_source_error, on_target_error,
block_job_cb, bs, &local_err);
if (local_err != NULL) {
diff --git a/hmp.c b/hmp.c
index ef0d583..9a4b8da 100644
--- a/hmp.c
+++ b/hmp.c
@@ -930,7 +930,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
}
qmp_drive_mirror(device, filename, !!format, format,
- false, NULL,
+ false, NULL, false, NULL,
full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
true, mode, false, 0, false, 0, false, 0,
false, 0, false, 0, &err);
diff --git a/include/block/block.h b/include/block/block.h
index 7d86e29..b0ed701 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -179,6 +179,7 @@ typedef enum BlockOpType {
BLOCK_OP_TYPE_MIRROR,
BLOCK_OP_TYPE_RESIZE,
BLOCK_OP_TYPE_STREAM,
+ BLOCK_OP_TYPE_REPLACE,
BLOCK_OP_TYPE_MAX,
} BlockOpType;
@@ -319,6 +320,9 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
BlockDriverState *candidate);
bool bdrv_is_first_non_filter(BlockDriverState *candidate);
+/* check if a named node can be replaced when doing drive-mirror */
+BlockDriverState *check_to_replace_node(const char *node_name, Error **errp);
+
/* async block I/O */
typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
int sector_num);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 8d58334..fdf3ed2 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -86,7 +86,6 @@ struct BlockDriver {
*/
bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs,
BlockDriverState *candidate);
-
int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
int (*bdrv_probe_device)(const char *filename);
@@ -492,6 +491,8 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
* mirror_start:
* @bs: Block device to operate on.
* @target: Block device to write to.
+ * @replaces: Block graph node name to replace once the mirror is done. Can
+ * only be used when full mirroring is selected.
* @speed: The maximum speed, in bytes per second, or 0 for unlimited.
* @granularity: The chosen granularity for the dirty bitmap.
* @buf_size: The amount of data that can be in flight at one time.
@@ -508,6 +509,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
* @bs will be switched to read from @target.
*/
void mirror_start(BlockDriverState *bs, BlockDriverState *target,
+ const char *replaces,
int64_t speed, int64_t granularity, int64_t buf_size,
MirrorSyncMode mode, BlockdevOnError on_source_error,
BlockdevOnError on_target_error,
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4c28d7b..0f8e703 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -772,6 +772,10 @@
# @node-name: #optional the new block driver state node name in the graph
# (Since 2.1)
#
+# @replaces: #optional with sync=full graph node name to be replaced by the new
+# image when a whole image copy is done. This can be used to repair
+# broken Quorum files. (Since 2.1)
+#
# @mode: #optional whether and how QEMU should create a new image, default is
# 'absolute-paths'.
#
@@ -804,7 +808,7 @@
##
{ 'command': 'drive-mirror',
'data': { 'device': 'str', 'target': 'str', '*format': 'str',
- '*node-name': 'str',
+ '*node-name': 'str', '*replaces': 'str',
'sync': 'MirrorSyncMode', '*mode': 'NewImageMode',
'*speed': 'int', '*granularity': 'uint32',
'*buf-size': 'int', '*on-source-error': 'BlockdevOnError',
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 98c28f5..4a76054 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1293,7 +1293,7 @@ EQMP
{
.name = "drive-mirror",
.args_type = "sync:s,device:B,target:s,speed:i?,mode:s?,format:s?,"
- "node-name:s?,"
+ "node-name:s?,replaces:s?,"
"on-source-error:s?,on-target-error:s?,"
"granularity:i?,buf-size:i?",
.mhandler.cmd_new = qmp_marshal_input_drive_mirror,
@@ -1317,6 +1317,8 @@ Arguments:
- "format": format of new image (json-string, optional)
- "node-name": the name of the new block driver state in the node graph
(json-string, optional)
+- "replaces": the block driver node name to replace when finished
+ (json-string, optional)
- "mode": how an image file should be created into the target
file/device (NewImageMode, optional, default 'absolute-paths')
- "speed": maximum speed of the streaming job, in bytes per second
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v9 3/4] block: Add replaces argument to drive-mirror
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 3/4] block: Add replaces " Benoît Canet
@ 2014-06-13 23:59 ` Max Reitz
0 siblings, 0 replies; 10+ messages in thread
From: Max Reitz @ 2014-06-13 23:59 UTC (permalink / raw)
To: Benoît Canet, qemu-devel; +Cc: kwolf, Benoit Canet, stefanha
On 11.06.2014 15:24, Benoît Canet wrote:
> drive-mirror will bdrv_swap the new BDS named node-name with the one
> pointed by replaces when the mirroring is finished.
>
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
> block.c | 17 ++++++++++++++
> block/mirror.c | 60 +++++++++++++++++++++++++++++++++++++----------
> blockdev.c | 30 +++++++++++++++++++++++-
> hmp.c | 2 +-
> include/block/block.h | 4 ++++
> include/block/block_int.h | 4 +++-
> qapi/block-core.json | 6 ++++-
> qmp-commands.hx | 4 +++-
> 8 files changed, 109 insertions(+), 18 deletions(-)
>
> diff --git a/block.c b/block.c
> index 17f763d..318f1e6 100644
> --- a/block.c
> +++ b/block.c
> @@ -5795,3 +5795,20 @@ bool bdrv_is_first_non_filter(BlockDriverState *candidate)
>
> return false;
> }
> +
> +BlockDriverState *check_to_replace_node(const char *node_name, Error **errp)
> +{
> + BlockDriverState *to_replace_bs = bdrv_find_node(node_name);
> + if (!to_replace_bs) {
> + error_setg(errp, "Node name '%s' not found",
> + node_name);
> + return NULL;
> + }
> +
> + if (bdrv_op_is_blocked(to_replace_bs, BLOCK_OP_TYPE_REPLACE, errp)) {
> + return NULL;
> + }
> +
> + return to_replace_bs;
> +}
> +
> diff --git a/block/mirror.c b/block/mirror.c
> index 94c8661..b00365f 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -32,6 +32,12 @@ typedef struct MirrorBlockJob {
> RateLimit limit;
> BlockDriverState *target;
> BlockDriverState *base;
> + /* The name of the graph node to replace */
> + char *replaces;
> + /* The block BDS to replace */
Maybe s/block //
> + BlockDriverState *to_replace;
> + /* Used to block operations on the drive-mirror-replace target */
> + Error *replace_blocker;
> bool is_none_mode;
> BlockdevOnError on_source_error, on_target_error;
> bool synced;
> @@ -490,10 +496,14 @@ immediate_exit:
> bdrv_release_dirty_bitmap(bs, s->dirty_bitmap);
> bdrv_iostatus_disable(s->target);
> if (s->should_complete && ret == 0) {
> - if (bdrv_get_flags(s->target) != bdrv_get_flags(s->common.bs)) {
> - bdrv_reopen(s->target, bdrv_get_flags(s->common.bs), NULL);
> + BlockDriverState *to_replace = s->common.bs;
> + if (s->to_replace) {
> + to_replace = s->to_replace;
> }
> - bdrv_swap(s->target, s->common.bs);
> + if (bdrv_get_flags(s->target) != bdrv_get_flags(to_replace)) {
> + bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL);
> + }
> + bdrv_swap(s->target, to_replace);
> if (s->common.driver->job_type == BLOCK_JOB_TYPE_COMMIT) {
> /* drop the bs loop chain formed by the swap: break the loop then
> * trigger the unref from the top one */
> @@ -502,6 +512,12 @@ immediate_exit:
> bdrv_unref(p);
> }
> }
> + if (s->to_replace) {
> + bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
> + error_free(s->replace_blocker);
> + bdrv_unref(s->to_replace);
> + g_free(s->replaces);
If we get here without mirror_complete() having been invoked (which is
possible, as far as I know), s->to_replace will still be NULL while
s->replaces may be not. In that case, s->replaces will be leaked.
> + }
> bdrv_unref(s->target);
> block_job_completed(&s->common, ret);
> }
> @@ -540,6 +556,20 @@ static void mirror_complete(BlockJob *job, Error **errp)
> return;
> }
>
> + /* check the target bs is not block and block all operations on it */
s/block/blocked/
> + if (s->replaces) {
> + s->to_replace = check_to_replace_node(s->replaces, errp);
> +
> + if (!s->to_replace) {
> + return;
> + }
> +
> + error_setg(&s->replace_blocker,
> + "block device is in use by block-job-complete");
> + bdrv_op_block_all(s->to_replace, s->replace_blocker);
> + bdrv_ref(s->to_replace);
> + }
> +
> s->should_complete = true;
> block_job_resume(job);
> }
> @@ -562,14 +592,15 @@ static const BlockJobDriver commit_active_job_driver = {
> };
>
> static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
> - int64_t speed, int64_t granularity,
> - int64_t buf_size,
> - BlockdevOnError on_source_error,
> - BlockdevOnError on_target_error,
> - BlockDriverCompletionFunc *cb,
> - void *opaque, Error **errp,
> - const BlockJobDriver *driver,
> - bool is_none_mode, BlockDriverState *base)
> + const char *replaces,
> + int64_t speed, int64_t granularity,
> + int64_t buf_size,
> + BlockdevOnError on_source_error,
> + BlockdevOnError on_target_error,
> + BlockDriverCompletionFunc *cb,
> + void *opaque, Error **errp,
> + const BlockJobDriver *driver,
> + bool is_none_mode, BlockDriverState *base)
> {
> MirrorBlockJob *s;
>
> @@ -600,6 +631,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
> return;
> }
>
> + s->replaces = g_strdup(replaces);
> s->on_source_error = on_source_error;
> s->on_target_error = on_target_error;
> s->target = target;
> @@ -621,6 +653,7 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
> }
>
> void mirror_start(BlockDriverState *bs, BlockDriverState *target,
> + const char *replaces,
> int64_t speed, int64_t granularity, int64_t buf_size,
> MirrorSyncMode mode, BlockdevOnError on_source_error,
> BlockdevOnError on_target_error,
> @@ -632,7 +665,8 @@ void mirror_start(BlockDriverState *bs, BlockDriverState *target,
>
> is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
> base = mode == MIRROR_SYNC_MODE_TOP ? bs->backing_hd : NULL;
> - mirror_start_job(bs, target, speed, granularity, buf_size,
> + mirror_start_job(bs, target, replaces,
> + speed, granularity, buf_size,
> on_source_error, on_target_error, cb, opaque, errp,
> &mirror_job_driver, is_none_mode, base);
> }
> @@ -680,7 +714,7 @@ void commit_active_start(BlockDriverState *bs, BlockDriverState *base,
> }
>
> bdrv_ref(base);
> - mirror_start_job(bs, base, speed, 0, 0,
> + mirror_start_job(bs, base, NULL, speed, 0, 0,
> on_error, on_error, cb, opaque, &local_err,
> &commit_active_job_driver, false, base);
> if (local_err) {
> diff --git a/blockdev.c b/blockdev.c
> index 06b14f2..237a548 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -2107,6 +2107,7 @@ BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
> void qmp_drive_mirror(const char *device, const char *target,
> bool has_format, const char *format,
> bool has_node_name, const char *node_name,
> + bool has_replaces, const char *replaces,
> enum MirrorSyncMode sync,
> bool has_mode, enum NewImageMode mode,
> bool has_speed, int64_t speed,
> @@ -2194,6 +2195,28 @@ void qmp_drive_mirror(const char *device, const char *target,
> return;
> }
>
> + if (has_replaces) {
> + BlockDriverState *to_replace_bs;
> +
> + if (!has_node_name) {
> + error_setg(errp, "a node-name must be provided when replacing a"
> + " named node of the graph");
> + return;
> + }
> +
> + to_replace_bs = check_to_replace_node(replaces, errp);
> +
> + if (!to_replace_bs) {
> + return;
> + }
> +
> + if (size != bdrv_getlength(to_replace_bs)) {
> + error_setg(errp, "cannot replace image with a mirror image of "
> + "different size");
> + return;
> + }
> + }
> +
> if ((sync == MIRROR_SYNC_MODE_FULL || !source)
> && mode != NEW_IMAGE_MODE_EXISTING)
> {
> @@ -2238,7 +2261,12 @@ void qmp_drive_mirror(const char *device, const char *target,
> return;
> }
>
> - mirror_start(bs, target_bs, speed, granularity, buf_size, sync,
> + /* pass the node name to replace to mirror start since it's loose coupling
> + * and will allow to check whether the node still exist at mirror completion
> + */
> + mirror_start(bs, target_bs,
> + has_replaces ? replaces : NULL,
> + speed, granularity, buf_size, sync,
> on_source_error, on_target_error,
> block_job_cb, bs, &local_err);
> if (local_err != NULL) {
> diff --git a/hmp.c b/hmp.c
> index ef0d583..9a4b8da 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -930,7 +930,7 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
> }
>
> qmp_drive_mirror(device, filename, !!format, format,
> - false, NULL,
> + false, NULL, false, NULL,
> full ? MIRROR_SYNC_MODE_FULL : MIRROR_SYNC_MODE_TOP,
> true, mode, false, 0, false, 0, false, 0,
> false, 0, false, 0, &err);
> diff --git a/include/block/block.h b/include/block/block.h
> index 7d86e29..b0ed701 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -179,6 +179,7 @@ typedef enum BlockOpType {
> BLOCK_OP_TYPE_MIRROR,
> BLOCK_OP_TYPE_RESIZE,
> BLOCK_OP_TYPE_STREAM,
> + BLOCK_OP_TYPE_REPLACE,
> BLOCK_OP_TYPE_MAX,
> } BlockOpType;
>
> @@ -319,6 +320,9 @@ bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
> BlockDriverState *candidate);
> bool bdrv_is_first_non_filter(BlockDriverState *candidate);
>
> +/* check if a named node can be replaced when doing drive-mirror */
> +BlockDriverState *check_to_replace_node(const char *node_name, Error **errp);
> +
> /* async block I/O */
> typedef void BlockDriverDirtyHandler(BlockDriverState *bs, int64_t sector,
> int sector_num);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 8d58334..fdf3ed2 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -86,7 +86,6 @@ struct BlockDriver {
> */
> bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs,
> BlockDriverState *candidate);
> -
> int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
> int (*bdrv_probe_device)(const char *filename);
>
This hunk doesn't seem intended...?
Other than these, the patch looks fine to me. If you fix the leakage of
s->replaces (or explain why mirror_complete() is always invoked before
immediate_exit):
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH v9 4/4] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode.
2014-06-11 13:24 [Qemu-devel] [PATCH v9 0/4] Quorum maintainance operations Benoît Canet
` (2 preceding siblings ...)
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 3/4] block: Add replaces " Benoît Canet
@ 2014-06-11 13:24 ` Benoît Canet
2014-06-14 0:44 ` Max Reitz
3 siblings, 1 reply; 10+ messages in thread
From: Benoît Canet @ 2014-06-11 13:24 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Benoît Canet, Benoit Canet, mreitz, stefanha
The to-replace-node-name is designed to allow repairing of broken Quorum file.
This patch introduce a new class TestRepairQuorum testing that the feature
works.
Some further work will be done on QEMU to improve the robutness of the tests.
Signed-off-by: Benoit Canet <benoit@irqsave.net>
---
tests/qemu-iotests/041 | 196 ++++++++++++++++++++++++++++++++++++++++++++-
tests/qemu-iotests/041.out | 4 +-
2 files changed, 194 insertions(+), 6 deletions(-)
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index ec470b2..c0480e1 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -28,6 +28,12 @@ target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
test_img = os.path.join(iotests.test_dir, 'test.img')
target_img = os.path.join(iotests.test_dir, 'target.img')
+quorum_img1 = os.path.join(iotests.test_dir, 'quorum1.img')
+quorum_img2 = os.path.join(iotests.test_dir, 'quorum2.img')
+quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img')
+quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img')
+quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img')
+
class ImageMirroringTestCase(iotests.QMPTestCase):
'''Abstract base class for image mirroring test cases'''
@@ -42,8 +48,8 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
ready = True
def wait_ready_and_cancel(self, drive='drive0'):
- self.wait_ready(drive)
- event = self.cancel_and_wait()
+ self.wait_ready(drive=drive)
+ event = self.cancel_and_wait(drive=drive)
self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
self.assert_qmp(event, 'data/type', 'mirror')
self.assert_qmp(event, 'data/offset', self.image_len)
@@ -52,12 +58,12 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
def complete_and_wait(self, drive='drive0', wait_ready=True):
'''Complete a block job and wait for it to finish'''
if wait_ready:
- self.wait_ready()
+ self.wait_ready(drive=drive)
result = self.vm.qmp('block-job-complete', device=drive)
self.assert_qmp(result, 'return', {})
- event = self.wait_until_completed()
+ event = self.wait_until_completed(drive=drive)
self.assert_qmp(event, 'data/type', 'mirror')
class TestSingleDrive(ImageMirroringTestCase):
@@ -718,5 +724,187 @@ class TestUnbackedSource(ImageMirroringTestCase):
self.complete_and_wait()
self.assert_no_active_block_jobs()
+class TestRepairQuorum(ImageMirroringTestCase):
+ """ This class test quorum file repair using drive-mirror.
+ It's mostly a fork of TestSingleDrive """
+ image_len = 1 * 1024 * 1024 # MB
+ IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
+
+ def setUp(self):
+ self.vm = iotests.VM()
+
+ # Add each individual quorum images
+ for i in self.IMAGES:
+ qemu_img('create', '-f', iotests.imgfmt, i,
+ str(TestSingleDrive.image_len))
+ # Assign a node name to each quorum image in order to manipulate
+ # them
+ opts = "node-name=img%i" % self.IMAGES.index(i)
+ self.vm = self.vm.add_drive(i, opts)
+
+ self.vm.launch()
+
+ #assemble the quorum block device from the individual files
+ args = { "options" : { "driver": "quorum", "id": "quorum0",
+ "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } }
+ result = self.vm.qmp("blockdev-add", **args)
+ self.assert_qmp(result, 'return', {})
+
+
+ def tearDown(self):
+ self.vm.shutdown()
+ for i in self.IMAGES + [ quorum_repair_img ]:
+ # Do a try/except because the test may have deleted some images
+ try:
+ os.remove(i)
+ except OSError:
+ pass
+
+ def test_complete(self):
+ self.assert_no_active_block_jobs()
+
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name="repair0",
+ replaces="img1",
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'return', {})
+
+ self.complete_and_wait(drive="quorum0")
+ result = self.vm.qmp('query-named-block-nodes')
+ self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
+ # TODO: a better test requiring some QEMU infrastructure will be added
+ # to check that this file is really driven by quorum
+ self.vm.shutdown()
+ self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
+ 'target image does not match source after mirroring')
+
+ def test_cancel(self):
+ self.assert_no_active_block_jobs()
+
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name="repair0",
+ replaces="img1",
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'return', {})
+
+ self.cancel_and_wait(drive="quorum0", force=True)
+ # here we check that the last registered quorum file has not been
+ # swapped out and unref
+ result = self.vm.qmp('query-named-block-nodes')
+ self.assert_qmp(result, 'return[0]/file', quorum_img3)
+ self.vm.shutdown()
+
+ def test_cancel_after_ready(self):
+ self.assert_no_active_block_jobs()
+
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name="repair0",
+ replaces="img1",
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'return', {})
+
+ self.wait_ready_and_cancel(drive="quorum0")
+ result = self.vm.qmp('query-named-block-nodes')
+ # here we check that the last registered quorum file has not been
+ # swapped out and unref
+ self.assert_qmp(result, 'return[0]/file', quorum_img3)
+ self.vm.shutdown()
+ self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
+ 'target image does not match source after mirroring')
+
+ def test_pause(self):
+ self.assert_no_active_block_jobs()
+
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name="repair0",
+ replaces="img1",
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'return', {})
+
+ result = self.vm.qmp('block-job-pause', device='quorum0')
+ self.assert_qmp(result, 'return', {})
+
+ time.sleep(1)
+ result = self.vm.qmp('query-block-jobs')
+ offset = self.dictpath(result, 'return[0]/offset')
+
+ time.sleep(1)
+ result = self.vm.qmp('query-block-jobs')
+ self.assert_qmp(result, 'return[0]/offset', offset)
+
+ result = self.vm.qmp('block-job-resume', device='quorum0')
+ self.assert_qmp(result, 'return', {})
+
+ self.complete_and_wait(drive="quorum0")
+ self.vm.shutdown()
+ self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
+ 'target image does not match source after mirroring')
+
+ def test_medium_not_found(self):
+ result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
+ node_name='repair0',
+ replaces='img1',
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_image_not_found(self):
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name='repair0',
+ replaces='img1',
+ mode='existing',
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_device_not_found(self):
+ result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
+ node_name='repair0',
+ replaces='img1',
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'error/class', 'DeviceNotFound')
+
+ def test_wrong_sync_mode(self):
+ result = self.vm.qmp('drive-mirror', device='quorum0',
+ node_name='repair0',
+ replaces='img1',
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_no_node_name(self):
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ replaces='img1',
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_unexistant_replaces(self):
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name='repair0',
+ replaces='img77',
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ def test_after_a_quorum_snapshot(self):
+ result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
+ snapshot_file=quorum_snapshot_file,
+ snapshot_node_name="snap1");
+
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name='repair0',
+ replaces="img1",
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'error/class', 'GenericError')
+
+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
+ node_name='repair0',
+ replaces="snap1",
+ target=quorum_repair_img, format=iotests.imgfmt)
+ self.assert_qmp(result, 'return', {})
+
+ self.complete_and_wait(drive="quorum0")
+ result = self.vm.qmp('query-named-block-nodes')
+ self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
+ # TODO: a better test requiring some QEMU infrastructure will be added
+ # to check that this file is really driven by quorum
+ self.vm.shutdown()
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2', 'qed'])
diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
index 6d9bee1..73e375a 100644
--- a/tests/qemu-iotests/041.out
+++ b/tests/qemu-iotests/041.out
@@ -1,5 +1,5 @@
-...........................
+......................................
----------------------------------------------------------------------
-Ran 27 tests
+Ran 38 tests
OK
--
1.9.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v9 4/4] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode.
2014-06-11 13:24 ` [Qemu-devel] [PATCH v9 4/4] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode Benoît Canet
@ 2014-06-14 0:44 ` Max Reitz
2014-06-16 9:58 ` Benoît Canet
0 siblings, 1 reply; 10+ messages in thread
From: Max Reitz @ 2014-06-14 0:44 UTC (permalink / raw)
To: Benoît Canet, qemu-devel; +Cc: kwolf, Benoit Canet, stefanha
On 11.06.2014 15:24, Benoît Canet wrote:
> The to-replace-node-name is designed to allow repairing of broken Quorum file.
"a broken Quorum file" or "broken Quorum files".
> This patch introduce a new class TestRepairQuorum testing that the feature
*introduces
> works.
> Some further work will be done on QEMU to improve the robutness of the tests.
*robustness
> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> ---
> tests/qemu-iotests/041 | 196 ++++++++++++++++++++++++++++++++++++++++++++-
> tests/qemu-iotests/041.out | 4 +-
> 2 files changed, 194 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index ec470b2..c0480e1 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -28,6 +28,12 @@ target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
> test_img = os.path.join(iotests.test_dir, 'test.img')
> target_img = os.path.join(iotests.test_dir, 'target.img')
>
> +quorum_img1 = os.path.join(iotests.test_dir, 'quorum1.img')
> +quorum_img2 = os.path.join(iotests.test_dir, 'quorum2.img')
> +quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img')
> +quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img')
> +quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img')
> +
> class ImageMirroringTestCase(iotests.QMPTestCase):
> '''Abstract base class for image mirroring test cases'''
>
> @@ -42,8 +48,8 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
> ready = True
>
> def wait_ready_and_cancel(self, drive='drive0'):
> - self.wait_ready(drive)
> - event = self.cancel_and_wait()
> + self.wait_ready(drive=drive)
> + event = self.cancel_and_wait(drive=drive)
My Python is very bad (if existent at all), but is this "drive=drive"
really necessary? Wouldn't simply "drive" suffice?
> self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
> self.assert_qmp(event, 'data/type', 'mirror')
> self.assert_qmp(event, 'data/offset', self.image_len)
> @@ -52,12 +58,12 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
> def complete_and_wait(self, drive='drive0', wait_ready=True):
> '''Complete a block job and wait for it to finish'''
> if wait_ready:
> - self.wait_ready()
> + self.wait_ready(drive=drive)
>
> result = self.vm.qmp('block-job-complete', device=drive)
> self.assert_qmp(result, 'return', {})
>
> - event = self.wait_until_completed()
> + event = self.wait_until_completed(drive=drive)
> self.assert_qmp(event, 'data/type', 'mirror')
>
> class TestSingleDrive(ImageMirroringTestCase):
> @@ -718,5 +724,187 @@ class TestUnbackedSource(ImageMirroringTestCase):
> self.complete_and_wait()
> self.assert_no_active_block_jobs()
>
> +class TestRepairQuorum(ImageMirroringTestCase):
> + """ This class test quorum file repair using drive-mirror.
> + It's mostly a fork of TestSingleDrive """
> + image_len = 1 * 1024 * 1024 # MB
> + IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
> +
> + def setUp(self):
> + self.vm = iotests.VM()
> +
> + # Add each individual quorum images
> + for i in self.IMAGES:
> + qemu_img('create', '-f', iotests.imgfmt, i,
> + str(TestSingleDrive.image_len))
> + # Assign a node name to each quorum image in order to manipulate
> + # them
> + opts = "node-name=img%i" % self.IMAGES.index(i)
> + self.vm = self.vm.add_drive(i, opts)
Just an idea: As you are constantly swapping out img1, why don't you
write some data to img2 and img3 and different data to img1 to test it
all how it's supposed to work?
> +
> + self.vm.launch()
> +
> + #assemble the quorum block device from the individual files
> + args = { "options" : { "driver": "quorum", "id": "quorum0",
> + "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } }
> + result = self.vm.qmp("blockdev-add", **args)
> + self.assert_qmp(result, 'return', {})
> +
> +
> + def tearDown(self):
> + self.vm.shutdown()
> + for i in self.IMAGES + [ quorum_repair_img ]:
> + # Do a try/except because the test may have deleted some images
> + try:
> + os.remove(i)
> + except OSError:
> + pass
> +
> + def test_complete(self):
> + self.assert_no_active_block_jobs()
> +
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name="repair0",
> + replaces="img1",
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'return', {})
> +
> + self.complete_and_wait(drive="quorum0")
> + result = self.vm.qmp('query-named-block-nodes')
> + self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
Hm, why do you know this is at index 0?
> + # TODO: a better test requiring some QEMU infrastructure will be added
> + # to check that this file is really driven by quorum
> + self.vm.shutdown()
> + self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
> + 'target image does not match source after mirroring')
> +
> + def test_cancel(self):
> + self.assert_no_active_block_jobs()
> +
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name="repair0",
> + replaces="img1",
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'return', {})
> +
> + self.cancel_and_wait(drive="quorum0", force=True)
> + # here we check that the last registered quorum file has not been
> + # swapped out and unref
> + result = self.vm.qmp('query-named-block-nodes')
> + self.assert_qmp(result, 'return[0]/file', quorum_img3)
Why would img3 be affected at all? Aren't you trying to replace img1?
(Also, again, why index 0?)
> + self.vm.shutdown()
> +
> + def test_cancel_after_ready(self):
> + self.assert_no_active_block_jobs()
> +
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name="repair0",
> + replaces="img1",
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'return', {})
> +
> + self.wait_ready_and_cancel(drive="quorum0")
> + result = self.vm.qmp('query-named-block-nodes')
> + # here we check that the last registered quorum file has not been
> + # swapped out and unref
> + self.assert_qmp(result, 'return[0]/file', quorum_img3)
Same here.
> + self.vm.shutdown()
> + self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
> + 'target image does not match source after mirroring')
> +
> + def test_pause(self):
> + self.assert_no_active_block_jobs()
> +
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name="repair0",
> + replaces="img1",
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'return', {})
> +
> + result = self.vm.qmp('block-job-pause', device='quorum0')
> + self.assert_qmp(result, 'return', {})
> +
> + time.sleep(1)
> + result = self.vm.qmp('query-block-jobs')
> + offset = self.dictpath(result, 'return[0]/offset')
> +
> + time.sleep(1)
> + result = self.vm.qmp('query-block-jobs')
> + self.assert_qmp(result, 'return[0]/offset', offset)
> +
> + result = self.vm.qmp('block-job-resume', device='quorum0')
> + self.assert_qmp(result, 'return', {})
> +
> + self.complete_and_wait(drive="quorum0")
> + self.vm.shutdown()
> + self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
> + 'target image does not match source after mirroring')
> +
> + def test_medium_not_found(self):
> + result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
I don't know whether this device is supposed to exist forever, but if it
disappears from default configuration one day, TestSingleDrive will
break as well, so it's probably fine.
Max
> + node_name='repair0',
> + replaces='img1',
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'error/class', 'GenericError')
> +
> + def test_image_not_found(self):
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name='repair0',
> + replaces='img1',
> + mode='existing',
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'error/class', 'GenericError')
> +
> + def test_device_not_found(self):
> + result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
> + node_name='repair0',
> + replaces='img1',
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'error/class', 'DeviceNotFound')
> +
> + def test_wrong_sync_mode(self):
> + result = self.vm.qmp('drive-mirror', device='quorum0',
> + node_name='repair0',
> + replaces='img1',
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'error/class', 'GenericError')
> +
> + def test_no_node_name(self):
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + replaces='img1',
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'error/class', 'GenericError')
> +
> + def test_unexistant_replaces(self):
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name='repair0',
> + replaces='img77',
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'error/class', 'GenericError')
> +
> + def test_after_a_quorum_snapshot(self):
> + result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
> + snapshot_file=quorum_snapshot_file,
> + snapshot_node_name="snap1");
> +
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name='repair0',
> + replaces="img1",
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'error/class', 'GenericError')
> +
> + result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> + node_name='repair0',
> + replaces="snap1",
> + target=quorum_repair_img, format=iotests.imgfmt)
> + self.assert_qmp(result, 'return', {})
> +
> + self.complete_and_wait(drive="quorum0")
> + result = self.vm.qmp('query-named-block-nodes')
> + self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
> + # TODO: a better test requiring some QEMU infrastructure will be added
> + # to check that this file is really driven by quorum
> + self.vm.shutdown()
> +
> if __name__ == '__main__':
> iotests.main(supported_fmts=['qcow2', 'qed'])
> diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
> index 6d9bee1..73e375a 100644
> --- a/tests/qemu-iotests/041.out
> +++ b/tests/qemu-iotests/041.out
> @@ -1,5 +1,5 @@
> -...........................
> +......................................
> ----------------------------------------------------------------------
> -Ran 27 tests
> +Ran 38 tests
>
> OK
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH v9 4/4] qemu-iotests: Add TestRepairQuorum to 041 to test drive-mirror node-name mode.
2014-06-14 0:44 ` Max Reitz
@ 2014-06-16 9:58 ` Benoît Canet
0 siblings, 0 replies; 10+ messages in thread
From: Benoît Canet @ 2014-06-16 9:58 UTC (permalink / raw)
To: Max Reitz; +Cc: Benoît Canet, kwolf, qemu-devel, stefanha
The Saturday 14 Jun 2014 à 02:44:29 (+0200), Max Reitz wrote :
> On 11.06.2014 15:24, Benoît Canet wrote:
> >The to-replace-node-name is designed to allow repairing of broken Quorum file.
>
> "a broken Quorum file" or "broken Quorum files".
>
> >This patch introduce a new class TestRepairQuorum testing that the feature
>
> *introduces
>
> >works.
> >Some further work will be done on QEMU to improve the robutness of the tests.
>
> *robustness
>
> >Signed-off-by: Benoit Canet <benoit@irqsave.net>
> >---
> > tests/qemu-iotests/041 | 196 ++++++++++++++++++++++++++++++++++++++++++++-
> > tests/qemu-iotests/041.out | 4 +-
> > 2 files changed, 194 insertions(+), 6 deletions(-)
> >
> >diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> >index ec470b2..c0480e1 100755
> >--- a/tests/qemu-iotests/041
> >+++ b/tests/qemu-iotests/041
> >@@ -28,6 +28,12 @@ target_backing_img = os.path.join(iotests.test_dir, 'target-backing.img')
> > test_img = os.path.join(iotests.test_dir, 'test.img')
> > target_img = os.path.join(iotests.test_dir, 'target.img')
> >+quorum_img1 = os.path.join(iotests.test_dir, 'quorum1.img')
> >+quorum_img2 = os.path.join(iotests.test_dir, 'quorum2.img')
> >+quorum_img3 = os.path.join(iotests.test_dir, 'quorum3.img')
> >+quorum_repair_img = os.path.join(iotests.test_dir, 'quorum_repair.img')
> >+quorum_snapshot_file = os.path.join(iotests.test_dir, 'quorum_snapshot.img')
> >+
> > class ImageMirroringTestCase(iotests.QMPTestCase):
> > '''Abstract base class for image mirroring test cases'''
> >@@ -42,8 +48,8 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
> > ready = True
> > def wait_ready_and_cancel(self, drive='drive0'):
> >- self.wait_ready(drive)
> >- event = self.cancel_and_wait()
> >+ self.wait_ready(drive=drive)
> >+ event = self.cancel_and_wait(drive=drive)
>
> My Python is very bad (if existent at all), but is this
> "drive=drive" really necessary? Wouldn't simply "drive" suffice?
drive=drive means pass the drive variable in the parameter of the callee named
drive.
cancel_and_wait have multiple named parameters I need to pass the variable into
the right one.
>
> > self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
> > self.assert_qmp(event, 'data/type', 'mirror')
> > self.assert_qmp(event, 'data/offset', self.image_len)
> >@@ -52,12 +58,12 @@ class ImageMirroringTestCase(iotests.QMPTestCase):
> > def complete_and_wait(self, drive='drive0', wait_ready=True):
> > '''Complete a block job and wait for it to finish'''
> > if wait_ready:
> >- self.wait_ready()
> >+ self.wait_ready(drive=drive)
> > result = self.vm.qmp('block-job-complete', device=drive)
> > self.assert_qmp(result, 'return', {})
> >- event = self.wait_until_completed()
> >+ event = self.wait_until_completed(drive=drive)
> > self.assert_qmp(event, 'data/type', 'mirror')
> > class TestSingleDrive(ImageMirroringTestCase):
> >@@ -718,5 +724,187 @@ class TestUnbackedSource(ImageMirroringTestCase):
> > self.complete_and_wait()
> > self.assert_no_active_block_jobs()
> >+class TestRepairQuorum(ImageMirroringTestCase):
> >+ """ This class test quorum file repair using drive-mirror.
> >+ It's mostly a fork of TestSingleDrive """
> >+ image_len = 1 * 1024 * 1024 # MB
> >+ IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
> >+
> >+ def setUp(self):
> >+ self.vm = iotests.VM()
> >+
> >+ # Add each individual quorum images
> >+ for i in self.IMAGES:
> >+ qemu_img('create', '-f', iotests.imgfmt, i,
> >+ str(TestSingleDrive.image_len))
> >+ # Assign a node name to each quorum image in order to manipulate
> >+ # them
> >+ opts = "node-name=img%i" % self.IMAGES.index(i)
> >+ self.vm = self.vm.add_drive(i, opts)
>
> Just an idea: As you are constantly swapping out img1, why don't you
> write some data to img2 and img3 and different data to img1 to test
> it all how it's supposed to work?
Hmm we have another test for quorum file corruption repairing.
This one is for major failure like the lost of a filer containing a particular
quorum file.
>
> >+
> >+ self.vm.launch()
> >+
> >+ #assemble the quorum block device from the individual files
> >+ args = { "options" : { "driver": "quorum", "id": "quorum0",
> >+ "vote-threshold": 2, "children": [ "img0", "img1", "img2" ] } }
> >+ result = self.vm.qmp("blockdev-add", **args)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+
> >+ def tearDown(self):
> >+ self.vm.shutdown()
> >+ for i in self.IMAGES + [ quorum_repair_img ]:
> >+ # Do a try/except because the test may have deleted some images
> >+ try:
> >+ os.remove(i)
> >+ except OSError:
> >+ pass
> >+
> >+ def test_complete(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name="repair0",
> >+ replaces="img1",
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.complete_and_wait(drive="quorum0")
> >+ result = self.vm.qmp('query-named-block-nodes')
> >+ self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
>
> Hm, why do you know this is at index 0?
Because I ran the comand manually and it's predictible. It's all linked list
insertion and traversal.
>
> >+ # TODO: a better test requiring some QEMU infrastructure will be added
> >+ # to check that this file is really driven by quorum
> >+ self.vm.shutdown()
> >+ self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
> >+ 'target image does not match source after mirroring')
> >+
> >+ def test_cancel(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name="repair0",
> >+ replaces="img1",
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.cancel_and_wait(drive="quorum0", force=True)
> >+ # here we check that the last registered quorum file has not been
> >+ # swapped out and unref
> >+ result = self.vm.qmp('query-named-block-nodes')
> >+ self.assert_qmp(result, 'return[0]/file', quorum_img3)
>
> Why would img3 be affected at all? Aren't you trying to replace
> img1? (Also, again, why index 0?)
It means quorum_img3 should be at position 0 in the linked list: the last inserted
quorum file since we canceled the drive-mirror.
>
> >+ self.vm.shutdown()
> >+
> >+ def test_cancel_after_ready(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name="repair0",
> >+ replaces="img1",
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.wait_ready_and_cancel(drive="quorum0")
> >+ result = self.vm.qmp('query-named-block-nodes')
> >+ # here we check that the last registered quorum file has not been
> >+ # swapped out and unref
> >+ self.assert_qmp(result, 'return[0]/file', quorum_img3)
>
> Same here.
same
>
> >+ self.vm.shutdown()
> >+ self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
> >+ 'target image does not match source after mirroring')
> >+
> >+ def test_pause(self):
> >+ self.assert_no_active_block_jobs()
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name="repair0",
> >+ replaces="img1",
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ result = self.vm.qmp('block-job-pause', device='quorum0')
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ time.sleep(1)
> >+ result = self.vm.qmp('query-block-jobs')
> >+ offset = self.dictpath(result, 'return[0]/offset')
> >+
> >+ time.sleep(1)
> >+ result = self.vm.qmp('query-block-jobs')
> >+ self.assert_qmp(result, 'return[0]/offset', offset)
> >+
> >+ result = self.vm.qmp('block-job-resume', device='quorum0')
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.complete_and_wait(drive="quorum0")
> >+ self.vm.shutdown()
> >+ self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
> >+ 'target image does not match source after mirroring')
> >+
> >+ def test_medium_not_found(self):
> >+ result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full',
>
> I don't know whether this device is supposed to exist forever, but
> if it disappears from default configuration one day, TestSingleDrive
> will break as well, so it's probably fine.
>
> Max
>
> >+ node_name='repair0',
> >+ replaces='img1',
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ def test_image_not_found(self):
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name='repair0',
> >+ replaces='img1',
> >+ mode='existing',
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ def test_device_not_found(self):
> >+ result = self.vm.qmp('drive-mirror', device='nonexistent', sync='full',
> >+ node_name='repair0',
> >+ replaces='img1',
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'error/class', 'DeviceNotFound')
> >+
> >+ def test_wrong_sync_mode(self):
> >+ result = self.vm.qmp('drive-mirror', device='quorum0',
> >+ node_name='repair0',
> >+ replaces='img1',
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ def test_no_node_name(self):
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ replaces='img1',
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ def test_unexistant_replaces(self):
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name='repair0',
> >+ replaces='img77',
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ def test_after_a_quorum_snapshot(self):
> >+ result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
> >+ snapshot_file=quorum_snapshot_file,
> >+ snapshot_node_name="snap1");
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name='repair0',
> >+ replaces="img1",
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'error/class', 'GenericError')
> >+
> >+ result = self.vm.qmp('drive-mirror', device='quorum0', sync='full',
> >+ node_name='repair0',
> >+ replaces="snap1",
> >+ target=quorum_repair_img, format=iotests.imgfmt)
> >+ self.assert_qmp(result, 'return', {})
> >+
> >+ self.complete_and_wait(drive="quorum0")
> >+ result = self.vm.qmp('query-named-block-nodes')
> >+ self.assert_qmp(result, 'return[0]/file', quorum_repair_img)
> >+ # TODO: a better test requiring some QEMU infrastructure will be added
> >+ # to check that this file is really driven by quorum
> >+ self.vm.shutdown()
> >+
> > if __name__ == '__main__':
> > iotests.main(supported_fmts=['qcow2', 'qed'])
> >diff --git a/tests/qemu-iotests/041.out b/tests/qemu-iotests/041.out
> >index 6d9bee1..73e375a 100644
> >--- a/tests/qemu-iotests/041.out
> >+++ b/tests/qemu-iotests/041.out
> >@@ -1,5 +1,5 @@
> >-...........................
> >+......................................
> > ----------------------------------------------------------------------
> >-Ran 27 tests
> >+Ran 38 tests
> > OK
>
^ permalink raw reply [flat|nested] 10+ messages in thread