* [PATCH v6 1/6] qcow2: Fix dangling pointer after reopen for 'file'
2021-07-08 11:47 [PATCH v6 0/6] Make blockdev-reopen stable Kevin Wolf
@ 2021-07-08 11:47 ` Kevin Wolf
2021-07-08 13:26 ` Eric Blake
2021-07-08 13:52 ` Vladimir Sementsov-Ogievskiy
2021-07-08 11:47 ` [PATCH v6 2/6] block: Add bdrv_reopen_queue_free() Kevin Wolf
` (4 subsequent siblings)
5 siblings, 2 replies; 10+ messages in thread
From: Kevin Wolf @ 2021-07-08 11:47 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, berto, qemu-devel, mreitz
Without an external data file, s->data_file is a second pointer with the
same value as bs->file. When changing bs->file to a different BdrvChild
and freeing the old BdrvChild, s->data_file must also be updated,
otherwise it points to freed memory and causes crashes.
This problem was caught by iotests case 245.
Fixes: df2b7086f169239ebad5d150efa29c9bb6d4f820
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qcow2.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index ee4530cdbd..9126127633 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1926,6 +1926,7 @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)
static int qcow2_reopen_prepare(BDRVReopenState *state,
BlockReopenQueue *queue, Error **errp)
{
+ BDRVQcow2State *s = state->bs->opaque;
Qcow2ReopenState *r;
int ret;
@@ -1956,6 +1957,16 @@ static int qcow2_reopen_prepare(BDRVReopenState *state,
}
}
+ /*
+ * Without an external data file, s->data_file points to the same BdrvChild
+ * as bs->file. It needs to be resynced after reopen because bs->file may
+ * be changed. We can't use it in the meantime.
+ */
+ if (!has_data_file(state->bs)) {
+ assert(s->data_file == state->bs->file);
+ s->data_file = NULL;
+ }
+
return 0;
fail:
@@ -1966,7 +1977,16 @@ fail:
static void qcow2_reopen_commit(BDRVReopenState *state)
{
+ BDRVQcow2State *s = state->bs->opaque;
+
qcow2_update_options_commit(state->bs, state->opaque);
+ if (!s->data_file) {
+ /*
+ * If we don't have an external data file, s->data_file was cleared by
+ * qcow2_reopen_prepare() and needs to be updated.
+ */
+ s->data_file = state->bs->file;
+ }
g_free(state->opaque);
}
@@ -1990,6 +2010,15 @@ static void qcow2_reopen_commit_post(BDRVReopenState *state)
static void qcow2_reopen_abort(BDRVReopenState *state)
{
+ BDRVQcow2State *s = state->bs->opaque;
+
+ if (!s->data_file) {
+ /*
+ * If we don't have an external data file, s->data_file was cleared by
+ * qcow2_reopen_prepare() and needs to be restored.
+ */
+ s->data_file = state->bs->file;
+ }
qcow2_update_options_abort(state->bs, state->opaque);
g_free(state->opaque);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v6 1/6] qcow2: Fix dangling pointer after reopen for 'file'
2021-07-08 11:47 ` [PATCH v6 1/6] qcow2: Fix dangling pointer after reopen for 'file' Kevin Wolf
@ 2021-07-08 13:26 ` Eric Blake
2021-07-08 13:52 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 10+ messages in thread
From: Eric Blake @ 2021-07-08 13:26 UTC (permalink / raw)
To: Kevin Wolf; +Cc: vsementsov, berto, qemu-devel, qemu-block, mreitz
On Thu, Jul 08, 2021 at 01:47:04PM +0200, Kevin Wolf wrote:
> Without an external data file, s->data_file is a second pointer with the
> same value as bs->file. When changing bs->file to a different BdrvChild
> and freeing the old BdrvChild, s->data_file must also be updated,
> otherwise it points to freed memory and causes crashes.
>
> This problem was caught by iotests case 245.
>
> Fixes: df2b7086f169239ebad5d150efa29c9bb6d4f820
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/qcow2.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v6 1/6] qcow2: Fix dangling pointer after reopen for 'file'
2021-07-08 11:47 ` [PATCH v6 1/6] qcow2: Fix dangling pointer after reopen for 'file' Kevin Wolf
2021-07-08 13:26 ` Eric Blake
@ 2021-07-08 13:52 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 10+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-07-08 13:52 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: mreitz, berto, qemu-devel
08.07.2021 14:47, Kevin Wolf wrote:
> Without an external data file, s->data_file is a second pointer with the
> same value as bs->file. When changing bs->file to a different BdrvChild
> and freeing the old BdrvChild, s->data_file must also be updated,
> otherwise it points to freed memory and causes crashes.
>
> This problem was caught by iotests case 245.
>
> Fixes: df2b7086f169239ebad5d150efa29c9bb6d4f820
> Signed-off-by: Kevin Wolf<kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v6 2/6] block: Add bdrv_reopen_queue_free()
2021-07-08 11:47 [PATCH v6 0/6] Make blockdev-reopen stable Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 1/6] qcow2: Fix dangling pointer after reopen for 'file' Kevin Wolf
@ 2021-07-08 11:47 ` Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 3/6] block: Acquire AioContexts during bdrv_reopen_multiple() Kevin Wolf
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2021-07-08 11:47 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, berto, qemu-devel, mreitz
From: Alberto Garcia <berto@igalia.com>
Move the code to free a BlockReopenQueue to a separate function.
It will be used in a subsequent patch.
[ kwolf: Also free explicit_options and options, and explicitly
qobject_ref() the value when it continues to be used. This makes
future memory leaks less likely. ]
Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block.h | 1 +
block.c | 22 ++++++++++++++++------
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 7ec77ecb1a..6d42992985 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -386,6 +386,7 @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name,
BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
BlockDriverState *bs, QDict *options,
bool keep_old_opts);
+void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue);
int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
Error **errp);
diff --git a/block.c b/block.c
index acd35cb0cb..ee9b46e95e 100644
--- a/block.c
+++ b/block.c
@@ -4095,6 +4095,19 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
NULL, 0, keep_old_opts);
}
+void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue)
+{
+ if (bs_queue) {
+ BlockReopenQueueEntry *bs_entry, *next;
+ QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
+ qobject_unref(bs_entry->state.explicit_options);
+ qobject_unref(bs_entry->state.options);
+ g_free(bs_entry);
+ }
+ g_free(bs_queue);
+ }
+}
+
/*
* Reopen multiple BlockDriverStates atomically & transactionally.
*
@@ -4197,15 +4210,10 @@ abort:
if (bs_entry->prepared) {
bdrv_reopen_abort(&bs_entry->state);
}
- qobject_unref(bs_entry->state.explicit_options);
- qobject_unref(bs_entry->state.options);
}
cleanup:
- QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
- g_free(bs_entry);
- }
- g_free(bs_queue);
+ bdrv_reopen_queue_free(bs_queue);
return ret;
}
@@ -4573,6 +4581,8 @@ static void bdrv_reopen_commit(BDRVReopenState *reopen_state)
/* set BDS specific flags now */
qobject_unref(bs->explicit_options);
qobject_unref(bs->options);
+ qobject_ref(reopen_state->explicit_options);
+ qobject_ref(reopen_state->options);
bs->explicit_options = reopen_state->explicit_options;
bs->options = reopen_state->options;
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 3/6] block: Acquire AioContexts during bdrv_reopen_multiple()
2021-07-08 11:47 [PATCH v6 0/6] Make blockdev-reopen stable Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 1/6] qcow2: Fix dangling pointer after reopen for 'file' Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 2/6] block: Add bdrv_reopen_queue_free() Kevin Wolf
@ 2021-07-08 11:47 ` Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 4/6] block: Support multiple reopening with x-blockdev-reopen Kevin Wolf
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2021-07-08 11:47 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, berto, qemu-devel, mreitz
As the BlockReopenQueue can contain nodes in multiple AioContexts, only
one of which may be locked when AIO_WAIT_WHILE() can be called, we can't
let the caller lock the right contexts. Instead, individually lock the
AioContext of a single node when iterating the queue.
Reintroduce bdrv_reopen() as a wrapper for reopening a single node that
drains the node and temporarily drops the AioContext lock for
bdrv_reopen_multiple().
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block.h | 2 ++
block.c | 49 ++++++++++++++++++++++++++++++++++++-------
block/replication.c | 7 +++++++
blockdev.c | 5 +++++
qemu-io-cmds.c | 7 +------
5 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 6d42992985..3477290f9a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -388,6 +388,8 @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
bool keep_old_opts);
void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue);
int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp);
+int bdrv_reopen(BlockDriverState *bs, QDict *opts, bool keep_old_opts,
+ Error **errp);
int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
Error **errp);
int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset,
diff --git a/block.c b/block.c
index ee9b46e95e..06fb69df5c 100644
--- a/block.c
+++ b/block.c
@@ -4124,19 +4124,26 @@ void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue)
*
* All affected nodes must be drained between bdrv_reopen_queue() and
* bdrv_reopen_multiple().
+ *
+ * To be called from the main thread, with all other AioContexts unlocked.
*/
int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
{
int ret = -1;
BlockReopenQueueEntry *bs_entry, *next;
+ AioContext *ctx;
Transaction *tran = tran_new();
g_autoptr(GHashTable) found = NULL;
g_autoptr(GSList) refresh_list = NULL;
+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());
assert(bs_queue != NULL);
QTAILQ_FOREACH(bs_entry, bs_queue, entry) {
+ ctx = bdrv_get_aio_context(bs_entry->state.bs);
+ aio_context_acquire(ctx);
ret = bdrv_flush(bs_entry->state.bs);
+ aio_context_release(ctx);
if (ret < 0) {
error_setg_errno(errp, -ret, "Error flushing drive");
goto abort;
@@ -4145,7 +4152,10 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
QTAILQ_FOREACH(bs_entry, bs_queue, entry) {
assert(bs_entry->state.bs->quiesce_counter > 0);
+ ctx = bdrv_get_aio_context(bs_entry->state.bs);
+ aio_context_acquire(ctx);
ret = bdrv_reopen_prepare(&bs_entry->state, bs_queue, tran, errp);
+ aio_context_release(ctx);
if (ret < 0) {
goto abort;
}
@@ -4188,7 +4198,10 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
* to first element.
*/
QTAILQ_FOREACH_REVERSE(bs_entry, bs_queue, entry) {
+ ctx = bdrv_get_aio_context(bs_entry->state.bs);
+ aio_context_acquire(ctx);
bdrv_reopen_commit(&bs_entry->state);
+ aio_context_release(ctx);
}
tran_commit(tran);
@@ -4197,7 +4210,10 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
BlockDriverState *bs = bs_entry->state.bs;
if (bs->drv->bdrv_reopen_commit_post) {
+ ctx = bdrv_get_aio_context(bs);
+ aio_context_acquire(ctx);
bs->drv->bdrv_reopen_commit_post(&bs_entry->state);
+ aio_context_release(ctx);
}
}
@@ -4208,7 +4224,10 @@ abort:
tran_abort(tran);
QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {
if (bs_entry->prepared) {
+ ctx = bdrv_get_aio_context(bs_entry->state.bs);
+ aio_context_acquire(ctx);
bdrv_reopen_abort(&bs_entry->state);
+ aio_context_release(ctx);
}
}
@@ -4218,23 +4237,39 @@ cleanup:
return ret;
}
-int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
- Error **errp)
+int bdrv_reopen(BlockDriverState *bs, QDict *opts, bool keep_old_opts,
+ Error **errp)
{
- int ret;
+ AioContext *ctx = bdrv_get_aio_context(bs);
BlockReopenQueue *queue;
- QDict *opts = qdict_new();
-
- qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only);
+ int ret;
bdrv_subtree_drained_begin(bs);
- queue = bdrv_reopen_queue(NULL, bs, opts, true);
+ if (ctx != qemu_get_aio_context()) {
+ aio_context_release(ctx);
+ }
+
+ queue = bdrv_reopen_queue(NULL, bs, opts, keep_old_opts);
ret = bdrv_reopen_multiple(queue, errp);
+
+ if (ctx != qemu_get_aio_context()) {
+ aio_context_acquire(ctx);
+ }
bdrv_subtree_drained_end(bs);
return ret;
}
+int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only,
+ Error **errp)
+{
+ QDict *opts = qdict_new();
+
+ qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only);
+
+ return bdrv_reopen(bs, opts, true, errp);
+}
+
/*
* Take a BDRVReopenState and check if the value of 'backing' in the
* reopen_state->options QDict is valid or not.
diff --git a/block/replication.c b/block/replication.c
index 52163f2d1f..774e15df16 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -390,7 +390,14 @@ static void reopen_backing_file(BlockDriverState *bs, bool writable,
}
if (reopen_queue) {
+ AioContext *ctx = bdrv_get_aio_context(bs);
+ if (ctx != qemu_get_aio_context()) {
+ aio_context_release(ctx);
+ }
bdrv_reopen_multiple(reopen_queue, errp);
+ if (ctx != qemu_get_aio_context()) {
+ aio_context_acquire(ctx);
+ }
}
bdrv_subtree_drained_end(s->hidden_disk->bs);
diff --git a/blockdev.c b/blockdev.c
index f08192deda..f657d090d3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3593,8 +3593,13 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)
ctx = bdrv_get_aio_context(bs);
aio_context_acquire(ctx);
bdrv_subtree_drained_begin(bs);
+ aio_context_release(ctx);
+
queue = bdrv_reopen_queue(NULL, bs, qdict, false);
bdrv_reopen_multiple(queue, errp);
+
+ ctx = bdrv_get_aio_context(bs);
+ aio_context_acquire(ctx);
bdrv_subtree_drained_end(bs);
aio_context_release(ctx);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e8d862a426..46593d632d 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2116,8 +2116,6 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
bool writethrough = !blk_enable_write_cache(blk);
bool has_rw_option = false;
bool has_cache_option = false;
-
- BlockReopenQueue *brq;
Error *local_err = NULL;
while ((c = getopt(argc, argv, "c:o:rw")) != -1) {
@@ -2210,10 +2208,7 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
qdict_put_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, flags & BDRV_O_NO_FLUSH);
}
- bdrv_subtree_drained_begin(bs);
- brq = bdrv_reopen_queue(NULL, bs, opts, true);
- bdrv_reopen_multiple(brq, &local_err);
- bdrv_subtree_drained_end(bs);
+ bdrv_reopen(bs, opts, true, &local_err);
if (local_err) {
error_report_err(local_err);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 4/6] block: Support multiple reopening with x-blockdev-reopen
2021-07-08 11:47 [PATCH v6 0/6] Make blockdev-reopen stable Kevin Wolf
` (2 preceding siblings ...)
2021-07-08 11:47 ` [PATCH v6 3/6] block: Acquire AioContexts during bdrv_reopen_multiple() Kevin Wolf
@ 2021-07-08 11:47 ` Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 5/6] iotests: Test reopening multiple devices at the same time Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 6/6] block: Make blockdev-reopen stable API Kevin Wolf
5 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2021-07-08 11:47 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, berto, qemu-devel, mreitz
From: Alberto Garcia <berto@igalia.com>
[ kwolf: Fixed AioContext locking ]
Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
qapi/block-core.json | 18 +++--
blockdev.c | 81 ++++++++++---------
tests/qemu-iotests/155 | 9 ++-
tests/qemu-iotests/165 | 4 +-
tests/qemu-iotests/245 | 27 ++++---
tests/qemu-iotests/248 | 2 +-
tests/qemu-iotests/248.out | 2 +-
tests/qemu-iotests/296 | 9 ++-
tests/qemu-iotests/298 | 4 +-
.../tests/remove-bitmap-from-backing | 18 +++--
10 files changed, 99 insertions(+), 75 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 4a46552816..052520331e 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4221,13 +4221,15 @@
##
# @x-blockdev-reopen:
#
-# Reopens a block device using the given set of options. Any option
-# not specified will be reset to its default value regardless of its
-# previous status. If an option cannot be changed or a particular
+# Reopens one or more block devices using the given set of options.
+# Any option not specified will be reset to its default value regardless
+# of its previous status. If an option cannot be changed or a particular
# driver does not support reopening then the command will return an
-# error.
+# error. All devices in the list are reopened in one transaction, so
+# if one of them fails then the whole transaction is cancelled.
#
-# The top-level @node-name option (from BlockdevOptions) must be
+# The command receives a list of block devices to reopen. For each one
+# of them, the top-level @node-name option (from BlockdevOptions) must be
# specified and is used to select the block device to be reopened.
# Other @node-name options must be either omitted or set to the
# current name of the appropriate node. This command won't change any
@@ -4247,8 +4249,8 @@
#
# 4) NULL: the current child (if any) is detached.
#
-# Options (1) and (2) are supported in all cases, but at the moment
-# only @backing allows replacing or detaching an existing child.
+# Options (1) and (2) are supported in all cases. Option (3) is
+# supported for @file and @backing, and option (4) for @backing only.
#
# Unlike with blockdev-add, the @backing option must always be present
# unless the node being reopened does not have a backing file and its
@@ -4258,7 +4260,7 @@
# Since: 4.0
##
{ 'command': 'x-blockdev-reopen',
- 'data': 'BlockdevOptions', 'boxed': true }
+ 'data': { 'options': ['BlockdevOptions'] } }
##
# @blockdev-del:
diff --git a/blockdev.c b/blockdev.c
index f657d090d3..fddcccbdbd 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3560,51 +3560,60 @@ fail:
visit_free(v);
}
-void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)
-{
- BlockDriverState *bs;
- AioContext *ctx;
- QObject *obj;
- Visitor *v = qobject_output_visitor_new(&obj);
- BlockReopenQueue *queue;
- QDict *qdict;
+void qmp_x_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp)
+{
+ BlockReopenQueue *queue = NULL;
+ GSList *drained = NULL;
+
+ /* Add each one of the BDS that we want to reopen to the queue */
+ for (; reopen_list != NULL; reopen_list = reopen_list->next) {
+ BlockdevOptions *options = reopen_list->value;
+ BlockDriverState *bs;
+ AioContext *ctx;
+ QObject *obj;
+ Visitor *v;
+ QDict *qdict;
+
+ /* Check for the selected node name */
+ if (!options->has_node_name) {
+ error_setg(errp, "node-name not specified");
+ goto fail;
+ }
- /* Check for the selected node name */
- if (!options->has_node_name) {
- error_setg(errp, "node-name not specified");
- goto fail;
- }
+ bs = bdrv_find_node(options->node_name);
+ if (!bs) {
+ error_setg(errp, "Failed to find node with node-name='%s'",
+ options->node_name);
+ goto fail;
+ }
- bs = bdrv_find_node(options->node_name);
- if (!bs) {
- error_setg(errp, "Failed to find node with node-name='%s'",
- options->node_name);
- goto fail;
- }
+ /* Put all options in a QDict and flatten it */
+ v = qobject_output_visitor_new(&obj);
+ visit_type_BlockdevOptions(v, NULL, &options, &error_abort);
+ visit_complete(v, &obj);
+ visit_free(v);
- /* Put all options in a QDict and flatten it */
- visit_type_BlockdevOptions(v, NULL, &options, &error_abort);
- visit_complete(v, &obj);
- qdict = qobject_to(QDict, obj);
+ qdict = qobject_to(QDict, obj);
- qdict_flatten(qdict);
+ qdict_flatten(qdict);
- /* Perform the reopen operation */
- ctx = bdrv_get_aio_context(bs);
- aio_context_acquire(ctx);
- bdrv_subtree_drained_begin(bs);
- aio_context_release(ctx);
+ ctx = bdrv_get_aio_context(bs);
+ aio_context_acquire(ctx);
- queue = bdrv_reopen_queue(NULL, bs, qdict, false);
- bdrv_reopen_multiple(queue, errp);
+ bdrv_subtree_drained_begin(bs);
+ queue = bdrv_reopen_queue(queue, bs, qdict, false);
+ drained = g_slist_prepend(drained, bs);
- ctx = bdrv_get_aio_context(bs);
- aio_context_acquire(ctx);
- bdrv_subtree_drained_end(bs);
- aio_context_release(ctx);
+ aio_context_release(ctx);
+ }
+
+ /* Perform the reopen operation */
+ bdrv_reopen_multiple(queue, errp);
+ queue = NULL;
fail:
- visit_free(v);
+ bdrv_reopen_queue_free(queue);
+ g_slist_free_full(drained, (GDestroyNotify) bdrv_subtree_drained_end);
}
void qmp_blockdev_del(const char *node_name, Error **errp)
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index bafef9dd9a..2947bfb81a 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -261,9 +261,12 @@ class TestBlockdevMirrorReopen(MirrorBaseClass):
result = self.vm.qmp('blockdev-add', node_name="backing",
driver="null-co")
self.assert_qmp(result, 'return', {})
- result = self.vm.qmp('x-blockdev-reopen', node_name="target",
- driver=iotests.imgfmt, file="target-file",
- backing="backing")
+ result = self.vm.qmp('x-blockdev-reopen', options=[{
+ 'node-name': "target",
+ 'driver': iotests.imgfmt,
+ 'file': "target-file",
+ 'backing': "backing"
+ }])
self.assert_qmp(result, 'return', {})
class TestBlockdevMirrorReopenIothread(TestBlockdevMirrorReopen):
diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
index abc4ffadd5..ef4cf14516 100755
--- a/tests/qemu-iotests/165
+++ b/tests/qemu-iotests/165
@@ -137,7 +137,7 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
assert sha256_1 == self.getSha256()
# Reopen to RW
- result = self.vm.qmp('x-blockdev-reopen', **{
+ result = self.vm.qmp('x-blockdev-reopen', options=[{
'node-name': 'node0',
'driver': iotests.imgfmt,
'file': {
@@ -145,7 +145,7 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
'filename': disk
},
'read-only': False
- })
+ }])
self.assert_qmp(result, 'return', {})
# Check that bitmap is reopened to RW and we can write to it.
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 0295129cbb..ca08955207 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -85,8 +85,18 @@ class TestBlockdevReopen(iotests.QMPTestCase):
"Expected output of %d qemu-io commands, found %d" %
(found, self.total_io_cmds))
- # Run x-blockdev-reopen with 'opts' but applying 'newopts'
- # on top of it. The original 'opts' dict is unmodified
+ # Run x-blockdev-reopen on a list of block devices
+ def reopenMultiple(self, opts, errmsg = None):
+ result = self.vm.qmp('x-blockdev-reopen', conv_keys=False, options=opts)
+ if errmsg:
+ self.assert_qmp(result, 'error/class', 'GenericError')
+ self.assert_qmp(result, 'error/desc', errmsg)
+ else:
+ self.assert_qmp(result, 'return', {})
+
+ # Run x-blockdev-reopen on a single block device (specified by
+ # 'opts') but applying 'newopts' on top of it. The original 'opts'
+ # dict is unmodified
def reopen(self, opts, newopts = {}, errmsg = None):
opts = copy.deepcopy(opts)
@@ -101,12 +111,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
subdict = opts[prefix]
subdict[key] = value
- result = self.vm.qmp('x-blockdev-reopen', conv_keys = False, **opts)
- if errmsg:
- self.assert_qmp(result, 'error/class', 'GenericError')
- self.assert_qmp(result, 'error/desc', errmsg)
- else:
- self.assert_qmp(result, 'return', {})
+ self.reopenMultiple([ opts ], errmsg)
# Run query-named-block-nodes and return the specified entry
@@ -142,10 +147,10 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# We cannot change any of these
self.reopen(opts, {'node-name': 'not-found'}, "Failed to find node with node-name='not-found'")
self.reopen(opts, {'node-name': ''}, "Failed to find node with node-name=''")
- self.reopen(opts, {'node-name': None}, "Invalid parameter type for 'node-name', expected: string")
+ self.reopen(opts, {'node-name': None}, "Invalid parameter type for 'options[0].node-name', expected: string")
self.reopen(opts, {'driver': 'raw'}, "Cannot change the option 'driver'")
self.reopen(opts, {'driver': ''}, "Invalid parameter ''")
- self.reopen(opts, {'driver': None}, "Invalid parameter type for 'driver', expected: string")
+ self.reopen(opts, {'driver': None}, "Invalid parameter type for 'options[0].driver', expected: string")
self.reopen(opts, {'file': 'not-found'}, "Cannot find device='' nor node-name='not-found'")
self.reopen(opts, {'file': ''}, "Cannot find device='' nor node-name=''")
self.reopen(opts, {'file': None}, "Invalid parameter type for 'file', expected: BlockdevRef")
@@ -154,7 +159,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
self.reopen(opts, {'file.filename': hd_path[1]}, "Cannot change the option 'filename'")
self.reopen(opts, {'file.aio': 'native'}, "Cannot change the option 'aio'")
self.reopen(opts, {'file.locking': 'off'}, "Cannot change the option 'locking'")
- self.reopen(opts, {'file.filename': None}, "Invalid parameter type for 'file.filename', expected: string")
+ self.reopen(opts, {'file.filename': None}, "Invalid parameter type for 'options[0].file.filename', expected: string")
# node-name is optional in BlockdevOptions, but x-blockdev-reopen needs it
del opts['node-name']
diff --git a/tests/qemu-iotests/248 b/tests/qemu-iotests/248
index 4daaed1530..03911333c4 100755
--- a/tests/qemu-iotests/248
+++ b/tests/qemu-iotests/248
@@ -63,7 +63,7 @@ vm.get_qmp_events()
del blockdev_opts['file']['size']
vm.qmp_log('x-blockdev-reopen', filters=[filter_qmp_testfiles],
- **blockdev_opts)
+ options = [ blockdev_opts ])
vm.qmp_log('block-job-resume', device='drive0')
vm.event_wait('JOB_STATUS_CHANGE', timeout=1.0,
diff --git a/tests/qemu-iotests/248.out b/tests/qemu-iotests/248.out
index 369b25bf26..893f625347 100644
--- a/tests/qemu-iotests/248.out
+++ b/tests/qemu-iotests/248.out
@@ -2,7 +2,7 @@
{"return": {}}
{"execute": "blockdev-mirror", "arguments": {"device": "drive0", "on-target-error": "enospc", "sync": "full", "target": "target"}}
{"return": {}}
-{"execute": "x-blockdev-reopen", "arguments": {"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}}
+{"execute": "x-blockdev-reopen", "arguments": {"options": [{"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}]}}
{"return": {}}
{"execute": "block-job-resume", "arguments": {"device": "drive0"}}
{"return": {}}
diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296
index 7c65e987a1..fa07b98bb4 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -120,8 +120,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
command = 'x-blockdev-reopen' if reOpen else 'blockdev-add'
- result = vm.qmp(command, **
- {
+ opts = {
'driver': iotests.imgfmt,
'node-name': id,
'read-only': readOnly,
@@ -131,7 +130,11 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
'filename': test_img,
}
}
- )
+
+ if reOpen:
+ result = vm.qmp(command, options=[opts])
+ else:
+ result = vm.qmp(command, **opts)
self.assert_qmp(result, 'return', {})
diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
index d535946b5f..a5b0d91224 100755
--- a/tests/qemu-iotests/298
+++ b/tests/qemu-iotests/298
@@ -98,7 +98,7 @@ class TestPreallocateFilter(TestPreallocateBase):
self.check_big()
def test_reopen_opts(self):
- result = self.vm.qmp('x-blockdev-reopen', **{
+ result = self.vm.qmp('x-blockdev-reopen', options=[{
'node-name': 'disk',
'driver': iotests.imgfmt,
'file': {
@@ -112,7 +112,7 @@ class TestPreallocateFilter(TestPreallocateBase):
'filename': disk
}
}
- })
+ }])
self.assert_qmp(result, 'return', {})
self.vm.hmp_qemu_io('drive0', 'write 0 1M')
diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing b/tests/qemu-iotests/tests/remove-bitmap-from-backing
index 0ea4c36507..0b07f7e836 100755
--- a/tests/qemu-iotests/tests/remove-bitmap-from-backing
+++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing
@@ -41,13 +41,15 @@ log('Trying to remove persistent bitmap from r-o base node, should fail:')
vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0')
new_base_opts = {
- 'node-name': 'base',
- 'driver': 'qcow2',
- 'file': {
- 'driver': 'file',
- 'filename': base
- },
- 'read-only': False
+ 'options': [{
+ 'node-name': 'base',
+ 'driver': 'qcow2',
+ 'file': {
+ 'driver': 'file',
+ 'filename': base
+ },
+ 'read-only': False
+ }]
}
# Don't want to bother with filtering qmp_log for reopen command
@@ -58,7 +60,7 @@ if result != {'return': {}}:
log('Remove persistent bitmap from base node reopened to RW:')
vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0')
-new_base_opts['read-only'] = True
+new_base_opts['options'][0]['read-only'] = True
result = vm.qmp('x-blockdev-reopen', **new_base_opts)
if result != {'return': {}}:
log('Failed to reopen: ' + str(result))
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 5/6] iotests: Test reopening multiple devices at the same time
2021-07-08 11:47 [PATCH v6 0/6] Make blockdev-reopen stable Kevin Wolf
` (3 preceding siblings ...)
2021-07-08 11:47 ` [PATCH v6 4/6] block: Support multiple reopening with x-blockdev-reopen Kevin Wolf
@ 2021-07-08 11:47 ` Kevin Wolf
2021-07-08 11:47 ` [PATCH v6 6/6] block: Make blockdev-reopen stable API Kevin Wolf
5 siblings, 0 replies; 10+ messages in thread
From: Kevin Wolf @ 2021-07-08 11:47 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, berto, qemu-devel, mreitz
From: Alberto Garcia <berto@igalia.com>
This test swaps the images used by two active block devices.
This is now possible thanks to the new ability to run
x-blockdev-reopen on multiple devices at the same time.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
tests/qemu-iotests/245 | 47 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/245.out | 4 ++--
2 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index ca08955207..8bc8101e6d 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -649,6 +649,53 @@ class TestBlockdevReopen(iotests.QMPTestCase):
'-c', 'read -P 0x40 0x40008 1',
'-c', 'read -P 0x80 0x40010 1', hd_path[0])
+ # Swap the disk images of two active block devices
+ def test_swap_files(self):
+ # Add hd0 and hd2 (none of them with backing files)
+ opts0 = hd_opts(0)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts0)
+ self.assert_qmp(result, 'return', {})
+
+ opts2 = hd_opts(2)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts2)
+ self.assert_qmp(result, 'return', {})
+
+ # Write different data to both block devices
+ self.run_qemu_io("hd0", "write -P 0xa0 0 1k")
+ self.run_qemu_io("hd2", "write -P 0xa2 0 1k")
+
+ # Check that the data reads correctly
+ self.run_qemu_io("hd0", "read -P 0xa0 0 1k")
+ self.run_qemu_io("hd2", "read -P 0xa2 0 1k")
+
+ # It's not possible to make a block device use an image that
+ # is already being used by the other device.
+ self.reopen(opts0, {'file': 'hd2-file'},
+ "Permission conflict on node 'hd2-file': permissions "
+ "'write, resize' are both required by node 'hd2' (uses "
+ "node 'hd2-file' as 'file' child) and unshared by node "
+ "'hd0' (uses node 'hd2-file' as 'file' child).")
+ self.reopen(opts2, {'file': 'hd0-file'},
+ "Permission conflict on node 'hd0-file': permissions "
+ "'write, resize' are both required by node 'hd0' (uses "
+ "node 'hd0-file' as 'file' child) and unshared by node "
+ "'hd2' (uses node 'hd0-file' as 'file' child).")
+
+ # But we can swap the images if we reopen both devices at the
+ # same time
+ opts0['file'] = 'hd2-file'
+ opts2['file'] = 'hd0-file'
+ self.reopenMultiple([opts0, opts2])
+ self.run_qemu_io("hd0", "read -P 0xa2 0 1k")
+ self.run_qemu_io("hd2", "read -P 0xa0 0 1k")
+
+ # And we can of course come back to the original state
+ opts0['file'] = 'hd0-file'
+ opts2['file'] = 'hd2-file'
+ self.reopenMultiple([opts0, opts2])
+ self.run_qemu_io("hd0", "read -P 0xa0 0 1k")
+ self.run_qemu_io("hd2", "read -P 0xa2 0 1k")
+
# Misc reopen tests with different block drivers
@iotests.skip_if_unsupported(['quorum', 'throttle'])
def test_misc_drivers(self):
diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out
index daf1e51922..4eced19294 100644
--- a/tests/qemu-iotests/245.out
+++ b/tests/qemu-iotests/245.out
@@ -17,8 +17,8 @@ read 1/1 bytes at offset 262152
read 1/1 bytes at offset 262160
1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-..............
+...............
----------------------------------------------------------------------
-Ran 24 tests
+Ran 25 tests
OK
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v6 6/6] block: Make blockdev-reopen stable API
2021-07-08 11:47 [PATCH v6 0/6] Make blockdev-reopen stable Kevin Wolf
` (4 preceding siblings ...)
2021-07-08 11:47 ` [PATCH v6 5/6] iotests: Test reopening multiple devices at the same time Kevin Wolf
@ 2021-07-08 11:47 ` Kevin Wolf
2021-07-08 14:37 ` Eric Blake
5 siblings, 1 reply; 10+ messages in thread
From: Kevin Wolf @ 2021-07-08 11:47 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, berto, qemu-devel, mreitz
From: Alberto Garcia <berto@igalia.com>
This patch drops the 'x-' prefix from x-blockdev-reopen.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
qapi/block-core.json | 6 +++---
blockdev.c | 2 +-
tests/qemu-iotests/155 | 2 +-
tests/qemu-iotests/165 | 2 +-
tests/qemu-iotests/245 | 10 +++++-----
tests/qemu-iotests/248 | 2 +-
tests/qemu-iotests/248.out | 2 +-
tests/qemu-iotests/296 | 2 +-
tests/qemu-iotests/298 | 2 +-
tests/qemu-iotests/tests/remove-bitmap-from-backing | 4 ++--
10 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 052520331e..2eb399f0d4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4219,7 +4219,7 @@
{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true }
##
-# @x-blockdev-reopen:
+# @blockdev-reopen:
#
# Reopens one or more block devices using the given set of options.
# Any option not specified will be reset to its default value regardless
@@ -4257,9 +4257,9 @@
# image does not have a default backing file name as part of its
# metadata.
#
-# Since: 4.0
+# Since: 6.0
##
-{ 'command': 'x-blockdev-reopen',
+{ 'command': 'blockdev-reopen',
'data': { 'options': ['BlockdevOptions'] } }
##
diff --git a/blockdev.c b/blockdev.c
index fddcccbdbd..6466a106f4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3560,7 +3560,7 @@ fail:
visit_free(v);
}
-void qmp_x_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp)
+void qmp_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp)
{
BlockReopenQueue *queue = NULL;
GSList *drained = NULL;
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index 2947bfb81a..eadda52615 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -261,7 +261,7 @@ class TestBlockdevMirrorReopen(MirrorBaseClass):
result = self.vm.qmp('blockdev-add', node_name="backing",
driver="null-co")
self.assert_qmp(result, 'return', {})
- result = self.vm.qmp('x-blockdev-reopen', options=[{
+ result = self.vm.qmp('blockdev-reopen', options=[{
'node-name': "target",
'driver': iotests.imgfmt,
'file': "target-file",
diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165
index ef4cf14516..ce499946b8 100755
--- a/tests/qemu-iotests/165
+++ b/tests/qemu-iotests/165
@@ -137,7 +137,7 @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase):
assert sha256_1 == self.getSha256()
# Reopen to RW
- result = self.vm.qmp('x-blockdev-reopen', options=[{
+ result = self.vm.qmp('blockdev-reopen', options=[{
'node-name': 'node0',
'driver': iotests.imgfmt,
'file': {
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 8bc8101e6d..bf8261eec0 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -1,7 +1,7 @@
#!/usr/bin/env python3
# group: rw
#
-# Test cases for the QMP 'x-blockdev-reopen' command
+# Test cases for the QMP 'blockdev-reopen' command
#
# Copyright (C) 2018-2019 Igalia, S.L.
# Author: Alberto Garcia <berto@igalia.com>
@@ -85,16 +85,16 @@ class TestBlockdevReopen(iotests.QMPTestCase):
"Expected output of %d qemu-io commands, found %d" %
(found, self.total_io_cmds))
- # Run x-blockdev-reopen on a list of block devices
+ # Run blockdev-reopen on a list of block devices
def reopenMultiple(self, opts, errmsg = None):
- result = self.vm.qmp('x-blockdev-reopen', conv_keys=False, options=opts)
+ result = self.vm.qmp('blockdev-reopen', conv_keys=False, options=opts)
if errmsg:
self.assert_qmp(result, 'error/class', 'GenericError')
self.assert_qmp(result, 'error/desc', errmsg)
else:
self.assert_qmp(result, 'return', {})
- # Run x-blockdev-reopen on a single block device (specified by
+ # Run blockdev-reopen on a single block device (specified by
# 'opts') but applying 'newopts' on top of it. The original 'opts'
# dict is unmodified
def reopen(self, opts, newopts = {}, errmsg = None):
@@ -161,7 +161,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
self.reopen(opts, {'file.locking': 'off'}, "Cannot change the option 'locking'")
self.reopen(opts, {'file.filename': None}, "Invalid parameter type for 'options[0].file.filename', expected: string")
- # node-name is optional in BlockdevOptions, but x-blockdev-reopen needs it
+ # node-name is optional in BlockdevOptions, but blockdev-reopen needs it
del opts['node-name']
self.reopen(opts, {}, "node-name not specified")
diff --git a/tests/qemu-iotests/248 b/tests/qemu-iotests/248
index 03911333c4..2ec2416e8a 100755
--- a/tests/qemu-iotests/248
+++ b/tests/qemu-iotests/248
@@ -62,7 +62,7 @@ vm.event_wait('JOB_STATUS_CHANGE', timeout=3.0,
vm.get_qmp_events()
del blockdev_opts['file']['size']
-vm.qmp_log('x-blockdev-reopen', filters=[filter_qmp_testfiles],
+vm.qmp_log('blockdev-reopen', filters=[filter_qmp_testfiles],
options = [ blockdev_opts ])
vm.qmp_log('block-job-resume', device='drive0')
diff --git a/tests/qemu-iotests/248.out b/tests/qemu-iotests/248.out
index 893f625347..66e94ccd7e 100644
--- a/tests/qemu-iotests/248.out
+++ b/tests/qemu-iotests/248.out
@@ -2,7 +2,7 @@
{"return": {}}
{"execute": "blockdev-mirror", "arguments": {"device": "drive0", "on-target-error": "enospc", "sync": "full", "target": "target"}}
{"return": {}}
-{"execute": "x-blockdev-reopen", "arguments": {"options": [{"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}]}}
+{"execute": "blockdev-reopen", "arguments": {"options": [{"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}]}}
{"return": {}}
{"execute": "block-job-resume", "arguments": {"device": "drive0"}}
{"return": {}}
diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296
index fa07b98bb4..099a3eeaa5 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -118,7 +118,7 @@ class EncryptionSetupTestCase(iotests.QMPTestCase):
def openImageQmp(self, vm, id, file, secret,
readOnly = False, reOpen = False):
- command = 'x-blockdev-reopen' if reOpen else 'blockdev-add'
+ command = 'blockdev-reopen' if reOpen else 'blockdev-add'
opts = {
'driver': iotests.imgfmt,
diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
index a5b0d91224..fae72211b1 100755
--- a/tests/qemu-iotests/298
+++ b/tests/qemu-iotests/298
@@ -98,7 +98,7 @@ class TestPreallocateFilter(TestPreallocateBase):
self.check_big()
def test_reopen_opts(self):
- result = self.vm.qmp('x-blockdev-reopen', options=[{
+ result = self.vm.qmp('blockdev-reopen', options=[{
'node-name': 'disk',
'driver': iotests.imgfmt,
'file': {
diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing b/tests/qemu-iotests/tests/remove-bitmap-from-backing
index 0b07f7e836..8d48fc0f3c 100755
--- a/tests/qemu-iotests/tests/remove-bitmap-from-backing
+++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing
@@ -53,7 +53,7 @@ new_base_opts = {
}
# Don't want to bother with filtering qmp_log for reopen command
-result = vm.qmp('x-blockdev-reopen', **new_base_opts)
+result = vm.qmp('blockdev-reopen', **new_base_opts)
if result != {'return': {}}:
log('Failed to reopen: ' + str(result))
@@ -61,7 +61,7 @@ log('Remove persistent bitmap from base node reopened to RW:')
vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0')
new_base_opts['options'][0]['read-only'] = True
-result = vm.qmp('x-blockdev-reopen', **new_base_opts)
+result = vm.qmp('blockdev-reopen', **new_base_opts)
if result != {'return': {}}:
log('Failed to reopen: ' + str(result))
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v6 6/6] block: Make blockdev-reopen stable API
2021-07-08 11:47 ` [PATCH v6 6/6] block: Make blockdev-reopen stable API Kevin Wolf
@ 2021-07-08 14:37 ` Eric Blake
0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2021-07-08 14:37 UTC (permalink / raw)
To: Kevin Wolf; +Cc: vsementsov, qemu-devel, qemu-block, mreitz
On Thu, Jul 08, 2021 at 01:47:09PM +0200, Kevin Wolf wrote:
> From: Alberto Garcia <berto@igalia.com>
>
> This patch drops the 'x-' prefix from x-blockdev-reopen.
>
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> +++ b/qapi/block-core.json
> @@ -4219,7 +4219,7 @@
> { 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true }
>
> ##
> -# @x-blockdev-reopen:
> +# @blockdev-reopen:
> #
> # Reopens one or more block devices using the given set of options.
> # Any option not specified will be reset to its default value regardless
> @@ -4257,9 +4257,9 @@
> # image does not have a default backing file name as part of its
> # metadata.
> #
> -# Since: 4.0
> +# Since: 6.0
6.1 now
And yay! We are finally getting here!
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread