Re: [Qemu-block] [PATCH 1/6] qemu-io: Reset qemuio_blk permissions before each command
Am 21.09.2017 um 15:53 hat Kevin Wolf geschrieben: > Am 15.09.2017 um 12:10 hat Kevin Wolf geschrieben: > > qemu-io provides a 'reopen' command that allows switching from writable > > to read-only access. We need to make sure that we don't try to keep > > write permissions to a BlockBackend that becomes read-only, otherwise > > things are going to fail. > > > > command() already makes sure to request any additional permissions that > > a qemu-io command requires, so just resetting the permissions to values > > that are safe for read-only images is enough to fix this. > > > > As a side effect, this makes the output of qemu-iotests case 187 more > > consistent. > > > > Signed-off-by: Kevin Wolf > > This seems to break qemu-iotests 077 and 081 for raw. I'll look into > it tomorrow. The problem seems to be related to 'aio_write': We already reset the permissions while the request is still in flight and might still start new internal requests that aren't allowed any more. We would have to drain blk around resetting the permissions, but this would obviously defeat the purpose of the aio_* commands. I tried creating a separate temporary BB, but it doesn't seem to be that straightforward (still crashes during 'aio_flush'). It would also provide the wrong semantics, HMP 'qemu-io' commands are supposed to be executed on the exact BlockBackend that was given. So unless someone has an idea how to salvage this patch, I'm afraid I'll just have to drop it. The original state isn't really correct either, but it errs on the side of having more permissions than necessary, so failure is less likely. Kevin
Re: [Qemu-block] [PATCH 1/6] qemu-io: Reset qemuio_blk permissions before each command
Am 15.09.2017 um 12:10 hat Kevin Wolf geschrieben: > qemu-io provides a 'reopen' command that allows switching from writable > to read-only access. We need to make sure that we don't try to keep > write permissions to a BlockBackend that becomes read-only, otherwise > things are going to fail. > > command() already makes sure to request any additional permissions that > a qemu-io command requires, so just resetting the permissions to values > that are safe for read-only images is enough to fix this. > > As a side effect, this makes the output of qemu-iotests case 187 more > consistent. > > Signed-off-by: Kevin Wolf This seems to break qemu-iotests 077 and 081 for raw. I'll look into it tomorrow. Kevin
Re: [Qemu-block] [PATCH 1/6] qemu-io: Reset qemuio_blk permissions before each command
On Fri, 09/15 12:10, Kevin Wolf wrote:
> qemu-io provides a 'reopen' command that allows switching from writable
> to read-only access. We need to make sure that we don't try to keep
> write permissions to a BlockBackend that becomes read-only, otherwise
> things are going to fail.
>
> command() already makes sure to request any additional permissions that
> a qemu-io command requires, so just resetting the permissions to values
> that are safe for read-only images is enough to fix this.
>
> As a side effect, this makes the output of qemu-iotests case 187 more
> consistent.
>
> Signed-off-by: Kevin Wolf
> ---
> qemu-io.c | 13 +
> tests/qemu-iotests/187.out | 2 +-
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/qemu-io.c b/qemu-io.c
> index 265445ad89..9e031f0f8e 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -366,6 +366,12 @@ static void command_loop(void)
> char *input;
>
> for (i = 0; !done && i < ncmdline; i++) {
> +/* Make sure that we start each command with clean permissions and
> only
> + * add whatever the specific cmdinfo_t describes */
> +if (qemuio_blk) {
> +blk_set_perm(qemuio_blk, BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL,
> + &error_abort);
> +}
> done = qemuio_command(qemuio_blk, cmdline[i]);
> }
> if (cmdline) {
> @@ -391,6 +397,13 @@ static void command_loop(void)
> if (input == NULL) {
> break;
> }
> +
> +/* Make sure that we start each command with clean permissions and
> only
> + * add whatever the specific cmdinfo_t describes */
> +if (qemuio_blk) {
> +blk_set_perm(qemuio_blk, BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL,
> + &error_abort);
> +}
> done = qemuio_command(qemuio_blk, input);
> g_free(input);
>
> diff --git a/tests/qemu-iotests/187.out b/tests/qemu-iotests/187.out
> index 68fb944cd5..30b987f71f 100644
> --- a/tests/qemu-iotests/187.out
> +++ b/tests/qemu-iotests/187.out
> @@ -12,7 +12,7 @@ Start from read-write
>
> wrote 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -write failed: Operation not permitted
> +Block node is read-only
> wrote 65536/65536 bytes at offset 0
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> *** done
> --
> 2.13.5
>
Reviewed-by: Fam Zheng
[Qemu-block] [PATCH 1/6] qemu-io: Reset qemuio_blk permissions before each command
qemu-io provides a 'reopen' command that allows switching from writable
to read-only access. We need to make sure that we don't try to keep
write permissions to a BlockBackend that becomes read-only, otherwise
things are going to fail.
command() already makes sure to request any additional permissions that
a qemu-io command requires, so just resetting the permissions to values
that are safe for read-only images is enough to fix this.
As a side effect, this makes the output of qemu-iotests case 187 more
consistent.
Signed-off-by: Kevin Wolf
---
qemu-io.c | 13 +
tests/qemu-iotests/187.out | 2 +-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/qemu-io.c b/qemu-io.c
index 265445ad89..9e031f0f8e 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -366,6 +366,12 @@ static void command_loop(void)
char *input;
for (i = 0; !done && i < ncmdline; i++) {
+/* Make sure that we start each command with clean permissions and only
+ * add whatever the specific cmdinfo_t describes */
+if (qemuio_blk) {
+blk_set_perm(qemuio_blk, BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL,
+ &error_abort);
+}
done = qemuio_command(qemuio_blk, cmdline[i]);
}
if (cmdline) {
@@ -391,6 +397,13 @@ static void command_loop(void)
if (input == NULL) {
break;
}
+
+/* Make sure that we start each command with clean permissions and only
+ * add whatever the specific cmdinfo_t describes */
+if (qemuio_blk) {
+blk_set_perm(qemuio_blk, BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL,
+ &error_abort);
+}
done = qemuio_command(qemuio_blk, input);
g_free(input);
diff --git a/tests/qemu-iotests/187.out b/tests/qemu-iotests/187.out
index 68fb944cd5..30b987f71f 100644
--- a/tests/qemu-iotests/187.out
+++ b/tests/qemu-iotests/187.out
@@ -12,7 +12,7 @@ Start from read-write
wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-write failed: Operation not permitted
+Block node is read-only
wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
--
2.13.5
