* [Qemu-devel] [PATCH v2 0/2] block: Fix permission during reopen
@ 2018-03-13 11:58 Fam Zheng
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 1/2] block: Fix flags in reopen queue Fam Zheng
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 2/2] iotests: Add regression test for commit base locking Fam Zheng
0 siblings, 2 replies; 5+ messages in thread
From: Fam Zheng @ 2018-03-13 11:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Max Reitz, qemu-block
v2: Use update_flags_from_options. [Kevin]
We write open the whole backing chain during reopen. It is not necessary and
will cause image locking problems if the backing image is shared.
Fam Zheng (2):
block: Fix flags in reopen queue
iotests: Add regression test for commit base locking
block.c | 7 +++++++
tests/qemu-iotests/153 | 8 ++++++++
tests/qemu-iotests/153.out | 4 ++++
3 files changed, 19 insertions(+)
--
2.14.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 1/2] block: Fix flags in reopen queue
2018-03-13 11:58 [Qemu-devel] [PATCH v2 0/2] block: Fix permission during reopen Fam Zheng
@ 2018-03-13 11:58 ` Fam Zheng
2018-03-13 12:59 ` Max Reitz
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 2/2] iotests: Add regression test for commit base locking Fam Zheng
1 sibling, 1 reply; 5+ messages in thread
From: Fam Zheng @ 2018-03-13 11:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Max Reitz, qemu-block
Reopen flags are not synchronized according to the
bdrv_reopen_queue_child precedence until bdrv_reopen_prepare. It is a
bit too late: we already check the consistency in bdrv_check_perm before
that.
This fixes the bug that when bdrv_reopen a RO node as RW, the flags for
backing child are wrong. Before, we could recurse with flags.rw=1; now,
role->inherit_options + update_flags_from_options will make sure to
clear the bit when necessary. Note that this will not clear an
explicitly set bit, as in the case of parallel block jobs (e.g.
test_stream_parallel in 030), because the explicit options include
'read-only=false' (for an intermediate node used by a different job).
Signed-off-by: Fam Zheng <famz@redhat.com>
---
block.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block.c b/block.c
index 75a9fd49de..a121d2ebcc 100644
--- a/block.c
+++ b/block.c
@@ -2883,8 +2883,15 @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
/* Inherit from parent node */
if (parent_options) {
+ QemuOpts *opts;
+ QDict *options_copy;
assert(!flags);
role->inherit_options(&flags, options, parent_flags, parent_options);
+ options_copy = qdict_clone_shallow(options);
+ opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort);
+ qemu_opts_absorb_qdict(opts, options_copy, NULL);
+ update_flags_from_options(&flags, opts);
+ qemu_opts_del(opts);
}
/* Old values are used for options that aren't set yet */
--
2.14.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v2 2/2] iotests: Add regression test for commit base locking
2018-03-13 11:58 [Qemu-devel] [PATCH v2 0/2] block: Fix permission during reopen Fam Zheng
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 1/2] block: Fix flags in reopen queue Fam Zheng
@ 2018-03-13 11:58 ` Fam Zheng
2018-03-13 12:59 ` Max Reitz
1 sibling, 1 reply; 5+ messages in thread
From: Fam Zheng @ 2018-03-13 11:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Max Reitz, qemu-block
Signed-off-by: Fam Zheng <famz@redhat.com>
---
tests/qemu-iotests/153 | 8 ++++++++
tests/qemu-iotests/153.out | 4 ++++
2 files changed, 12 insertions(+)
diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
index adfd02695b..a7875e6899 100755
--- a/tests/qemu-iotests/153
+++ b/tests/qemu-iotests/153
@@ -178,6 +178,14 @@ rm -f "${TEST_IMG}.lnk" &>/dev/null
ln -s ${TEST_IMG} "${TEST_IMG}.lnk" || echo "Failed to create link"
_run_qemu_with_images "${TEST_IMG}.lnk" "${TEST_IMG}"
+echo
+echo "== Active commit to intermediate layer should work when base in use =="
+_launch_qemu -drive format=$IMGFMT,file="${TEST_IMG}.a",id=drive0 \
+ -device virtio-blk,drive=drive0
+_run_cmd $QEMU_IMG commit -b "${TEST_IMG}.b" "${TEST_IMG}.c"
+
+_cleanup_qemu
+
_launch_qemu
_send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out
index 34309cfb20..28f8250dd2 100644
--- a/tests/qemu-iotests/153.out
+++ b/tests/qemu-iotests/153.out
@@ -372,6 +372,10 @@ Is another process using the image?
== Symbolic link ==
QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2: Failed to get "write" lock
Is another process using the image?
+
+== Active commit to intermediate layer should work when base in use ==
+
+_qemu_img_wrapper commit -b TEST_DIR/t.qcow2.b TEST_DIR/t.qcow2.c
{"return": {}}
Adding drive
--
2.14.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/2] iotests: Add regression test for commit base locking
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 2/2] iotests: Add regression test for commit base locking Fam Zheng
@ 2018-03-13 12:59 ` Max Reitz
0 siblings, 0 replies; 5+ messages in thread
From: Max Reitz @ 2018-03-13 12:59 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Kevin Wolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]
On 2018-03-13 12:58, Fam Zheng wrote:
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> tests/qemu-iotests/153 | 8 ++++++++
> tests/qemu-iotests/153.out | 4 ++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
> index adfd02695b..a7875e6899 100755
> --- a/tests/qemu-iotests/153
> +++ b/tests/qemu-iotests/153
> @@ -178,6 +178,14 @@ rm -f "${TEST_IMG}.lnk" &>/dev/null
> ln -s ${TEST_IMG} "${TEST_IMG}.lnk" || echo "Failed to create link"
> _run_qemu_with_images "${TEST_IMG}.lnk" "${TEST_IMG}"
>
> +echo
> +echo "== Active commit to intermediate layer should work when base in use =="
> +_launch_qemu -drive format=$IMGFMT,file="${TEST_IMG}.a",id=drive0 \
> + -device virtio-blk,drive=drive0
> +_run_cmd $QEMU_IMG commit -b "${TEST_IMG}.b" "${TEST_IMG}.c"
> +
> +_cleanup_qemu
> +
> _launch_qemu
>
> _send_qemu_cmd $QEMU_HANDLE \
> diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out
> index 34309cfb20..28f8250dd2 100644
> --- a/tests/qemu-iotests/153.out
> +++ b/tests/qemu-iotests/153.out
> @@ -372,6 +372,10 @@ Is another process using the image?
> == Symbolic link ==
> QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2: Failed to get "write" lock
> Is another process using the image?
> +
> +== Active commit to intermediate layer should work when base in use ==
> +
> +_qemu_img_wrapper commit -b TEST_DIR/t.qcow2.b TEST_DIR/t.qcow2.c
> {"return": {}}
> Adding drive
Hmmm... I don't know, but this just passes on my machine without your
previous patch.
[Two minutes later]
Now I do know why, qemu simply isn't properly started at the time
$QEMU_IMG commit runs (see also 6bfc907deed83af7). Therefore, no error
here.
So if I just add a
_send_qemu_cmd $QEMU_HANDLE \
"{ 'execute': 'qmp_capabilities' }" \
'return'
after the _launch_qemu, this is what I get:
QEMU_PROG: -device virtio-blk,drive=drive0: Drive 'drive0' is already in
use because it has been automatically connected to another device (did
you need 'if=none' in the drive options?)
With if=none (or just -blockdev instead of -drive), I get the error
message I was hoping for.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/2] block: Fix flags in reopen queue
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 1/2] block: Fix flags in reopen queue Fam Zheng
@ 2018-03-13 12:59 ` Max Reitz
0 siblings, 0 replies; 5+ messages in thread
From: Max Reitz @ 2018-03-13 12:59 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Kevin Wolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 918 bytes --]
On 2018-03-13 12:58, Fam Zheng wrote:
> Reopen flags are not synchronized according to the
> bdrv_reopen_queue_child precedence until bdrv_reopen_prepare. It is a
> bit too late: we already check the consistency in bdrv_check_perm before
> that.
>
> This fixes the bug that when bdrv_reopen a RO node as RW, the flags for
> backing child are wrong. Before, we could recurse with flags.rw=1; now,
> role->inherit_options + update_flags_from_options will make sure to
> clear the bit when necessary. Note that this will not clear an
> explicitly set bit, as in the case of parallel block jobs (e.g.
> test_stream_parallel in 030), because the explicit options include
> 'read-only=false' (for an intermediate node used by a different job).
>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> block.c | 7 +++++++
> 1 file changed, 7 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-13 12:59 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 11:58 [Qemu-devel] [PATCH v2 0/2] block: Fix permission during reopen Fam Zheng
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 1/2] block: Fix flags in reopen queue Fam Zheng
2018-03-13 12:59 ` Max Reitz
2018-03-13 11:58 ` [Qemu-devel] [PATCH v2 2/2] iotests: Add regression test for commit base locking Fam Zheng
2018-03-13 12:59 ` Max Reitz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.