* [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
@ 2014-05-06 10:19 Kevin Wolf
2014-05-06 11:10 ` Jan Kiszka
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Kevin Wolf @ 2014-05-06 10:19 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jan.kiszka, stefanha, mreitz
The immediately visible effect of this patch is that it fixes committing
a temporary snapshot to its backing file. Previously, it would fail with
a "permission denied" error because bdrv_inherited_flags() forced the
backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
The bigger problem this releaved is that the original open flags must
actually only be applied to the temporary snapshot, and the original
image file must be treated as a backing file of the temporary snapshot
and get the right flags for that.
Reported-by: Jan Kiszka <jan.kiszka@web.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 34 +++++++++++++++++++---------------
include/block/block.h | 2 +-
tests/qemu-iotests/051 | 4 ++++
tests/qemu-iotests/051.out | 10 ++++++++++
4 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/block.c b/block.c
index b749d31..c90c71a 100644
--- a/block.c
+++ b/block.c
@@ -775,6 +775,16 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
}
/*
+ * Returns the flags that a temporary snapshot should get, based on the
+ * originally requested flags (the originally requested image will have flags
+ * like a backing file)
+ */
+static int bdrv_temp_snapshot_flags(int flags)
+{
+ return (flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY;
+}
+
+/*
* Returns the flags that bs->file should get, based on the given flags for
* the parent BDS
*/
@@ -787,11 +797,6 @@ static int bdrv_inherited_flags(int flags)
* so we can enable both unconditionally on lower layers. */
flags |= BDRV_O_CACHE_WB | BDRV_O_UNMAP;
- /* The backing file of a temporary snapshot is read-only */
- if (flags & BDRV_O_SNAPSHOT) {
- flags &= ~BDRV_O_RDWR;
- }
-
/* Clear flags that only apply to the top layer */
flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
@@ -817,11 +822,6 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
{
int open_flags = flags | BDRV_O_CACHE_WB;
- /* The backing file of a temporary snapshot is read-only */
- if (flags & BDRV_O_SNAPSHOT) {
- open_flags &= ~BDRV_O_RDWR;
- }
-
/*
* Clear flags that are internal to the block layer before opening the
* image.
@@ -1206,7 +1206,7 @@ done:
return ret;
}
-void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
+void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
{
/* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
char *tmp_filename = g_malloc0(PATH_MAX + 1);
@@ -1262,8 +1262,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
bs_snapshot = bdrv_new("", &error_abort);
ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
- (bs->open_flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY,
- bdrv_qcow2, &local_err);
+ flags, bdrv_qcow2, &local_err);
if (ret < 0) {
error_propagate(errp, local_err);
goto out;
@@ -1298,6 +1297,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
BlockDriverState *file = NULL, *bs;
const char *drvname;
Error *local_err = NULL;
+ int snapshot_flags = 0;
assert(pbs);
@@ -1358,6 +1358,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
if (flags & BDRV_O_RDWR) {
flags |= BDRV_O_ALLOW_RDWR;
}
+ if (flags & BDRV_O_SNAPSHOT) {
+ snapshot_flags = bdrv_temp_snapshot_flags(flags);
+ flags = bdrv_backing_flags(flags);
+ }
assert(file == NULL);
ret = bdrv_open_image(&file, filename, options, "file",
@@ -1417,8 +1421,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
/* For snapshot=on, create a temporary qcow2 overlay. bs points to the
* temporary snapshot afterwards. */
- if (flags & BDRV_O_SNAPSHOT) {
- bdrv_append_temp_snapshot(bs, &local_err);
+ if (snapshot_flags) {
+ bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
if (local_err) {
error_propagate(errp, local_err);
goto close_and_fail;
diff --git a/include/block/block.h b/include/block/block.h
index 467fb2b..2fda81c 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -191,7 +191,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
QDict *options, const char *bdref_key, int flags,
bool allow_none, Error **errp);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
-void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp);
+void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp);
int bdrv_open(BlockDriverState **pbs, const char *filename,
const char *reference, QDict *options, int flags,
BlockDriver *drv, Error **errp);
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 073dc7a..c4af131 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -233,6 +233,10 @@ echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",
$QEMU_IO -c "read -P 0x22 0 4k" "$TEST_IMG" | _filter_qemu_io
+echo -e 'qemu-io ide0-hd0 "write -P 0x33 0 4k"\ncommit ide0-hd0' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io
+
+$QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 01b0384..31e329e 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -358,4 +358,14 @@ wrote 4096/4096 bytes at offset 0
read 4096/4096 bytes at offset 0
4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Testing: -drive file=TEST_DIR/t.qcow2,snapshot=on
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqe^[[K^[[D^[[Dqem^[[K^[[D^[[D^[[Dqemu^[[K^[[D^[[D^[[D^[[Dqemu-^[[K^[[D^[[D^[[D^[[D^[[Dqemu-i^[[K^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io i^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io id^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-h^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "w^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "wr^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "wri^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D!
^[[D^[[Dqemu-io ide0-hd0 "writ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x3^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "w!
rite -P 0x33^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[
[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4k^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4k"^[[K
+wrote 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+(qemu) c^[[K^[[Dco^[[K^[[D^[[Dcom^[[K^[[D^[[D^[[Dcomm^[[K^[[D^[[D^[[D^[[Dcommi^[[K^[[D^[[D^[[D^[[D^[[Dcommit^[[K^[[D^[[D^[[D^[[D^[[D^[[Dcommit ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit i^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit id^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-h^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-hd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-hd0^[[K
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+read 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
--
1.8.3.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
2014-05-06 10:19 [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT Kevin Wolf
@ 2014-05-06 11:10 ` Jan Kiszka
2014-05-06 21:03 ` Max Reitz
2014-05-06 21:01 ` Max Reitz
2014-05-07 8:31 ` Stefan Hajnoczi
2 siblings, 1 reply; 7+ messages in thread
From: Jan Kiszka @ 2014-05-06 11:10 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: stefanha, mreitz
[-- Attachment #1: Type: text/plain, Size: 10133 bytes --]
On 2014-05-06 12:19, Kevin Wolf wrote:
> The immediately visible effect of this patch is that it fixes committing
> a temporary snapshot to its backing file. Previously, it would fail with
> a "permission denied" error because bdrv_inherited_flags() forced the
> backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
>
> The bigger problem this releaved is that the original open flags must
> actually only be applied to the temporary snapshot, and the original
> image file must be treated as a backing file of the temporary snapshot
> and get the right flags for that.
>
> Reported-by: Jan Kiszka <jan.kiszka@web.de>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 34 +++++++++++++++++++---------------
> include/block/block.h | 2 +-
> tests/qemu-iotests/051 | 4 ++++
> tests/qemu-iotests/051.out | 10 ++++++++++
> 4 files changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/block.c b/block.c
> index b749d31..c90c71a 100644
> --- a/block.c
> +++ b/block.c
> @@ -775,6 +775,16 @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
> }
>
> /*
> + * Returns the flags that a temporary snapshot should get, based on the
> + * originally requested flags (the originally requested image will have flags
> + * like a backing file)
> + */
> +static int bdrv_temp_snapshot_flags(int flags)
> +{
> + return (flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY;
> +}
> +
> +/*
> * Returns the flags that bs->file should get, based on the given flags for
> * the parent BDS
> */
> @@ -787,11 +797,6 @@ static int bdrv_inherited_flags(int flags)
> * so we can enable both unconditionally on lower layers. */
> flags |= BDRV_O_CACHE_WB | BDRV_O_UNMAP;
>
> - /* The backing file of a temporary snapshot is read-only */
> - if (flags & BDRV_O_SNAPSHOT) {
> - flags &= ~BDRV_O_RDWR;
> - }
> -
> /* Clear flags that only apply to the top layer */
> flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_COPY_ON_READ);
>
> @@ -817,11 +822,6 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)
> {
> int open_flags = flags | BDRV_O_CACHE_WB;
>
> - /* The backing file of a temporary snapshot is read-only */
> - if (flags & BDRV_O_SNAPSHOT) {
> - open_flags &= ~BDRV_O_RDWR;
> - }
> -
> /*
> * Clear flags that are internal to the block layer before opening the
> * image.
> @@ -1206,7 +1206,7 @@ done:
> return ret;
> }
>
> -void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
> +void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
> {
> /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> char *tmp_filename = g_malloc0(PATH_MAX + 1);
> @@ -1262,8 +1262,7 @@ void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
> bs_snapshot = bdrv_new("", &error_abort);
>
> ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
> - (bs->open_flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY,
> - bdrv_qcow2, &local_err);
> + flags, bdrv_qcow2, &local_err);
> if (ret < 0) {
> error_propagate(errp, local_err);
> goto out;
> @@ -1298,6 +1297,7 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
> BlockDriverState *file = NULL, *bs;
> const char *drvname;
> Error *local_err = NULL;
> + int snapshot_flags = 0;
>
> assert(pbs);
>
> @@ -1358,6 +1358,10 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
> if (flags & BDRV_O_RDWR) {
> flags |= BDRV_O_ALLOW_RDWR;
> }
> + if (flags & BDRV_O_SNAPSHOT) {
> + snapshot_flags = bdrv_temp_snapshot_flags(flags);
> + flags = bdrv_backing_flags(flags);
> + }
>
> assert(file == NULL);
> ret = bdrv_open_image(&file, filename, options, "file",
> @@ -1417,8 +1421,8 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>
> /* For snapshot=on, create a temporary qcow2 overlay. bs points to the
> * temporary snapshot afterwards. */
> - if (flags & BDRV_O_SNAPSHOT) {
> - bdrv_append_temp_snapshot(bs, &local_err);
> + if (snapshot_flags) {
> + bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> goto close_and_fail;
> diff --git a/include/block/block.h b/include/block/block.h
> index 467fb2b..2fda81c 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -191,7 +191,7 @@ int bdrv_open_image(BlockDriverState **pbs, const char *filename,
> QDict *options, const char *bdref_key, int flags,
> bool allow_none, Error **errp);
> int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp);
> -void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp);
> +void bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp);
> int bdrv_open(BlockDriverState **pbs, const char *filename,
> const char *reference, QDict *options, int flags,
> BlockDriver *drv, Error **errp);
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 073dc7a..c4af131 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -233,6 +233,10 @@ echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",
>
> $QEMU_IO -c "read -P 0x22 0 4k" "$TEST_IMG" | _filter_qemu_io
>
> +echo -e 'qemu-io ide0-hd0 "write -P 0x33 0 4k"\ncommit ide0-hd0' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io
> +
> +$QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io
> +
> # success, all done
> echo "*** done"
> rm -f $seq.full
> diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
> index 01b0384..31e329e 100644
> --- a/tests/qemu-iotests/051.out
> +++ b/tests/qemu-iotests/051.out
> @@ -358,4 +358,14 @@ wrote 4096/4096 bytes at offset 0
>
> read 4096/4096 bytes at offset 0
> 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Testing: -drive file=TEST_DIR/t.qcow2,snapshot=on
> +QEMU X.Y.Z monitor - type 'help' for more information
> +(qemu) q^[[K^[[Dqe^[[K^[[D^[[Dqem^[[K^[[D^[[D^[[Dqemu^[[K^[[D^[[D^[[D^[[Dqemu-^[[K^[[D^[[D^[[D^[[D^[[Dqemu-i^[[K^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io i^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io id^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-h^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "w^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "wr^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "wri^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D!
> ^[[D^[[Dqemu-io ide0-hd0 "writ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x3^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "w!
> rite -P 0x33^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[
> [D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4k^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4k"^[[K
> +wrote 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +(qemu) c^[[K^[[Dco^[[K^[[D^[[Dcom^[[K^[[D^[[D^[[Dcomm^[[K^[[D^[[D^[[D^[[Dcommi^[[K^[[D^[[D^[[D^[[D^[[Dcommit^[[K^[[D^[[D^[[D^[[D^[[D^[[Dcommit ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit i^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit id^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-h^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-hd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-hd0^[[K
> +(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
> +
> +read 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> *** done
>
Works fine here!
(For unknown reason, applying the iotest hunk didn't work for me, though.)
Thanks,
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
2014-05-06 10:19 [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT Kevin Wolf
2014-05-06 11:10 ` Jan Kiszka
@ 2014-05-06 21:01 ` Max Reitz
2014-05-07 8:31 ` Stefan Hajnoczi
2 siblings, 0 replies; 7+ messages in thread
From: Max Reitz @ 2014-05-06 21:01 UTC (permalink / raw)
To: Kevin Wolf, qemu-devel; +Cc: jan.kiszka, stefanha
On 06.05.2014 12:19, Kevin Wolf wrote:
> The immediately visible effect of this patch is that it fixes committing
> a temporary snapshot to its backing file. Previously, it would fail with
> a "permission denied" error because bdrv_inherited_flags() forced the
> backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
>
> The bigger problem this releaved is that the original open flags must
> actually only be applied to the temporary snapshot, and the original
> image file must be treated as a backing file of the temporary snapshot
> and get the right flags for that.
>
> Reported-by: Jan Kiszka <jan.kiszka@web.de>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 34 +++++++++++++++++++---------------
> include/block/block.h | 2 +-
> tests/qemu-iotests/051 | 4 ++++
> tests/qemu-iotests/051.out | 10 ++++++++++
> 4 files changed, 34 insertions(+), 16 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
2014-05-06 11:10 ` Jan Kiszka
@ 2014-05-06 21:03 ` Max Reitz
2014-05-07 8:24 ` Kevin Wolf
0 siblings, 1 reply; 7+ messages in thread
From: Max Reitz @ 2014-05-06 21:03 UTC (permalink / raw)
To: Jan Kiszka, Kevin Wolf, qemu-devel; +Cc: stefanha
On 06.05.2014 13:10, Jan Kiszka wrote:
> On 2014-05-06 12:19, Kevin Wolf wrote:
>> The immediately visible effect of this patch is that it fixes committing
>> a temporary snapshot to its backing file. Previously, it would fail with
>> a "permission denied" error because bdrv_inherited_flags() forced the
>> backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
>>
>> The bigger problem this releaved is that the original open flags must
>> actually only be applied to the temporary snapshot, and the original
>> image file must be treated as a backing file of the temporary snapshot
>> and get the right flags for that.
>>
>> Reported-by: Jan Kiszka <jan.kiszka@web.de>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>> block.c | 34 +++++++++++++++++++---------------
>> include/block/block.h | 2 +-
>> tests/qemu-iotests/051 | 4 ++++
>> tests/qemu-iotests/051.out | 10 ++++++++++
>> 4 files changed, 34 insertions(+), 16 deletions(-)
[snip]
>> diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
>> index 01b0384..31e329e 100644
>> --- a/tests/qemu-iotests/051.out
>> +++ b/tests/qemu-iotests/051.out
>> @@ -358,4 +358,14 @@ wrote 4096/4096 bytes at offset 0
>>
>> read 4096/4096 bytes at offset 0
>> 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +Testing: -drive file=TEST_DIR/t.qcow2,snapshot=on
>> +QEMU X.Y.Z monitor - type 'help' for more information
>> +(qemu) q^[[K^[[Dqe^[[K^[[D^[[Dqem^[[K^[[D^[[D^[[Dqemu^[[K^[[D^[[D^[[D^[[Dqemu-^[[K^[[D^[[D^[[D^[[D^[[Dqemu-i^[[K^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io i^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io id^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-h^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "w^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "wr^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "wri^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D!
>> ^[[D^[[Dqemu-io ide0-hd0 "writ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x3^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "w!
>> rite -P 0x33^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[
>> [D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4k^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dqemu-io ide0-hd0 "write -P 0x33 0 4k"^[[K
>> +wrote 4096/4096 bytes at offset 0
>> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> +(qemu) c^[[K^[[Dco^[[K^[[D^[[Dcom^[[K^[[D^[[D^[[Dcomm^[[K^[[D^[[D^[[D^[[Dcommi^[[K^[[D^[[D^[[D^[[D^[[Dcommit^[[K^[[D^[[D^[[D^[[D^[[D^[[Dcommit ^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit i^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit id^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-h^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-hd^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dcommit ide0-hd0^[[K
>> +(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>> +
>> +read 4096/4096 bytes at offset 0
>> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>> *** done
>>
> Works fine here!
>
> (For unknown reason, applying the iotest hunk didn't work for me, though.)
The lines are too long and therefore split in this mail, they need to be
joined manually before applying the patch.
Max
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
2014-05-06 21:03 ` Max Reitz
@ 2014-05-07 8:24 ` Kevin Wolf
2014-05-09 8:14 ` Stefan Hajnoczi
0 siblings, 1 reply; 7+ messages in thread
From: Kevin Wolf @ 2014-05-07 8:24 UTC (permalink / raw)
To: Max Reitz; +Cc: Jan Kiszka, qemu-devel, stefanha
Am 06.05.2014 um 23:03 hat Max Reitz geschrieben:
> On 06.05.2014 13:10, Jan Kiszka wrote:
> >On 2014-05-06 12:19, Kevin Wolf wrote:
> >>The immediately visible effect of this patch is that it fixes committing
> >>a temporary snapshot to its backing file. Previously, it would fail with
> >>a "permission denied" error because bdrv_inherited_flags() forced the
> >>backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
> >>
> >>The bigger problem this releaved is that the original open flags must
> >>actually only be applied to the temporary snapshot, and the original
> >>image file must be treated as a backing file of the temporary snapshot
> >>and get the right flags for that.
> >>
> >>Reported-by: Jan Kiszka <jan.kiszka@web.de>
> >>Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >>---
> >> block.c | 34 +++++++++++++++++++---------------
> >> include/block/block.h | 2 +-
> >> tests/qemu-iotests/051 | 4 ++++
> >> tests/qemu-iotests/051.out | 10 ++++++++++
> >> 4 files changed, 34 insertions(+), 16 deletions(-)
> >Works fine here!
> >
> >(For unknown reason, applying the iotest hunk didn't work for me, though.)
>
> The lines are too long and therefore split in this mail, they need
> to be joined manually before applying the patch.
Perhaps the monitor should be changed to avoid printing so many useless
control characters, then we'd hit the limit less often...
Stefan, didn't you plan to do something like this? Or was it unrelated?
Kevin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
2014-05-06 10:19 [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT Kevin Wolf
2014-05-06 11:10 ` Jan Kiszka
2014-05-06 21:01 ` Max Reitz
@ 2014-05-07 8:31 ` Stefan Hajnoczi
2 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2014-05-07 8:31 UTC (permalink / raw)
To: Kevin Wolf; +Cc: jan.kiszka, qemu-devel, stefanha, mreitz
On Tue, May 06, 2014 at 12:19:10PM +0200, Kevin Wolf wrote:
> The immediately visible effect of this patch is that it fixes committing
> a temporary snapshot to its backing file. Previously, it would fail with
> a "permission denied" error because bdrv_inherited_flags() forced the
> backing file to be read-only, ignoring the r/w reopen of bdrv_commit().
>
> The bigger problem this releaved is that the original open flags must
> actually only be applied to the temporary snapshot, and the original
> image file must be treated as a backing file of the temporary snapshot
> and get the right flags for that.
>
> Reported-by: Jan Kiszka <jan.kiszka@web.de>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 34 +++++++++++++++++++---------------
> include/block/block.h | 2 +-
> tests/qemu-iotests/051 | 4 ++++
> tests/qemu-iotests/051.out | 10 ++++++++++
> 4 files changed, 34 insertions(+), 16 deletions(-)
Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT
2014-05-07 8:24 ` Kevin Wolf
@ 2014-05-09 8:14 ` Stefan Hajnoczi
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2014-05-09 8:14 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Jan Kiszka, qemu-devel, stefanha, Max Reitz
On Wed, May 07, 2014 at 10:24:21AM +0200, Kevin Wolf wrote:
> Perhaps the monitor should be changed to avoid printing so many useless
> control characters, then we'd hit the limit less often...
>
> Stefan, didn't you plan to do something like this? Or was it unrelated?
I encountered this when working on readline for qemu-io. It's been a
while but I remember it's not a trivial fix, would require reworking the
readline or monitor code.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-05-09 8:15 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06 10:19 [Qemu-devel] [PATCH] block: Fix open flags with BDRV_O_SNAPSHOT Kevin Wolf
2014-05-06 11:10 ` Jan Kiszka
2014-05-06 21:03 ` Max Reitz
2014-05-07 8:24 ` Kevin Wolf
2014-05-09 8:14 ` Stefan Hajnoczi
2014-05-06 21:01 ` Max Reitz
2014-05-07 8:31 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).