[Qemu-devel] [PATCH for-2.0 0/3] bdrv_open() fixes

[Qemu-devel] [PATCH for-2.0 0/3] bdrv_open() fixes

[Kevin Wolf]

Kevin Wolf (3):
  block: Don't parse 'filename' option
  qemu-iotests: Remove CR line endings in reference output
  block: Fix snapshot=on for protocol parsed from filename

 block.c                    | 152 +++++++++++++++++++++++----------------------
 include/block/block.h      |   1 +
 tests/qemu-iotests/051     |   6 ++
 tests/qemu-iotests/051.out |  35 +++++++++--
 4 files changed, 115 insertions(+), 79 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] block: Don't parse 'filename' option

[Kevin Wolf]

When using the QDict option 'filename', it is supposed to be interpreted
literally. The code did correctly avoid guessing the protocol from any
string before the first colon, but it still called bdrv_parse_filename()
which would, for example, incorrectly remove a 'file:' prefix in the
raw-posix driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                    |  8 ++++----
 tests/qemu-iotests/051     |  4 ++++
 tests/qemu-iotests/051.out | 11 +++++++++++
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index 7a90a1b..df2b8d1 100644
--- a/block.c
+++ b/block.c
@@ -968,7 +968,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
 {
     BlockDriver *drv;
     const char *drvname;
-    bool allow_protocol_prefix = false;
+    bool parse_filename = false;
     Error *local_err = NULL;
     int ret;
 
@@ -977,7 +977,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
         filename = qdict_get_try_str(*options, "filename");
     } else if (filename && !qdict_haskey(*options, "filename")) {
         qdict_put(*options, "filename", qstring_from_str(filename));
-        allow_protocol_prefix = true;
+        parse_filename = true;
     } else {
         error_setg(errp, "Can't specify 'file' and 'filename' options at the "
                    "same time");
@@ -994,7 +994,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
         }
         qdict_del(*options, "driver");
     } else if (filename) {
-        drv = bdrv_find_protocol(filename, allow_protocol_prefix);
+        drv = bdrv_find_protocol(filename, parse_filename);
         if (!drv) {
             error_setg(errp, "Unknown protocol");
         }
@@ -1010,7 +1010,7 @@ static int bdrv_file_open(BlockDriverState *bs, const char *filename,
     }
 
     /* Parse the filename and open it */
-    if (drv->bdrv_parse_filename && filename) {
+    if (drv->bdrv_parse_filename && parse_filename) {
         drv->bdrv_parse_filename(filename, *options, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 14694e1..2f79b26 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -204,6 +204,10 @@ run_qemu -hda foo:bar
 run_qemu -drive file=foo:bar
 run_qemu -drive file.filename=foo:bar
 
+run_qemu -hda "file:$TEST_IMG"
+run_qemu -drive file="file:$TEST_IMG"
+run_qemu -drive file.filename="file:$TEST_IMG"
+
 echo
 echo === Snapshot mode ===
 echo
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index f5e33ff..671ac5f 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -275,6 +275,17 @@ QEMU_PROG: -drive file=foo:bar: could not open disk image foo:bar: Unknown proto
 Testing: -drive file.filename=foo:bar
 QEMU_PROG: -drive file.filename=foo:bar: could not open disk image ide0-hd0: Could not open 'foo:bar': No such file or directory
 
+Testing: -hda file:TEST_DIR/t.qcow2
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=file:TEST_DIR/t.qcow2
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file.filename=file:TEST_DIR/t.qcow2
+QEMU_PROG: -drive file.filename=file:TEST_DIR/t.qcow2: could not open disk image ide0-hd0: Could not open 'file:TEST_DIR/t.qcow2': No such file or directory
+
 
 === Snapshot mode ===
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] qemu-iotests: Remove CR line endings in reference output

[Kevin Wolf]

qemu doesn't print these CRs any more. The test still didn't fail
because the output comparison ignores line endings, but the change turns
up each time when you want to update the output.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/051.out | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 671ac5f..a631b0b 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -44,11 +44,11 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,driver=foo: could not open disk image TE
 === Overriding backing file ===
 
 Testing: -drive file=TEST_DIR/t.qcow2,driver=qcow2,backing.file.filename=TEST_DIR/t.qcow2.orig -nodefaults
-QEMU X.Y.Z monitor - type 'help' for more information
-(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
-ide0-hd0: TEST_DIR/t.qcow2 (qcow2)
-    Backing file:     TEST_DIR/t.qcow2.orig (chain depth: 1)
-(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+QEMU X.Y.Z monitor - type 'help' for more information
+(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
+ide0-hd0: TEST_DIR/t.qcow2 (qcow2)
+    Backing file:     TEST_DIR/t.qcow2.orig (chain depth: 1)
+(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
 
 === Enable and disable lazy refcounting on the command line, plus some invalid values ===
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] block: Fix snapshot=on for protocol parsed from filename

[Kevin Wolf]

Since commit 9fd3171a, BDRV_O_SNAPSHOT uses an option QDict to specify
the originally requested image as the backing file of the newly created
temporary snapshot. This means that the filename is stored in
"file.filename", which is an option that is not parsed for protocol
names. Therefore things like -drive file=nbd:localhost:10809 were
broken because it looked for a local file with the literal name
'nbd:localhost:10809'.

This patch changes the way BDRV_O_SNAPSHOT works once again. We now open
the originally requested image as normal, and then do a similar
operation as for live snapshots to put the temporary snapshot on top.
This way, both driver specific options and parsed filenames work.

As a nice side effect, this results in code movement to factor
bdrv_append_temp_snapshot() out. This is a good preparation for moving
its call to drive_init() and friends eventually.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c                    | 144 +++++++++++++++++++++++----------------------
 include/block/block.h      |   1 +
 tests/qemu-iotests/051     |   2 +
 tests/qemu-iotests/051.out |  14 +++++
 4 files changed, 91 insertions(+), 70 deletions(-)

diff --git a/block.c b/block.c
index df2b8d1..7817fea 100644
--- a/block.c
+++ b/block.c
@@ -1162,6 +1162,69 @@ done:
     return ret;
 }
 
+void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
+{
+    /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
+    char tmp_filename[PATH_MAX + 1];
+
+    int64_t total_size;
+    BlockDriver *bdrv_qcow2;
+    QEMUOptionParameter *create_options;
+    QDict *snapshot_options;
+    BlockDriverState *bs_snapshot;
+    Error *local_err;
+    int ret;
+
+    /* if snapshot, we create a temporary backing file and open it
+       instead of opening 'filename' directly */
+
+    /* Get the required size from the image */
+    total_size = bdrv_getlength(bs) & BDRV_SECTOR_MASK;
+
+    /* Create the temporary image */
+    ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not get temporary filename");
+        return;
+    }
+
+    bdrv_qcow2 = bdrv_find_format("qcow2");
+    create_options = parse_option_parameters("", bdrv_qcow2->create_options,
+                                             NULL);
+
+    set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
+
+    ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
+    free_option_parameters(create_options);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not create temporary overlay "
+                         "'%s': %s", tmp_filename,
+                         error_get_pretty(local_err));
+        error_free(local_err);
+        return;
+    }
+
+    /* Prepare a new options QDict for the temporary file */
+    snapshot_options = qdict_new();
+    qdict_put(snapshot_options, "file.driver",
+              qstring_from_str("file"));
+    qdict_put(snapshot_options, "file.filename",
+              qstring_from_str(tmp_filename));
+
+    bs_snapshot = bdrv_new("");
+    bs_snapshot->is_temporary = 1;
+
+    ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
+                    bs->open_flags & ~BDRV_O_SNAPSHOT, bdrv_qcow2, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    bdrv_append(bs_snapshot, bs);
+    bdrv_reopen(bs_snapshot, bs_snapshot->open_flags & ~BDRV_O_RDWR, NULL);
+}
+
 /*
  * Opens a disk image (raw, qcow2, vmdk, ...)
  *
@@ -1182,8 +1245,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
               BlockDriver *drv, Error **errp)
 {
     int ret;
-    /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
-    char tmp_filename[PATH_MAX + 1];
     BlockDriverState *file = NULL, *bs;
     const char *drvname;
     Error *local_err = NULL;
@@ -1243,74 +1304,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         }
     }
 
-    /* For snapshot=on, create a temporary qcow2 overlay */
-    if (flags & BDRV_O_SNAPSHOT) {
-        BlockDriverState *bs1;
-        int64_t total_size;
-        BlockDriver *bdrv_qcow2;
-        QEMUOptionParameter *create_options;
-        QDict *snapshot_options;
-
-        /* if snapshot, we create a temporary backing file and open it
-           instead of opening 'filename' directly */
-
-        /* Get the required size from the image */
-        QINCREF(options);
-        bs1 = NULL;
-        ret = bdrv_open(&bs1, filename, NULL, options, BDRV_O_NO_BACKING,
-                        drv, &local_err);
-        if (ret < 0) {
-            goto fail;
-        }
-        total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK;
-
-        bdrv_unref(bs1);
-
-        /* Create the temporary image */
-        ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not get temporary filename");
-            goto fail;
-        }
-
-        bdrv_qcow2 = bdrv_find_format("qcow2");
-        create_options = parse_option_parameters("", bdrv_qcow2->create_options,
-                                                 NULL);
-
-        set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
-
-        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
-        free_option_parameters(create_options);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not create temporary overlay "
-                             "'%s': %s", tmp_filename,
-                             error_get_pretty(local_err));
-            error_free(local_err);
-            local_err = NULL;
-            goto fail;
-        }
-
-        /* Prepare a new options QDict for the temporary file, where user
-         * options refer to the backing file */
-        if (filename) {
-            qdict_put(options, "file.filename", qstring_from_str(filename));
-        }
-        if (drv) {
-            qdict_put(options, "driver", qstring_from_str(drv->format_name));
-        }
-
-        snapshot_options = qdict_new();
-        qdict_put(snapshot_options, "backing", options);
-        qdict_flatten(snapshot_options);
-
-        bs->options = snapshot_options;
-        options = qdict_clone_shallow(bs->options);
-
-        filename = tmp_filename;
-        drv = bdrv_qcow2;
-        bs->is_temporary = 1;
-    }
-
     /* Open image file without format layer */
     if (flags & BDRV_O_RDWR) {
         flags |= BDRV_O_ALLOW_RDWR;
@@ -1372,6 +1365,17 @@ 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 (local_err) {
+            error_propagate(errp, local_err);
+            goto close_and_fail;
+        }
+    }
+
+
 done:
     /* Check if any unknown options were used */
     if (options && (qdict_size(options) != 0)) {
diff --git a/include/block/block.h b/include/block/block.h
index 1ed55d8..b3230a2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -190,6 +190,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);
 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 2f79b26..d6d926a 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -218,6 +218,8 @@ echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG"
 echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io
 echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io
 echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2 -snapshot | _filter_qemu_io
+echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG" -snapshot | _filter_qemu_io
+echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG",snapshot=on | _filter_qemu_io
 
 $QEMU_IO -c "read -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index a631b0b..dae0f1c 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -319,6 +319,20 @@ wrote 4096/4096 bytes at offset 0
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
+Testing: -drive file=file:TEST_DIR/t.qcow2 -snapshot
+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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 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) 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)
 Testing: -drive file=TEST_DIR/t.qcow2,snapshot=off
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/3] block: Don't parse 'filename' option

[Max Reitz]

On 04.04.2014 14:03, Kevin Wolf wrote:
> When using the QDict option 'filename', it is supposed to be interpreted
> literally. The code did correctly avoid guessing the protocol from any
> string before the first colon, but it still called bdrv_parse_filename()
> which would, for example, incorrectly remove a 'file:' prefix in the
> raw-posix driver.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c                    |  8 ++++----
>   tests/qemu-iotests/051     |  4 ++++
>   tests/qemu-iotests/051.out | 11 +++++++++++
>   3 files changed, 19 insertions(+), 4 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 2/3] qemu-iotests: Remove CR line endings in reference output

[Max Reitz]

On 04.04.2014 14:03, Kevin Wolf wrote:
> qemu doesn't print these CRs any more. The test still didn't fail
> because the output comparison ignores line endings, but the change turns
> up each time when you want to update the output.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/051.out | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)

This patch doesn't work so well over email, but looking at 051.out 
itself, it seems correct.

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH 3/3] block: Fix snapshot=on for protocol parsed from filename

[Max Reitz]

On 04.04.2014 14:03, Kevin Wolf wrote:
> Since commit 9fd3171a, BDRV_O_SNAPSHOT uses an option QDict to specify
> the originally requested image as the backing file of the newly created
> temporary snapshot. This means that the filename is stored in
> "file.filename", which is an option that is not parsed for protocol
> names. Therefore things like -drive file=nbd:localhost:10809 were
> broken because it looked for a local file with the literal name
> 'nbd:localhost:10809'.
>
> This patch changes the way BDRV_O_SNAPSHOT works once again. We now open
> the originally requested image as normal, and then do a similar
> operation as for live snapshots to put the temporary snapshot on top.
> This way, both driver specific options and parsed filenames work.
>
> As a nice side effect, this results in code movement to factor
> bdrv_append_temp_snapshot() out. This is a good preparation for moving
> its call to drive_init() and friends eventually.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   block.c                    | 144 +++++++++++++++++++++++----------------------
>   include/block/block.h      |   1 +
>   tests/qemu-iotests/051     |   2 +
>   tests/qemu-iotests/051.out |  14 +++++
>   4 files changed, 91 insertions(+), 70 deletions(-)
>
> diff --git a/block.c b/block.c
> index df2b8d1..7817fea 100644
> --- a/block.c
> +++ b/block.c
> @@ -1162,6 +1162,69 @@ done:
>       return ret;
>   }
>   
> +void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
> +{
> +    /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> +    char tmp_filename[PATH_MAX + 1];
> +
> +    int64_t total_size;
> +    BlockDriver *bdrv_qcow2;
> +    QEMUOptionParameter *create_options;
> +    QDict *snapshot_options;
> +    BlockDriverState *bs_snapshot;
> +    Error *local_err;
> +    int ret;
> +
> +    /* if snapshot, we create a temporary backing file and open it
> +       instead of opening 'filename' directly */
> +
> +    /* Get the required size from the image */
> +    total_size = bdrv_getlength(bs) & BDRV_SECTOR_MASK;

bdrv_getlength() may fail.

> +
> +    /* Create the temporary image */
> +    ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "Could not get temporary filename");
> +        return;
> +    }
> +
> +    bdrv_qcow2 = bdrv_find_format("qcow2");
> +    create_options = parse_option_parameters("", bdrv_qcow2->create_options,
> +                                             NULL);
> +
> +    set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
> +
> +    ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
> +    free_option_parameters(create_options);
> +    if (ret < 0) {
> +        error_setg_errno(errp, -ret, "Could not create temporary overlay "
> +                         "'%s': %s", tmp_filename,
> +                         error_get_pretty(local_err));
> +        error_free(local_err);
> +        return;
> +    }
> +
> +    /* Prepare a new options QDict for the temporary file */
> +    snapshot_options = qdict_new();
> +    qdict_put(snapshot_options, "file.driver",
> +              qstring_from_str("file"));
> +    qdict_put(snapshot_options, "file.filename",
> +              qstring_from_str(tmp_filename));
> +
> +    bs_snapshot = bdrv_new("");
> +    bs_snapshot->is_temporary = 1;
> +
> +    ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
> +                    bs->open_flags & ~BDRV_O_SNAPSHOT, bdrv_qcow2, &local_err);
> +    if (ret < 0) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    bdrv_append(bs_snapshot, bs);
> +    bdrv_reopen(bs_snapshot, bs_snapshot->open_flags & ~BDRV_O_RDWR, NULL);

This may fail as well. I don't know how likely that is, but it might 
(for qcow2 at least after your patch "qcow2: Flush metadata during 
read-only reopen").

> +}
> +
>   /*
>    * Opens a disk image (raw, qcow2, vmdk, ...)
>    *
> @@ -1182,8 +1245,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>                 BlockDriver *drv, Error **errp)
>   {
>       int ret;
> -    /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> -    char tmp_filename[PATH_MAX + 1];
>       BlockDriverState *file = NULL, *bs;
>       const char *drvname;
>       Error *local_err = NULL;
> @@ -1243,74 +1304,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
>           }
>       }
>   
> -    /* For snapshot=on, create a temporary qcow2 overlay */
> -    if (flags & BDRV_O_SNAPSHOT) {
> -        BlockDriverState *bs1;
> -        int64_t total_size;
> -        BlockDriver *bdrv_qcow2;
> -        QEMUOptionParameter *create_options;
> -        QDict *snapshot_options;
> -
> -        /* if snapshot, we create a temporary backing file and open it
> -           instead of opening 'filename' directly */
> -
> -        /* Get the required size from the image */
> -        QINCREF(options);
> -        bs1 = NULL;
> -        ret = bdrv_open(&bs1, filename, NULL, options, BDRV_O_NO_BACKING,
> -                        drv, &local_err);
> -        if (ret < 0) {
> -            goto fail;
> -        }
> -        total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK;
> -
> -        bdrv_unref(bs1);
> -
> -        /* Create the temporary image */
> -        ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
> -        if (ret < 0) {
> -            error_setg_errno(errp, -ret, "Could not get temporary filename");
> -            goto fail;
> -        }
> -
> -        bdrv_qcow2 = bdrv_find_format("qcow2");
> -        create_options = parse_option_parameters("", bdrv_qcow2->create_options,
> -                                                 NULL);
> -
> -        set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
> -
> -        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
> -        free_option_parameters(create_options);
> -        if (ret < 0) {
> -            error_setg_errno(errp, -ret, "Could not create temporary overlay "
> -                             "'%s': %s", tmp_filename,
> -                             error_get_pretty(local_err));
> -            error_free(local_err);
> -            local_err = NULL;
> -            goto fail;
> -        }
> -
> -        /* Prepare a new options QDict for the temporary file, where user
> -         * options refer to the backing file */
> -        if (filename) {
> -            qdict_put(options, "file.filename", qstring_from_str(filename));
> -        }
> -        if (drv) {
> -            qdict_put(options, "driver", qstring_from_str(drv->format_name));
> -        }
> -
> -        snapshot_options = qdict_new();
> -        qdict_put(snapshot_options, "backing", options);
> -        qdict_flatten(snapshot_options);
> -
> -        bs->options = snapshot_options;
> -        options = qdict_clone_shallow(bs->options);
> -
> -        filename = tmp_filename;
> -        drv = bdrv_qcow2;
> -        bs->is_temporary = 1;
> -    }
> -
>       /* Open image file without format layer */
>       if (flags & BDRV_O_RDWR) {
>           flags |= BDRV_O_ALLOW_RDWR;
> @@ -1372,6 +1365,17 @@ 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 (local_err) {
> +            error_propagate(errp, local_err);
> +            goto close_and_fail;
> +        }
> +    }
> +
> +
>   done:
>       /* Check if any unknown options were used */
>       if (options && (qdict_size(options) != 0)) {
> diff --git a/include/block/block.h b/include/block/block.h
> index 1ed55d8..b3230a2 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -190,6 +190,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);
>   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 2f79b26..d6d926a 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -218,6 +218,8 @@ echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG"
>   echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io
>   echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io
>   echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2 -snapshot | _filter_qemu_io
> +echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG" -snapshot | _filter_qemu_io
> +echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG",snapshot=on | _filter_qemu_io
>   
>   $QEMU_IO -c "read -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io
>   
> diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
> index a631b0b..dae0f1c 100644
> --- a/tests/qemu-iotests/051.out
> +++ b/tests/qemu-iotests/051.out
> @@ -319,6 +319,20 @@ wrote 4096/4096 bytes at offset 0
>   4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>   (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>   
> +Testing: -drive file=file:TEST_DIR/t.qcow2 -snapshot
> +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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 0 4k"^[[K

The splitting if this one overly long line into three single lines gave 
git a hard time, but after manual fixing, it applies.

> +wrote 4096/4096 bytes at offset 0
> +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
> +
> +Testing: -drive file=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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 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) 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)
>   Testing: -drive file=TEST_DIR/t.qcow2,snapshot=off

I don't find not checking the return values too bad, as both functions 
are pretty unlikely to fail; normally, I'd probably give a reviewed-by, 
but after just having reviewed the block fix series…

Max

[Qemu-devel] [PATCH v2 3/3] block: Fix snapshot=on for protocol parsed from filename

[Kevin Wolf]

Since commit 9fd3171a, BDRV_O_SNAPSHOT uses an option QDict to specify
the originally requested image as the backing file of the newly created
temporary snapshot. This means that the filename is stored in
"file.filename", which is an option that is not parsed for protocol
names. Therefore things like -drive file=nbd:localhost:10809 were
broken because it looked for a local file with the literal name
'nbd:localhost:10809'.

This patch changes the way BDRV_O_SNAPSHOT works once again. We now open
the originally requested image as normal, and then do a similar
operation as for live snapshots to put the temporary snapshot on top.
This way, both driver specific options and parsed filenames work.

As a nice side effect, this results in code movement to factor
bdrv_append_temp_snapshot() out. This is a good preparation for moving
its call to drive_init() and friends eventually.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
v2:
- open the backing file for the temporary snapshot read-only from the
  beginning so that you can get an r/w block device from a read-only
  file using snapshot=on. Gets rid of the bdrv_reopen() whose return
  value was ignored in v1, too.

 block.c                    | 148 ++++++++++++++++++++++++---------------------
 include/block/block.h      |   1 +
 tests/qemu-iotests/051     |   8 +++
 tests/qemu-iotests/051.out |  28 +++++++++
 4 files changed, 115 insertions(+), 70 deletions(-)

diff --git a/block.c b/block.c
index df2b8d1..d89c344 100644
--- a/block.c
+++ b/block.c
@@ -767,6 +767,11 @@ 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.
@@ -1162,6 +1167,68 @@ done:
     return ret;
 }
 
+void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
+{
+    /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
+    char tmp_filename[PATH_MAX + 1];
+
+    int64_t total_size;
+    BlockDriver *bdrv_qcow2;
+    QEMUOptionParameter *create_options;
+    QDict *snapshot_options;
+    BlockDriverState *bs_snapshot;
+    Error *local_err;
+    int ret;
+
+    /* if snapshot, we create a temporary backing file and open it
+       instead of opening 'filename' directly */
+
+    /* Get the required size from the image */
+    total_size = bdrv_getlength(bs) & BDRV_SECTOR_MASK;
+
+    /* Create the temporary image */
+    ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not get temporary filename");
+        return;
+    }
+
+    bdrv_qcow2 = bdrv_find_format("qcow2");
+    create_options = parse_option_parameters("", bdrv_qcow2->create_options,
+                                             NULL);
+
+    set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
+
+    ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
+    free_option_parameters(create_options);
+    if (ret < 0) {
+        error_setg_errno(errp, -ret, "Could not create temporary overlay "
+                         "'%s': %s", tmp_filename,
+                         error_get_pretty(local_err));
+        error_free(local_err);
+        return;
+    }
+
+    /* Prepare a new options QDict for the temporary file */
+    snapshot_options = qdict_new();
+    qdict_put(snapshot_options, "file.driver",
+              qstring_from_str("file"));
+    qdict_put(snapshot_options, "file.filename",
+              qstring_from_str(tmp_filename));
+
+    bs_snapshot = bdrv_new("");
+    bs_snapshot->is_temporary = 1;
+
+    ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
+                    bs->open_flags & ~BDRV_O_SNAPSHOT, bdrv_qcow2, &local_err);
+    if (ret < 0) {
+        error_propagate(errp, local_err);
+        return;
+    }
+
+    bdrv_append(bs_snapshot, bs);
+}
+
 /*
  * Opens a disk image (raw, qcow2, vmdk, ...)
  *
@@ -1182,8 +1249,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
               BlockDriver *drv, Error **errp)
 {
     int ret;
-    /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
-    char tmp_filename[PATH_MAX + 1];
     BlockDriverState *file = NULL, *bs;
     const char *drvname;
     Error *local_err = NULL;
@@ -1243,74 +1308,6 @@ int bdrv_open(BlockDriverState **pbs, const char *filename,
         }
     }
 
-    /* For snapshot=on, create a temporary qcow2 overlay */
-    if (flags & BDRV_O_SNAPSHOT) {
-        BlockDriverState *bs1;
-        int64_t total_size;
-        BlockDriver *bdrv_qcow2;
-        QEMUOptionParameter *create_options;
-        QDict *snapshot_options;
-
-        /* if snapshot, we create a temporary backing file and open it
-           instead of opening 'filename' directly */
-
-        /* Get the required size from the image */
-        QINCREF(options);
-        bs1 = NULL;
-        ret = bdrv_open(&bs1, filename, NULL, options, BDRV_O_NO_BACKING,
-                        drv, &local_err);
-        if (ret < 0) {
-            goto fail;
-        }
-        total_size = bdrv_getlength(bs1) & BDRV_SECTOR_MASK;
-
-        bdrv_unref(bs1);
-
-        /* Create the temporary image */
-        ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not get temporary filename");
-            goto fail;
-        }
-
-        bdrv_qcow2 = bdrv_find_format("qcow2");
-        create_options = parse_option_parameters("", bdrv_qcow2->create_options,
-                                                 NULL);
-
-        set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
-
-        ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
-        free_option_parameters(create_options);
-        if (ret < 0) {
-            error_setg_errno(errp, -ret, "Could not create temporary overlay "
-                             "'%s': %s", tmp_filename,
-                             error_get_pretty(local_err));
-            error_free(local_err);
-            local_err = NULL;
-            goto fail;
-        }
-
-        /* Prepare a new options QDict for the temporary file, where user
-         * options refer to the backing file */
-        if (filename) {
-            qdict_put(options, "file.filename", qstring_from_str(filename));
-        }
-        if (drv) {
-            qdict_put(options, "driver", qstring_from_str(drv->format_name));
-        }
-
-        snapshot_options = qdict_new();
-        qdict_put(snapshot_options, "backing", options);
-        qdict_flatten(snapshot_options);
-
-        bs->options = snapshot_options;
-        options = qdict_clone_shallow(bs->options);
-
-        filename = tmp_filename;
-        drv = bdrv_qcow2;
-        bs->is_temporary = 1;
-    }
-
     /* Open image file without format layer */
     if (flags & BDRV_O_RDWR) {
         flags |= BDRV_O_ALLOW_RDWR;
@@ -1372,6 +1369,17 @@ 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 (local_err) {
+            error_propagate(errp, local_err);
+            goto close_and_fail;
+        }
+    }
+
+
 done:
     /* Check if any unknown options were used */
     if (options && (qdict_size(options) != 0)) {
diff --git a/include/block/block.h b/include/block/block.h
index 1ed55d8..b3230a2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -190,6 +190,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);
 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 2f79b26..073dc7a 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -218,6 +218,14 @@ echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG"
 echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io
 echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2,snapshot=on | _filter_qemu_io
 echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file.filename="$TEST_IMG",driver=qcow2 -snapshot | _filter_qemu_io
+echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG" -snapshot | _filter_qemu_io
+echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="file:$TEST_IMG",snapshot=on | _filter_qemu_io
+
+# Opening a read-only file r/w with snapshot=on
+chmod u-w "$TEST_IMG"
+echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG" -snapshot | _filter_qemu_io
+echo 'qemu-io ide0-hd0 "write -P 0x22 0 4k"' | run_qemu -drive file="$TEST_IMG",snapshot=on | _filter_qemu_io
+chmod u+w "$TEST_IMG"
 
 $QEMU_IO -c "read -P 0x11 0 4k" "$TEST_IMG" | _filter_qemu_io
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index a631b0b..01b0384 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -319,6 +319,34 @@ wrote 4096/4096 bytes at offset 0
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
 
+Testing: -drive file=file:TEST_DIR/t.qcow2 -snapshot
+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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+Testing: -drive file=TEST_DIR/t.qcow2 -snapshot
+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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 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) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
+
+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 0x2^[[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 0x22^[[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 0x22 ^[[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 0x22 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 0x22 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 0x22 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 0x22 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 0x22 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) 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)
 Testing: -drive file=TEST_DIR/t.qcow2,snapshot=off
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 3/3] block: Fix snapshot=on for protocol parsed from filename

[Kevin Wolf]

Am 04.04.2014 um 16:38 hat Max Reitz geschrieben:
> On 04.04.2014 14:03, Kevin Wolf wrote:
> >Since commit 9fd3171a, BDRV_O_SNAPSHOT uses an option QDict to specify
> >the originally requested image as the backing file of the newly created
> >temporary snapshot. This means that the filename is stored in
> >"file.filename", which is an option that is not parsed for protocol
> >names. Therefore things like -drive file=nbd:localhost:10809 were
> >broken because it looked for a local file with the literal name
> >'nbd:localhost:10809'.
> >
> >This patch changes the way BDRV_O_SNAPSHOT works once again. We now open
> >the originally requested image as normal, and then do a similar
> >operation as for live snapshots to put the temporary snapshot on top.
> >This way, both driver specific options and parsed filenames work.
> >
> >As a nice side effect, this results in code movement to factor
> >bdrv_append_temp_snapshot() out. This is a good preparation for moving
> >its call to drive_init() and friends eventually.
> >
> >Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> >---
> >  block.c                    | 144 +++++++++++++++++++++++----------------------
> >  include/block/block.h      |   1 +
> >  tests/qemu-iotests/051     |   2 +
> >  tests/qemu-iotests/051.out |  14 +++++
> >  4 files changed, 91 insertions(+), 70 deletions(-)
> >
> >diff --git a/block.c b/block.c
> >index df2b8d1..7817fea 100644
> >--- a/block.c
> >+++ b/block.c
> >@@ -1162,6 +1162,69 @@ done:
> >      return ret;
> >  }
> >+void bdrv_append_temp_snapshot(BlockDriverState *bs, Error **errp)
> >+{
> >+    /* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
> >+    char tmp_filename[PATH_MAX + 1];
> >+
> >+    int64_t total_size;
> >+    BlockDriver *bdrv_qcow2;
> >+    QEMUOptionParameter *create_options;
> >+    QDict *snapshot_options;
> >+    BlockDriverState *bs_snapshot;
> >+    Error *local_err;
> >+    int ret;
> >+
> >+    /* if snapshot, we create a temporary backing file and open it
> >+       instead of opening 'filename' directly */
> >+
> >+    /* Get the required size from the image */
> >+    total_size = bdrv_getlength(bs) & BDRV_SECTOR_MASK;
> 
> bdrv_getlength() may fail.

Aye. This is just code motion, but I'll send a follow-up patch for that.

> >+
> >+    /* Create the temporary image */
> >+    ret = get_tmp_filename(tmp_filename, sizeof(tmp_filename));
> >+    if (ret < 0) {
> >+        error_setg_errno(errp, -ret, "Could not get temporary filename");
> >+        return;
> >+    }
> >+
> >+    bdrv_qcow2 = bdrv_find_format("qcow2");
> >+    create_options = parse_option_parameters("", bdrv_qcow2->create_options,
> >+                                             NULL);
> >+
> >+    set_option_parameter_int(create_options, BLOCK_OPT_SIZE, total_size);
> >+
> >+    ret = bdrv_create(bdrv_qcow2, tmp_filename, create_options, &local_err);
> >+    free_option_parameters(create_options);
> >+    if (ret < 0) {
> >+        error_setg_errno(errp, -ret, "Could not create temporary overlay "
> >+                         "'%s': %s", tmp_filename,
> >+                         error_get_pretty(local_err));
> >+        error_free(local_err);
> >+        return;
> >+    }
> >+
> >+    /* Prepare a new options QDict for the temporary file */
> >+    snapshot_options = qdict_new();
> >+    qdict_put(snapshot_options, "file.driver",
> >+              qstring_from_str("file"));
> >+    qdict_put(snapshot_options, "file.filename",
> >+              qstring_from_str(tmp_filename));
> >+
> >+    bs_snapshot = bdrv_new("");
> >+    bs_snapshot->is_temporary = 1;
> >+
> >+    ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
> >+                    bs->open_flags & ~BDRV_O_SNAPSHOT, bdrv_qcow2, &local_err);
> >+    if (ret < 0) {
> >+        error_propagate(errp, local_err);
> >+        return;
> >+    }
> >+
> >+    bdrv_append(bs_snapshot, bs);
> >+    bdrv_reopen(bs_snapshot, bs_snapshot->open_flags & ~BDRV_O_RDWR, NULL);
> 
> This may fail as well. I don't know how likely that is, but it might
> (for qcow2 at least after your patch "qcow2: Flush metadata during
> read-only reopen").

Right. My v2 that fixes a real bug gets rid of this line, but otherwise
checking and returning failure would indeed have made sense here. I
looked at things like live commit for the reopen and there the best
option was to leave the backing file just r/w if the reopen fails.

Kevin

Re: [Qemu-devel] [PATCH v2 3/3] block: Fix snapshot=on for protocol parsed from filename

[Max Reitz]

On 04.04.2014 16:56, Kevin Wolf wrote:
> Since commit 9fd3171a, BDRV_O_SNAPSHOT uses an option QDict to specify
> the originally requested image as the backing file of the newly created
> temporary snapshot. This means that the filename is stored in
> "file.filename", which is an option that is not parsed for protocol
> names. Therefore things like -drive file=nbd:localhost:10809 were
> broken because it looked for a local file with the literal name
> 'nbd:localhost:10809'.
>
> This patch changes the way BDRV_O_SNAPSHOT works once again. We now open
> the originally requested image as normal, and then do a similar
> operation as for live snapshots to put the temporary snapshot on top.
> This way, both driver specific options and parsed filenames work.
>
> As a nice side effect, this results in code movement to factor
> bdrv_append_temp_snapshot() out. This is a good preparation for moving
> its call to drive_init() and friends eventually.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> v2:
> - open the backing file for the temporary snapshot read-only from the
>    beginning so that you can get an r/w block device from a read-only
>    file using snapshot=on. Gets rid of the bdrv_reopen() whose return
>    value was ignored in v1, too.
>
>   block.c                    | 148 ++++++++++++++++++++++++---------------------
>   include/block/block.h      |   1 +
>   tests/qemu-iotests/051     |   8 +++
>   tests/qemu-iotests/051.out |  28 +++++++++
>   4 files changed, 115 insertions(+), 70 deletions(-)

Together with "block: Check bdrv_getlength() return value in 
bdrv_append_temp_snapshot()":

Reviewed-by: Max Reitz <mreitz@redhat.com>