* [PATCH v3 1/6] block: Add bdrv_reopen_queue_free()
2021-03-09 17:08 [PATCH v3 0/6] Allow changing bs->file on reopen Alberto Garcia
@ 2021-03-09 17:08 ` Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 2/6] block: Allow changing bs->file on reopen Alberto Garcia
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2021-03-09 17:08 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Alberto Garcia,
qemu-block, Max Reitz
Move the code to free a BlockReopenQueue to a separate function.
It will be used in a subsequent patch.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
include/block/block.h | 1 +
block.c | 16 ++++++++++++----
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 82271d9ccd..aa3d568fec 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -371,6 +371,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 576b145cbf..03b36cd5df 100644
--- a/block.c
+++ b/block.c
@@ -3933,6 +3933,17 @@ 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) {
+ g_free(bs_entry);
+ }
+ g_free(bs_queue);
+ }
+}
+
/*
* Reopen multiple BlockDriverStates atomically & transactionally.
*
@@ -4020,10 +4031,7 @@ abort:
}
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;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 2/6] block: Allow changing bs->file on reopen
2021-03-09 17:08 [PATCH v3 0/6] Allow changing bs->file on reopen Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 1/6] block: Add bdrv_reopen_queue_free() Alberto Garcia
@ 2021-03-09 17:08 ` Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 3/6] iotests: Test replacing files with x-blockdev-reopen Alberto Garcia
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2021-03-09 17:08 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Alberto Garcia,
qemu-block, Max Reitz
When the x-blockdev-reopen was added it allowed reconfiguring the
graph by replacing backing files, but changing the 'file' option was
forbidden. Because of this restriction some operations are not
possible, notably inserting and removing block filters.
This patch adds support for replacing the 'file' option. This is
similar to replacing the backing file and the user is likewise
responsible for the correctness of the resulting graph, otherwise this
can lead to data corruption.
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
include/block/block.h | 1 +
block.c | 121 ++++++++++++++++++++++++++---------------
tests/qemu-iotests/245 | 9 +--
3 files changed, 82 insertions(+), 49 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index aa3d568fec..fe4a220da9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -196,6 +196,7 @@ typedef struct BDRVReopenState {
bool backing_missing;
bool replace_backing_bs; /* new_backing_bs is ignored if this is false */
BlockDriverState *old_backing_bs; /* keep pointer for permissions update */
+ BlockDriverState *old_file_bs; /* keep pointer for permissions update */
uint64_t perm, shared_perm;
QDict *options;
QDict *explicit_options;
diff --git a/block.c b/block.c
index 03b36cd5df..bce5d8a69c 100644
--- a/block.c
+++ b/block.c
@@ -106,7 +106,7 @@ static void bdrv_remove_backing(BlockDriverState *bs, GSList **tran);
static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
- GSList **set_backings_tran, Error **errp);
+ GSList **tran, Error **errp);
static void bdrv_reopen_commit(BDRVReopenState *reopen_state);
static void bdrv_reopen_abort(BDRVReopenState *reopen_state);
@@ -3989,6 +3989,10 @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp)
refresh_list = bdrv_topological_dfs(refresh_list, found,
state->old_backing_bs);
}
+ if (state->old_file_bs) {
+ refresh_list = bdrv_topological_dfs(refresh_list, found,
+ state->old_file_bs);
+ }
}
ret = bdrv_list_refresh_perms(refresh_list, bs_queue, &tran, errp);
@@ -4093,65 +4097,77 @@ static bool bdrv_reopen_can_attach(BlockDriverState *parent,
*
* Return 0 on success, otherwise return < 0 and set @errp.
*/
-static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
- GSList **set_backings_tran,
- Error **errp)
+static int bdrv_reopen_parse_file_or_backing(BDRVReopenState *reopen_state,
+ bool parse_file, GSList **tran,
+ Error **errp)
{
BlockDriverState *bs = reopen_state->bs;
- BlockDriverState *overlay_bs, *below_bs, *new_backing_bs;
+ BlockDriverState *overlay_bs, *below_bs, *new_child_bs;
+ BdrvChild *child = parse_file ? bs->file : bs->backing;
QObject *value;
const char *str;
- value = qdict_get(reopen_state->options, "backing");
+ value = qdict_get(reopen_state->options, parse_file ? "file" : "backing");
if (value == NULL) {
return 0;
}
switch (qobject_type(value)) {
case QTYPE_QNULL:
- new_backing_bs = NULL;
+ assert(!parse_file); /* The 'file' option does not allow a null value */
+ new_child_bs = NULL;
break;
case QTYPE_QSTRING:
- str = qobject_get_try_str(value);
- new_backing_bs = bdrv_lookup_bs(NULL, str, errp);
- if (new_backing_bs == NULL) {
+ str = qstring_get_str(qobject_to(QString, value));
+ new_child_bs = bdrv_lookup_bs(NULL, str, errp);
+ if (new_child_bs == NULL) {
return -EINVAL;
- } else if (bdrv_recurse_has_child(new_backing_bs, bs)) {
- error_setg(errp, "Making '%s' a backing file of '%s' "
- "would create a cycle", str, bs->node_name);
+ } else if (bdrv_recurse_has_child(new_child_bs, bs)) {
+ error_setg(errp, "Making '%s' a %s of '%s' would create a cycle",
+ str, parse_file ? "file" : "backing file",
+ bs->node_name);
return -EINVAL;
}
break;
default:
- /* 'backing' does not allow any other data type */
+ /* The options QDict has been flattened, so 'backing' and 'file'
+ * do not allow any other data type here. */
g_assert_not_reached();
}
- /*
- * Check AioContext compatibility so that the bdrv_set_backing_hd() call in
- * bdrv_reopen_commit() won't fail.
- */
- if (new_backing_bs) {
- if (!bdrv_reopen_can_attach(bs, bs->backing, new_backing_bs, errp)) {
+ /* If 'file' points to the current child then there's nothing to do */
+ if (child_bs(child) == new_child_bs) {
+ return 0;
+ }
+
+ /* Check AioContext compatibility */
+ if (new_child_bs) {
+ if (!bdrv_reopen_can_attach(bs, child, new_child_bs, errp)) {
return -EINVAL;
}
}
- /*
- * Ensure that @bs can really handle backing files, because we are
- * about to give it one (or swap the existing one)
- */
- if (bs->drv->is_filter) {
- /* Filters always have a file or a backing child */
- if (!bs->backing) {
- error_setg(errp, "'%s' is a %s filter node that does not support a "
- "backing child", bs->node_name, bs->drv->format_name);
+ if (parse_file) {
+ assert(child && child->bs);
+ } else {
+ /*
+ * Ensure that @bs can really handle backing files, because we are
+ * about to give it one (or swap the existing one)
+ */
+ if (bs->drv->is_filter) {
+ /* Filters always have a file or a backing child */
+ if (!bs->backing) {
+ error_setg(errp, "'%s' is a %s filter node "
+ "that does not support a backing child",
+ bs->node_name, bs->drv->format_name);
+ return -EINVAL;
+ }
+ } else if (!bs->drv->supports_backing) {
+ error_setg(errp, "Driver '%s' of node '%s' "
+ "does not support backing files",
+ bs->drv->format_name, bs->node_name);
return -EINVAL;
}
- } else if (!bs->drv->supports_backing) {
- error_setg(errp, "Driver '%s' of node '%s' does not support backing "
- "files", bs->drv->format_name, bs->node_name);
- return -EINVAL;
}
/*
@@ -4170,13 +4186,13 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
}
/* If we want to replace the backing file we need some extra checks */
- if (new_backing_bs != bdrv_filter_or_cow_bs(overlay_bs)) {
+ if (new_child_bs != bdrv_filter_or_cow_bs(overlay_bs)) {
int ret;
/* Check for implicit nodes between bs and its backing file */
if (bs != overlay_bs) {
- error_setg(errp, "Cannot change backing link if '%s' has "
- "an implicit backing file", bs->node_name);
+ error_setg(errp, "Cannot change %s link if '%s' has an implicit "
+ "child", parse_file ? "file" : "backing", bs->node_name);
return -EPERM;
}
/*
@@ -4188,16 +4204,24 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
* with bs->drv->supports_backing == true.
*/
if (bdrv_is_backing_chain_frozen(overlay_bs,
- child_bs(overlay_bs->backing), errp))
+ bdrv_filter_or_cow_bs(overlay_bs),
+ errp))
{
return -EPERM;
}
- reopen_state->replace_backing_bs = true;
- reopen_state->old_backing_bs = bs->backing ? bs->backing->bs : NULL;
- ret = bdrv_set_backing_noperm(bs, new_backing_bs, set_backings_tran,
- errp);
- if (ret < 0) {
- return ret;
+ if (parse_file) {
+ /* Store the old file bs, we'll need to refresh its permissions */
+ reopen_state->old_file_bs = bs->file->bs;
+
+ /* And finally replace the child */
+ bdrv_replace_child(bs->file, new_child_bs, tran);
+ } else {
+ reopen_state->replace_backing_bs = true;
+ reopen_state->old_backing_bs = child_bs(bs->backing);
+ ret = bdrv_set_backing_noperm(bs, new_child_bs, tran, errp);
+ if (ret < 0) {
+ return ret;
+ }
}
}
@@ -4223,7 +4247,7 @@ static int bdrv_reopen_parse_backing(BDRVReopenState *reopen_state,
*/
static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue,
- GSList **set_backings_tran, Error **errp)
+ GSList **tran, Error **errp)
{
int ret = -1;
int old_flags;
@@ -4349,12 +4373,19 @@ static int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
* either a reference to an existing node (using its node name)
* or NULL to simply detach the current backing file.
*/
- ret = bdrv_reopen_parse_backing(reopen_state, set_backings_tran, errp);
+ ret = bdrv_reopen_parse_file_or_backing(reopen_state, false, tran, errp);
if (ret < 0) {
goto error;
}
qdict_del(reopen_state->options, "backing");
+ /* Allow changing the 'file' option. In this case NULL is not allowed */
+ ret = bdrv_reopen_parse_file_or_backing(reopen_state, true, tran, errp);
+ if (ret < 0) {
+ goto error;
+ }
+ qdict_del(reopen_state->options, "file");
+
/* Options that are not handled are only okay if they are unchanged
* compared to the old state. It is expected that some options are only
* used for the initial open, but not reopen (e.g. filename) */
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index e60c8326d3..52c2ed7c2d 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -145,8 +145,8 @@ class TestBlockdevReopen(iotests.QMPTestCase):
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, {'file': 'not-found'}, "Cannot change the option 'file'")
- self.reopen(opts, {'file': ''}, "Cannot change the option 'file'")
+ 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")
self.reopen(opts, {'file.node-name': 'newname'}, "Cannot change the option 'node-name'")
self.reopen(opts, {'file.driver': 'host_device'}, "Cannot change the option 'driver'")
@@ -454,7 +454,8 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# More illegal operations
self.reopen(opts[2], {'backing': 'hd1'},
"Making 'hd1' a backing file of 'hd2' would create a cycle")
- self.reopen(opts[2], {'file': 'hd0-file'}, "Cannot change the option 'file'")
+ self.reopen(opts[2], {'file': 'hd0-file'},
+ "Conflicts with use by hd2 as 'file', which does not allow 'write, resize' on hd0-file")
result = self.vm.qmp('blockdev-del', conv_keys = True, node_name = 'hd2')
self.assert_qmp(result, 'error/class', 'GenericError')
@@ -964,7 +965,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# We can't remove hd1 while the commit job is ongoing
opts['backing'] = None
- self.reopen(opts, {}, "Cannot change backing link if 'hd0' has an implicit backing file")
+ self.reopen(opts, {}, "Cannot change backing link if 'hd0' has an implicit child")
# hd2 <- hd0
self.vm.run_job('commit0', auto_finalize = False, auto_dismiss = True)
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 3/6] iotests: Test replacing files with x-blockdev-reopen
2021-03-09 17:08 [PATCH v3 0/6] Allow changing bs->file on reopen Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 1/6] block: Add bdrv_reopen_queue_free() Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 2/6] block: Allow changing bs->file on reopen Alberto Garcia
@ 2021-03-09 17:08 ` Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 4/6] block: Support multiple reopening " Alberto Garcia
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2021-03-09 17:08 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Alberto Garcia,
qemu-block, Max Reitz
This patch adds new tests in which we use x-blockdev-reopen to change
bs->file
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
tests/qemu-iotests/245 | 109 ++++++++++++++++++++++++++++++++++++-
tests/qemu-iotests/245.out | 11 +++-
2 files changed, 117 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 52c2ed7c2d..1549a42e2b 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -78,7 +78,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
for line in log.split("\n"):
if line.startswith("Pattern verification failed"):
raise Exception("%s (command #%d)" % (line, found))
- if re.match("read .*/.* bytes at offset", line):
+ if re.match("(read|wrote) .*/.* bytes at offset", line):
found += 1
self.assertEqual(found, self.total_io_cmds,
"Expected output of %d qemu-io commands, found %d" %
@@ -536,6 +536,113 @@ class TestBlockdevReopen(iotests.QMPTestCase):
result = self.vm.qmp('blockdev-del', conv_keys = True, node_name = 'bv')
self.assert_qmp(result, 'return', {})
+ # Replace the protocol layer ('file' parameter) of a disk image
+ def test_replace_file(self):
+ # Create two small raw images and add them to a running VM
+ qemu_img('create', '-f', 'raw', hd_path[0], '10k')
+ qemu_img('create', '-f', 'raw', hd_path[1], '10k')
+
+ hd0_opts = {'driver': 'file', 'node-name': 'hd0-file', 'filename': hd_path[0] }
+ hd1_opts = {'driver': 'file', 'node-name': 'hd1-file', 'filename': hd_path[1] }
+
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **hd0_opts)
+ self.assert_qmp(result, 'return', {})
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **hd1_opts)
+ self.assert_qmp(result, 'return', {})
+
+ # Add a raw format layer that uses hd0-file as its protocol layer
+ opts = {'driver': 'raw', 'node-name': 'hd', 'file': 'hd0-file'}
+
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
+ self.assert_qmp(result, 'return', {})
+
+ # Fill the image with data
+ self.run_qemu_io("hd", "read -P 0 0 10k")
+ self.run_qemu_io("hd", "write -P 0xa0 0 10k")
+
+ # Replace hd0-file with hd1-file and fill it with (different) data
+ self.reopen(opts, {'file': 'hd1-file'})
+ self.run_qemu_io("hd", "read -P 0 0 10k")
+ self.run_qemu_io("hd", "write -P 0xa1 0 10k")
+
+ # Use hd0-file again and check that it contains the expected data
+ self.reopen(opts, {'file': 'hd0-file'})
+ self.run_qemu_io("hd", "read -P 0xa0 0 10k")
+
+ # And finally do the same with hd1-file
+ self.reopen(opts, {'file': 'hd1-file'})
+ self.run_qemu_io("hd", "read -P 0xa1 0 10k")
+
+ # Insert (and remove) a throttle filter
+ def test_insert_throttle_filter(self):
+ # Add an image to the VM
+ hd0_opts = hd_opts(0)
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **hd0_opts)
+ self.assert_qmp(result, 'return', {})
+
+ # Create a throttle-group object
+ opts = { 'qom-type': 'throttle-group', 'id': 'group0',
+ 'limits': { 'iops-total': 1000 } }
+ result = self.vm.qmp('object-add', conv_keys = False, **opts)
+ self.assert_qmp(result, 'return', {})
+
+ # Add a throttle filter with the group that we just created.
+ # The filter is not used by anyone yet
+ opts = { 'driver': 'throttle', 'node-name': 'throttle0',
+ 'throttle-group': 'group0', 'file': 'hd0-file' }
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
+ self.assert_qmp(result, 'return', {})
+
+ # Insert the throttle filter between hd0 and hd0-file
+ self.reopen(hd0_opts, {'file': 'throttle0'})
+
+ # Remove the throttle filter from hd0
+ self.reopen(hd0_opts, {'file': 'hd0-file'})
+
+ # Insert (and remove) a compress filter
+ def test_insert_compress_filter(self):
+ # Add an image to the VM: hd (raw) -> hd0 (qcow2) -> hd0-file (file)
+ opts = {'driver': 'raw', 'node-name': 'hd', 'file': hd_opts(0)}
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **opts)
+ self.assert_qmp(result, 'return', {})
+
+ # Add a 'compress' filter
+ filter_opts = {'driver': 'compress',
+ 'node-name': 'compress0',
+ 'file': 'hd0'}
+ result = self.vm.qmp('blockdev-add', conv_keys = False, **filter_opts)
+ self.assert_qmp(result, 'return', {})
+
+ # Unmap the beginning of the image (we cannot write compressed
+ # data to an allocated cluster)
+ self.run_qemu_io("hd", "write -z -u 0 128k")
+
+ # Write data to the first cluster
+ self.run_qemu_io("hd", "write -P 0xa0 0 64k")
+
+ # Insert the filter then write to the second cluster
+ # hd -> compress0 -> hd0 -> hd0-file
+ self.reopen(opts, {'file': 'compress0'})
+ self.run_qemu_io("hd", "write -P 0xa1 64k 64k")
+
+ # Remove the filter then write to the third cluster
+ # hd -> hd0 -> hd0-file
+ self.reopen(opts, {'file': 'hd0'})
+ self.run_qemu_io("hd", "write -P 0xa2 128k 64k")
+
+ # Verify the data that we just wrote
+ self.run_qemu_io("hd", "read -P 0xa0 0 64k")
+ self.run_qemu_io("hd", "read -P 0xa1 64k 64k")
+ self.run_qemu_io("hd", "read -P 0xa2 128k 64k")
+
+ self.vm.shutdown()
+
+ # Check the first byte of the first three L2 entries and verify that
+ # the second one is compressed (0x40) while the others are not (0x80)
+ iotests.qemu_io_log('-f', 'raw', '-c', 'read -P 0x80 0x40000 1',
+ '-c', 'read -P 0x40 0x40008 1',
+ '-c', 'read -P 0x80 0x40010 1', hd_path[0])
+
# 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 4b33dcaf5c..6ea1b2798f 100644
--- a/tests/qemu-iotests/245.out
+++ b/tests/qemu-iotests/245.out
@@ -10,8 +10,15 @@
{"return": {}}
{"data": {"id": "stream0", "type": "stream"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
{"data": {"device": "stream0", "len": 3145728, "offset": 3145728, "speed": 0, "type": "stream"}, "event": "BLOCK_JOB_COMPLETED", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
-.....................
+read 1/1 bytes at offset 262144
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1/1 bytes at offset 262152
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1/1 bytes at offset 262160
+1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+........................
----------------------------------------------------------------------
-Ran 21 tests
+Ran 24 tests
OK
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 4/6] block: Support multiple reopening with x-blockdev-reopen
2021-03-09 17:08 [PATCH v3 0/6] Allow changing bs->file on reopen Alberto Garcia
` (2 preceding siblings ...)
2021-03-09 17:08 ` [PATCH v3 3/6] iotests: Test replacing files with x-blockdev-reopen Alberto Garcia
@ 2021-03-09 17:08 ` Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 5/6] iotests: Test reopening multiple devices at the same time Alberto Garcia
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2021-03-09 17:08 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Alberto Garcia,
qemu-block, Max Reitz
Signed-off-by: Alberto Garcia <berto@igalia.com>
---
qapi/block-core.json | 18 ++++----
blockdev.c | 85 +++++++++++++++++++++-----------------
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 +-
9 files changed, 91 insertions(+), 69 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c0e7c23331..0363c901b7 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4140,13 +4140,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
@@ -4166,8 +4168,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
@@ -4177,7 +4179,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 098a05709d..6b688c0f73 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3528,38 +3528,16 @@ fail:
visit_free(v);
}
-void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)
+void qmp_x_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp)
{
- BlockDriverState *bs;
- QObject *obj;
- Visitor *v = qobject_output_visitor_new(&obj);
- BlockReopenQueue *queue;
- QDict *qdict;
-
- /* 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, "Cannot find node named '%s'", options->node_name);
- goto fail;
- }
-
- /* 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_flatten(qdict);
-
- /* Perform the reopen operation */
+ BlockReopenQueue *queue = NULL;
+ GSList *aio_ctxs = NULL;
+ GSList *visitors = NULL;
+ GSList *drained = NULL;
BdrvNextIterator it;
- GSList *aio_ctxs = NULL, *ctx;
BlockDriverState *it_bs;
+ /* Acquire all AIO contexts */
for (it_bs = bdrv_first(&it); it_bs; it_bs = bdrv_next(&it)) {
AioContext *aio_context = bdrv_get_aio_context(it_bs);
@@ -3569,19 +3547,50 @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp)
}
}
- bdrv_subtree_drained_begin(bs);
- queue = bdrv_reopen_queue(NULL, bs, qdict, false);
+ /* 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;
+ QDict *qdict;
+ Visitor *v;
+ BlockDriverState *bs;
+ QObject *obj;
+
+ /* 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, "Cannot find node named '%s'", options->node_name);
+ goto fail;
+ }
+
+ v = qobject_output_visitor_new(&obj);
+ visitors = g_slist_prepend(visitors, 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_flatten(qdict);
+
+ bdrv_subtree_drained_begin(bs);
+ queue = bdrv_reopen_queue(queue, bs, qdict, false);
+ drained = g_slist_prepend(drained, bs);
+ }
+
+ /* Perform the reopen operation */
bdrv_reopen_multiple(queue, errp);
- bdrv_subtree_drained_end(bs);
-
- for (ctx = aio_ctxs; ctx != NULL; ctx = ctx->next) {
- AioContext *aio_context = ctx->data;
- aio_context_release(aio_context);
- }
- g_slist_free(aio_ctxs);
+ queue = NULL;
fail:
- visit_free(v);
+ bdrv_reopen_queue_free(queue);
+ g_slist_free_full(drained, (GDestroyNotify) bdrv_subtree_drained_end);
+ g_slist_free_full(aio_ctxs, (GDestroyNotify) aio_context_release);
+ g_slist_free_full(visitors, (GDestroyNotify) visit_free);
}
void qmp_blockdev_del(const char *node_name, Error **errp)
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index 988f986144..5271f9541f 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -260,9 +260,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 fb56a769b4..32db5086e1 100755
--- a/tests/qemu-iotests/165
+++ b/tests/qemu-iotests/165
@@ -136,7 +136,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': {
@@ -144,7 +144,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 1549a42e2b..53281228bc 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -84,8 +84,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)
@@ -100,12 +110,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
@@ -141,10 +146,10 @@ class TestBlockdevReopen(iotests.QMPTestCase):
# We cannot change any of these
self.reopen(opts, {'node-name': 'not-found'}, "Cannot find node named 'not-found'")
self.reopen(opts, {'node-name': ''}, "Cannot find node named ''")
- 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")
@@ -153,7 +158,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 18ba03467e..2b43853183 100755
--- a/tests/qemu-iotests/248
+++ b/tests/qemu-iotests/248
@@ -62,7 +62,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 fb7dec88aa..cfa5122d6b 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -119,8 +119,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,
@@ -130,7 +129,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..4efdb35b91 100644
--- 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')
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 5/6] iotests: Test reopening multiple devices at the same time
2021-03-09 17:08 [PATCH v3 0/6] Allow changing bs->file on reopen Alberto Garcia
` (3 preceding siblings ...)
2021-03-09 17:08 ` [PATCH v3 4/6] block: Support multiple reopening " Alberto Garcia
@ 2021-03-09 17:08 ` Alberto Garcia
2021-03-09 17:08 ` [PATCH v3 6/6] block: Make blockdev-reopen stable API Alberto Garcia
2021-03-15 15:19 ` [PATCH v3 0/6] Allow changing bs->file on reopen Vladimir Sementsov-Ogievskiy
6 siblings, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2021-03-09 17:08 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Alberto Garcia,
qemu-block, Max Reitz
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>
---
tests/qemu-iotests/245 | 41 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/245.out | 4 ++--
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
index 53281228bc..fb11eb146d 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -648,6 +648,47 @@ 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'},
+ "Conflicts with use by hd0 as 'file', which does not allow 'write, resize' on hd2-file")
+ self.reopen(opts2, {'file': 'hd0-file'},
+ "Conflicts with use by hd2 as 'file', which does not allow 'write, resize' on hd0-file")
+
+ # 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 6ea1b2798f..c4230b51d1 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.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 6/6] block: Make blockdev-reopen stable API
2021-03-09 17:08 [PATCH v3 0/6] Allow changing bs->file on reopen Alberto Garcia
` (4 preceding siblings ...)
2021-03-09 17:08 ` [PATCH v3 5/6] iotests: Test reopening multiple devices at the same time Alberto Garcia
@ 2021-03-09 17:08 ` Alberto Garcia
2021-03-15 15:19 ` [PATCH v3 0/6] Allow changing bs->file on reopen Vladimir Sementsov-Ogievskiy
6 siblings, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2021-03-09 17:08 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, Alberto Garcia,
qemu-block, Max Reitz
This patch drops the 'x-' prefix from x-blockdev-reopen.
Signed-off-by: Alberto Garcia <berto@igalia.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 +-
9 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0363c901b7..c9ef9fdebb 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -4138,7 +4138,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
@@ -4176,9 +4176,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 6b688c0f73..ccc7978665 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3528,7 +3528,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 *aio_ctxs = NULL;
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index 5271f9541f..eb9b3c68ac 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -260,7 +260,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 32db5086e1..d76d5036f1 100755
--- a/tests/qemu-iotests/165
+++ b/tests/qemu-iotests/165
@@ -136,7 +136,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 fb11eb146d..57ff4d846f 100755
--- a/tests/qemu-iotests/245
+++ b/tests/qemu-iotests/245
@@ -1,6 +1,6 @@
#!/usr/bin/env python3
#
-# 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>
@@ -84,16 +84,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):
@@ -160,7 +160,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 2b43853183..0da519d79f 100755
--- a/tests/qemu-iotests/248
+++ b/tests/qemu-iotests/248
@@ -61,7 +61,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 cfa5122d6b..19c4cb3861 100755
--- a/tests/qemu-iotests/296
+++ b/tests/qemu-iotests/296
@@ -117,7 +117,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 4efdb35b91..b4d8bd9b55 100644
--- 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': {
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 0/6] Allow changing bs->file on reopen
2021-03-09 17:08 [PATCH v3 0/6] Allow changing bs->file on reopen Alberto Garcia
` (5 preceding siblings ...)
2021-03-09 17:08 ` [PATCH v3 6/6] block: Make blockdev-reopen stable API Alberto Garcia
@ 2021-03-15 15:19 ` Vladimir Sementsov-Ogievskiy
6 siblings, 0 replies; 8+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-15 15:19 UTC (permalink / raw)
To: Alberto Garcia, qemu-devel; +Cc: qemu-block, Kevin Wolf, Max Reitz
09.03.2021 20:08, Alberto Garcia wrote:
> Based-on: <20201127144522.29991-1-vsementsov@virtuozzo.com>
>
> Hello,
>
> here's the third version of the patches that allow replacing bs->file
> using (x-)blockdev-reopen. You can read more details here:
>
> https://lists.gnu.org/archive/html/qemu-block/2021-01/msg00437.html
>
> In summary, the series does three things:
>
> - Allows replacing bs->file
> - Allows reopening multiple block devices with one single command.
> - Drops the x- prefix from the command name.
>
> This is still depending on Vladimir's "update graph permissions
> update" branch.
>
I've rebased my series and handled all comments on v2 (except for additional test). Now I have some iotests failing
(028 030 039 040 041 051 056 124 127 129 141 153 155 156 176 185 191 195 200 203 216 222 224 228 240 245 247 255 256 258 260 267 274 281 291 307 309 310 migrate-bitmaps-test)
I'll start handle them today, hope most of the failures is one stupid mistake. But anyway it all doesn't seem feasible to finish before soft freeze (tomorrow)..
>
> v3:
> - Patch 1: Move bdrv_reopen_queue_free() to a new patch
> - Patch 2: Merge bdrv_reopen_parse_backing() and bdrv_reopen_parse_file()
> - Patch 3: Add more tests
> - Patch 4: Update documentation and fix iotest 296
> - Patch 5: Minor updates to iotest 245
> - Patch 6: New patch, drop the 'x-' prefix from x-blockdev-reopen
>
> v2: https://lists.gnu.org/archive/html/qemu-block/2021-02/msg00623.html
> v1: https://lists.gnu.org/archive/html/qemu-block/2021-01/msg00437.html
>
> Output of git backport-diff against v2:
>
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>
> 001/6:[down] 'block: Add bdrv_reopen_queue_free()'
> 002/6:[0160] [FC] 'block: Allow changing bs->file on reopen'
> 003/6:[down] 'iotests: Test replacing files with x-blockdev-reopen'
> 004/6:[0042] [FC] 'block: Support multiple reopening with x-blockdev-reopen'
> 005/6:[0015] [FC] 'iotests: Test reopening multiple devices at the same time'
> 006/6:[down] 'block: Make blockdev-reopen stable API'
>
> Alberto Garcia (6):
> block: Add bdrv_reopen_queue_free()
> block: Allow changing bs->file on reopen
> iotests: Test replacing files with x-blockdev-reopen
> block: Support multiple reopening with x-blockdev-reopen
> iotests: Test reopening multiple devices at the same time
> block: Make blockdev-reopen stable API
>
> qapi/block-core.json | 24 ++---
> include/block/block.h | 2 +
> block.c | 137 ++++++++++++++++----------
> blockdev.c | 85 +++++++++--------
> tests/qemu-iotests/155 | 9 +-
> tests/qemu-iotests/165 | 4 +-
> tests/qemu-iotests/245 | 190 +++++++++++++++++++++++++++++++++----
> tests/qemu-iotests/245.out | 11 ++-
> tests/qemu-iotests/248 | 4 +-
> tests/qemu-iotests/248.out | 2 +-
> tests/qemu-iotests/296 | 11 ++-
> tests/qemu-iotests/298 | 4 +-
> 12 files changed, 351 insertions(+), 132 deletions(-)
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 8+ messages in thread