qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd
@ 2013-11-22  4:27 Wenchao Xia
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 1/6] snapshot: distinguish id and name in load_tmp Wenchao Xia
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-11-22  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

This series allow user to read internal snapshot's contents without qemu-img
convert.

V2:
  Address Stefan's comments:
  02: add 'fall through' comments in the case statement.
  03: add doc about the difference of internal snapshot and backing chain
snapshot, which is used in previous '--snapshot' parameter.
  Other:
  01,04: rebased on upstream with conflict resolved.

v3:
  Address Paolo's comments:
  02: add parameter "-l snapshot_id_or_name", rename options
snapshot-load to load-snapshot, use QemuOpts.
  03: rename snapshot-load to load-snapshot.
  04: related change to test both -l and -L case.
  05-07: add similar parameter for qemu-img convert.
  Other:
  01: foldered original snapshot logic into function
bdrv_snapshot_load_tmp_by_id_or_name(), since multiple
caller present in this version. Refined error message from
", reason: %s" to ": %s".
  02: Refined error message from ", reason: %s" to ": %s".
  03: Rename PARAM to SNAPSHOT_PARAM.

v4:
  Address Eric's comments:
  01: typo fix.
  02: squashed patch. Use only one option -l and distiguish
the cases by the starting string.
  03: remove 'eval', remove '_supported_os Linux', remove the
comments that have typo.
  04: squashed patch. Add only one option -l and distiguish
the cases by the starting string.
  05: remove indentation.
  Address Paolo's comments:
  02: Use only one option -l and distiguish the cases by
the starting string.
  04: Use only one option -l and distiguish the cases by
the starting string, mark old -s as deprecated, added doc
for them.

v5:
  Address Jeff's comments:
  Quote image name in test cases.

v6:
  Address Kevin's comments:
  1: typo fix, remove device and snapshot info in error message.
  2: use strstart().
  3: use _require_command(), limit proto to file, since when proto=nbd
it can't work. also changed _require_command() to tip better.
  4: use strstart().
  6: new patch, since I found the doc missing in debugging.

Wenchao Xia (6):
  1 snapshot: distinguish id and name in load_tmp
  2 qemu-nbd: support internal snapshot export
  3 qemu-iotests: add 058 internal snapshot export with qemu-nbd case
  4 qemu-img: add -l for snapshot in convert
  5 qemu-iotests: add test for snapshot in qemu-img convert
  6 qemu-nbd: add doc for option -f

 block/qcow2-snapshot.c       |   10 +++-
 block/qcow2.h                |    5 ++-
 block/snapshot.c             |   77 ++++++++++++++++++++++++++-
 include/block/block_int.h    |    4 +-
 include/block/snapshot.h     |   15 +++++-
 qemu-img-cmds.hx             |    2 +-
 qemu-img.c                   |   44 +++++++++++++---
 qemu-img.texi                |   12 +++--
 qemu-nbd.c                   |   47 ++++++++++++++++-
 qemu-nbd.texi                |   10 +++-
 tests/qemu-iotests/058       |  120 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/058.out   |   44 +++++++++++++++
 tests/qemu-iotests/check     |    1 +
 tests/qemu-iotests/common.rc |    3 +-
 tests/qemu-iotests/group     |    1 +
 15 files changed, 371 insertions(+), 24 deletions(-)
 create mode 100755 tests/qemu-iotests/058
 create mode 100644 tests/qemu-iotests/058.out

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH V6 1/6] snapshot: distinguish id and name in load_tmp
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
@ 2013-11-22  4:27 ` Wenchao Xia
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 2/6] qemu-nbd: support internal snapshot export Wenchao Xia
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-11-22  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

Since later this function will be used so improve it. The only caller of it
now is qemu-img, and it is not impacted by introduce function
bdrv_snapshot_load_tmp_by_id_or_name() that call bdrv_snapshot_load_tmp()
twice to keep old search logic. bdrv_snapshot_load_tmp_by_id_or_name() return
int to let caller know the errno, and errno will be used later.
Also fix a typo in comments of bdrv_snapshot_delete().

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qcow2-snapshot.c    |   10 ++++++-
 block/qcow2.h             |    5 +++-
 block/snapshot.c          |   59 ++++++++++++++++++++++++++++++++++++++++++--
 include/block/block_int.h |    4 ++-
 include/block/snapshot.h  |    7 ++++-
 qemu-img.c                |    8 ++++-
 6 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 3529c68..ad8bf3d 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -675,7 +675,10 @@ int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab)
     return s->nb_snapshots;
 }
 
-int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
+int qcow2_snapshot_load_tmp(BlockDriverState *bs,
+                            const char *snapshot_id,
+                            const char *name,
+                            Error **errp)
 {
     int i, snapshot_index;
     BDRVQcowState *s = bs->opaque;
@@ -687,8 +690,10 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
     assert(bs->read_only);
 
     /* Search the snapshot */
-    snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_name);
+    snapshot_index = find_snapshot_by_id_and_name(bs, snapshot_id, name);
     if (snapshot_index < 0) {
+        error_setg(errp,
+                   "Can't find snapshot");
         return -ENOENT;
     }
     sn = &s->snapshots[snapshot_index];
@@ -699,6 +704,7 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
 
     ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes);
     if (ret < 0) {
+        error_setg(errp, "Failed to read l1 table for snapshot");
         g_free(new_l1_table);
         return ret;
     }
diff --git a/block/qcow2.h b/block/qcow2.h
index 922e190..303eb26 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -488,7 +488,10 @@ int qcow2_snapshot_delete(BlockDriverState *bs,
                           const char *name,
                           Error **errp);
 int qcow2_snapshot_list(BlockDriverState *bs, QEMUSnapshotInfo **psn_tab);
-int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name);
+int qcow2_snapshot_load_tmp(BlockDriverState *bs,
+                            const char *snapshot_id,
+                            const char *name,
+                            Error **errp);
 
 void qcow2_free_snapshots(BlockDriverState *bs);
 int qcow2_read_snapshots(BlockDriverState *bs);
diff --git a/block/snapshot.c b/block/snapshot.c
index a05c0c0..565222e 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -194,7 +194,7 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
  * If only @snapshot_id is specified, delete the first one with id
  * @snapshot_id.
  * If only @name is specified, delete the first one with name @name.
- * if none is specified, return -ENINVAL.
+ * if none is specified, return -EINVAL.
  *
  * Returns: 0 on success, -errno on failure. If @bs is not inserted, return
  * -ENOMEDIUM. If @snapshot_id and @name are both NULL, return -EINVAL. If @bs
@@ -265,18 +265,71 @@ int bdrv_snapshot_list(BlockDriverState *bs,
     return -ENOTSUP;
 }
 
+/**
+ * Temporarily load an internal snapshot by @snapshot_id and @name.
+ * @bs: block device used in the operation
+ * @snapshot_id: unique snapshot ID, or NULL
+ * @name: snapshot name, or NULL
+ * @errp: location to store error
+ *
+ * If both @snapshot_id and @name are specified, load the first one with
+ * id @snapshot_id and name @name.
+ * If only @snapshot_id is specified, load the first one with id
+ * @snapshot_id.
+ * If only @name is specified, load the first one with name @name.
+ * if none is specified, return -EINVAL.
+ *
+ * Returns: 0 on success, -errno on fail. If @bs is not inserted, return
+ * -ENOMEDIUM. If @bs is not readonly, return -EINVAL. If @bs did not support
+ * internal snapshot, return -ENOTSUP. If qemu can't find a matching @id and
+ * @name, return -ENOENT. If @errp != NULL, it will always be filled on
+ * failure.
+ */
 int bdrv_snapshot_load_tmp(BlockDriverState *bs,
-        const char *snapshot_name)
+                           const char *snapshot_id,
+                           const char *name,
+                           Error **errp)
 {
     BlockDriver *drv = bs->drv;
+
     if (!drv) {
+        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
         return -ENOMEDIUM;
     }
+    if (!snapshot_id && !name) {
+        error_setg(errp, "snapshot_id and name are both NULL");
+        return -EINVAL;
+    }
     if (!bs->read_only) {
+        error_setg(errp, "Device is not readonly");
         return -EINVAL;
     }
     if (drv->bdrv_snapshot_load_tmp) {
-        return drv->bdrv_snapshot_load_tmp(bs, snapshot_name);
+        return drv->bdrv_snapshot_load_tmp(bs, snapshot_id, name, errp);
     }
+    error_set(errp, QERR_BLOCK_FORMAT_FEATURE_NOT_SUPPORTED,
+              drv->format_name, bdrv_get_device_name(bs),
+              "temporarily load internal snapshot");
     return -ENOTSUP;
 }
+
+int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
+                                         const char *id_or_name,
+                                         Error **errp)
+{
+    int ret;
+    Error *local_err = NULL;
+
+    ret = bdrv_snapshot_load_tmp(bs, id_or_name, NULL, &local_err);
+    if (ret == -ENOENT || ret == -EINVAL) {
+        error_free(local_err);
+        local_err = NULL;
+        ret = bdrv_snapshot_load_tmp(bs, NULL, id_or_name, &local_err);
+    }
+
+    if (error_is_set(&local_err)) {
+        error_propagate(errp, local_err);
+    }
+
+    return ret;
+}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 1666066..8bbfb09 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -175,7 +175,9 @@ struct BlockDriver {
     int (*bdrv_snapshot_list)(BlockDriverState *bs,
                               QEMUSnapshotInfo **psn_info);
     int (*bdrv_snapshot_load_tmp)(BlockDriverState *bs,
-                                  const char *snapshot_name);
+                                  const char *snapshot_id,
+                                  const char *name,
+                                  Error **errp);
     int (*bdrv_get_info)(BlockDriverState *bs, BlockDriverInfo *bdi);
     ImageInfoSpecific *(*bdrv_get_specific_info)(BlockDriverState *bs);
 
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 012bf22..d05bea7 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -61,5 +61,10 @@ void bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
 int bdrv_snapshot_list(BlockDriverState *bs,
                        QEMUSnapshotInfo **psn_info);
 int bdrv_snapshot_load_tmp(BlockDriverState *bs,
-                           const char *snapshot_name);
+                           const char *snapshot_id,
+                           const char *name,
+                           Error **errp);
+int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
+                                         const char *id_or_name,
+                                         Error **errp);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index b6b5644..a73beb5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1260,8 +1260,12 @@ static int img_convert(int argc, char **argv)
             ret = -1;
             goto out;
         }
-        if (bdrv_snapshot_load_tmp(bs[0], snapshot_name) < 0) {
-            error_report("Failed to load snapshot");
+
+        bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
+        if (error_is_set(&local_err)) {
+            error_report("Failed to load snapshot: %s",
+                         error_get_pretty(local_err));
+            error_free(local_err);
             ret = -1;
             goto out;
         }
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH V6 2/6] qemu-nbd: support internal snapshot export
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 1/6] snapshot: distinguish id and name in load_tmp Wenchao Xia
@ 2013-11-22  4:27 ` Wenchao Xia
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 3/6] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-11-22  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

Now it is possible to directly export an internal snapshot, which
can be used to probe the snapshot's contents without qemu-img
convert.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/snapshot.c         |   18 ++++++++++++++++++
 include/block/snapshot.h |    8 ++++++++
 qemu-nbd.c               |   46 ++++++++++++++++++++++++++++++++++++++++++++--
 qemu-nbd.texi            |    8 +++++++-
 4 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index 565222e..9047f8d 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -25,6 +25,24 @@
 #include "block/snapshot.h"
 #include "block/block_int.h"
 
+QemuOptsList internal_snapshot_opts = {
+    .name = "snapshot",
+    .head = QTAILQ_HEAD_INITIALIZER(internal_snapshot_opts.head),
+    .desc = {
+        {
+            .name = SNAPSHOT_OPT_ID,
+            .type = QEMU_OPT_STRING,
+            .help = "snapshot id"
+        },{
+            .name = SNAPSHOT_OPT_NAME,
+            .type = QEMU_OPT_STRING,
+            .help = "snapshot name"
+        },{
+            /* end of list */
+        }
+    },
+};
+
 int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info,
                        const char *name)
 {
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index d05bea7..770d9bb 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -27,6 +27,14 @@
 
 #include "qemu-common.h"
 #include "qapi/error.h"
+#include "qemu/option.h"
+
+
+#define SNAPSHOT_OPT_BASE       "snapshot."
+#define SNAPSHOT_OPT_ID         "snapshot.id"
+#define SNAPSHOT_OPT_NAME       "snapshot.name"
+
+extern QemuOptsList internal_snapshot_opts;
 
 typedef struct QEMUSnapshotInfo {
     char id_str[128]; /* unique snapshot id */
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..fe6053c 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -20,6 +20,7 @@
 #include "block/block.h"
 #include "block/nbd.h"
 #include "qemu/main-loop.h"
+#include "block/snapshot.h"
 
 #include <stdarg.h>
 #include <stdio.h>
@@ -79,7 +80,14 @@ static void usage(const char *name)
 "\n"
 "Block device options:\n"
 "  -r, --read-only      export read-only\n"
-"  -s, --snapshot       use snapshot file\n"
+"  -s, --snapshot       use FILE as an external snapshot, create a temporary\n"
+"                       file with backing_file=FILE, redirect the write to\n"
+"                       the temporary one\n"
+"  -l, --load-snapshot=SNAPSHOT_PARAM\n"
+"                       load an internal snapshot inside FILE and export it\n"
+"                       as an read-only device, SNAPSHOT_PARAM format is\n"
+"                       'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
+"                       '[ID_OR_NAME]'\n"
 "  -n, --nocache        disable host cache\n"
 "      --cache=MODE     set cache mode (none, writeback, ...)\n"
 #ifdef CONFIG_LINUX_AIO
@@ -315,7 +323,9 @@ int main(int argc, char **argv)
     char *device = NULL;
     int port = NBD_DEFAULT_PORT;
     off_t fd_size;
-    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:t";
+    QemuOpts *sn_opts = NULL;
+    const char *sn_id_or_name = NULL;
+    const char *sopt = "hVb:o:p:rsnP:c:dvk:e:f:tl:";
     struct option lopt[] = {
         { "help", 0, NULL, 'h' },
         { "version", 0, NULL, 'V' },
@@ -328,6 +338,7 @@ int main(int argc, char **argv)
         { "connect", 1, NULL, 'c' },
         { "disconnect", 0, NULL, 'd' },
         { "snapshot", 0, NULL, 's' },
+        { "load-snapshot", 1, NULL, 'l' },
         { "nocache", 0, NULL, 'n' },
         { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
 #ifdef CONFIG_LINUX_AIO
@@ -428,6 +439,17 @@ int main(int argc, char **argv)
                 errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
             }
             break;
+        case 'l':
+            if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
+                sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+                if (!sn_opts) {
+                    errx(EXIT_FAILURE, "Failed in parsing snapshot param `%s'",
+                         optarg);
+                }
+            } else {
+                sn_id_or_name = optarg;
+            }
+            /* fall through */
         case 'r':
             nbdflags |= NBD_FLAG_READ_ONLY;
             flags &= ~BDRV_O_RDWR;
@@ -581,6 +603,22 @@ int main(int argc, char **argv)
             error_get_pretty(local_err));
     }
 
+    if (sn_opts) {
+        ret = bdrv_snapshot_load_tmp(bs,
+                                     qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
+                                     qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
+                                     &local_err);
+    } else if (sn_id_or_name) {
+        ret = bdrv_snapshot_load_tmp_by_id_or_name(bs, sn_id_or_name,
+                                                   &local_err);
+    }
+    if (ret < 0) {
+        errno = -ret;
+        err(EXIT_FAILURE,
+            "Failed to load snapshot: %s",
+            error_get_pretty(local_err));
+    }
+
     fd_size = bdrv_getlength(bs);
 
     if (partition != -1) {
@@ -641,6 +679,10 @@ int main(int argc, char **argv)
         unlink(sockpath);
     }
 
+    if (sn_opts) {
+        qemu_opts_del(sn_opts);
+    }
+
     if (device) {
         void *ret;
         pthread_join(client_thread, &ret);
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 6055ec6..5b55f76 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -27,7 +27,13 @@ Export QEMU disk image using NBD protocol.
 @item -P, --partition=@var{num}
   only expose partition @var{num}
 @item -s, --snapshot
-  use snapshot file
+  use @var{filename} as an external snapshot, create a temporary
+  file with backing_file=@var{filename}, redirect the write to
+  the temporary one
+@item -l, --load-snapshot=@var{snapshot_param}
+  load an internal snapshot inside @var{filename} and export it
+  as an read-only device, @var{snapshot_param} format is
+  'snapshot.id=[ID],snapshot.name=[NAME]' or '[ID_OR_NAME]'
 @item -n, --nocache
 @itemx --cache=@var{cache}
   set cache mode to be used with the file.  See the documentation of
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH V6 3/6] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 1/6] snapshot: distinguish id and name in load_tmp Wenchao Xia
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 2/6] qemu-nbd: support internal snapshot export Wenchao Xia
@ 2013-11-22  4:27 ` Wenchao Xia
  2013-12-03 13:45   ` Stefan Hajnoczi
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 4/6] qemu-img: add -l for snapshot in convert Wenchao Xia
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Wenchao Xia @ 2013-11-22  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

This case can't run when IMGPROTO=nbd, since it needs to create some
internal snapshot which would fail for EOF write request, even when
TEST_IMG is exported with "-f raw" in common.rc, so set _supported_proto
to file.

_require_command() is changed to tip what util is missing, instead
of printing a blank.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/qemu-iotests/058       |  103 ++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/058.out   |   32 +++++++++++++
 tests/qemu-iotests/check     |    1 +
 tests/qemu-iotests/common.rc |    3 +-
 tests/qemu-iotests/group     |    1 +
 5 files changed, 139 insertions(+), 1 deletions(-)
 create mode 100755 tests/qemu-iotests/058
 create mode 100644 tests/qemu-iotests/058.out

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
new file mode 100755
index 0000000..0b48386
--- /dev/null
+++ b/tests/qemu-iotests/058
@@ -0,0 +1,103 @@
+#!/bin/bash
+#
+# Test export internal snapshot by qemu-nbd.
+#
+# Copyright (C) 2013 IBM, Inc.
+#
+# Based on 029.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=xiawenc@linux.vnet.ibm.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+nbd_snapshot_port=10850
+nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
+
+_export_nbd_snapshot()
+{
+    $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
+    NBD_SNAPSHOT_PID=$!
+    sleep 1
+}
+
+_export_nbd_snapshot1()
+{
+    $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l snapshot.name=$1 &
+    NBD_SNAPSHOT_PID=$!
+    sleep 1
+}
+
+_cleanup()
+{
+    if [ -n "$NBD_SNAPSHOT_PID" ]; then
+        kill $NBD_SNAPSHOT_PID
+    fi
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt qcow2
+_supported_proto file
+_require_command QEMU_NBD
+
+echo
+echo "== preparing image =="
+_make_test_img 64M
+$QEMU_IO -c 'write -P 0xa 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 0xb 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IMG snapshot -c sn1 "$TEST_IMG"
+$QEMU_IO -c 'write -P 0xc 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 0xd 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+_check_test_img
+
+echo
+echo "== verifying the image file with patterns =="
+$QEMU_IO -c 'read -P 0xc 0x1000 0x1000' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xd 0x2000 0x1000' "$TEST_IMG" | _filter_qemu_io
+
+_export_nbd_snapshot sn1
+
+echo
+echo "== verifying the exported snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+
+kill $NBD_SNAPSHOT_PID
+sleep 1
+
+_export_nbd_snapshot1 sn1
+
+echo
+echo "== verifying the exported snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/058.out b/tests/qemu-iotests/058.out
new file mode 100644
index 0000000..cc4b8ca
--- /dev/null
+++ b/tests/qemu-iotests/058.out
@@ -0,0 +1,32 @@
+QA output created by 058
+
+== preparing image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+
+== verifying the image file with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the exported snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the exported snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index f5f328f..f879474 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -161,6 +161,7 @@ cat <<EOF
 QEMU          -- $QEMU
 QEMU_IMG      -- $QEMU_IMG
 QEMU_IO       -- $QEMU_IO
+QEMU_NBD      -- $QEMU_NBD
 IMGFMT        -- $FULL_IMGFMT_DETAILS
 IMGPROTO      -- $FULL_IMGPROTO_DETAILS
 PLATFORM      -- $FULL_HOST_DETAILS
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 7f62457..9e9979a 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -405,7 +405,8 @@ _unsupported_qemu_io_options()
 #
 _require_command()
 {
-    [ -x "$1" ] || _notrun "$1 utility required, skipped this test"
+    eval c=\$$1
+    [ -x "$c" ] || _notrun "$1 utility required, skipped this test"
 }
 
 _full_imgfmt_details()
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index b63b18c..bd6acb9 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -64,6 +64,7 @@
 055 rw auto
 056 rw auto backing
 057 rw auto
+058 rw auto
 059 rw auto
 060 rw auto
 061 rw auto
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH V6 4/6] qemu-img: add -l for snapshot in convert
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
                   ` (2 preceding siblings ...)
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 3/6] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
@ 2013-11-22  4:27 ` Wenchao Xia
  2013-12-03 11:02   ` Stefan Hajnoczi
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 5/6] qemu-iotests: add test for snapshot in qemu-img convert Wenchao Xia
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 14+ messages in thread
From: Wenchao Xia @ 2013-11-22  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

Now qemu-img convert have similar options as qemu-nbd for internal
snapshot.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 qemu-img-cmds.hx |    2 +-
 qemu-img.c       |   44 +++++++++++++++++++++++++++++++++++---------
 qemu-img.texi    |   12 ++++++++----
 3 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index da1d965..9a8153b 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -34,7 +34,7 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
+    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
 @item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
diff --git a/qemu-img.c b/qemu-img.c
index a73beb5..ee940c2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -93,6 +93,11 @@ static void help(void)
            "  'options' is a comma separated list of format specific options in a\n"
            "    name=value format. Use -o ? for an overview of the options supported by the\n"
            "    used format\n"
+           "  'snapshot_param' is param used for internal snapshot, format\n"
+           "    is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
+           "    '[ID_OR_NAME]'\n"
+           "  'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
+           "    instead\n"
            "  '-c' indicates that target image must be compressed (qcow format only)\n"
            "  '-u' enables unsafe rebasing. It is assumed that old and new backing file\n"
            "       match exactly. The image doesn't need a working backing file before\n"
@@ -1140,6 +1145,7 @@ static int img_convert(int argc, char **argv)
     int min_sparse = 8; /* Need at least 4k of zeros for sparse detection */
     bool quiet = false;
     Error *local_err = NULL;
+    QemuOpts *sn_opts = NULL;
 
     fmt = NULL;
     out_fmt = "raw";
@@ -1148,7 +1154,7 @@ static int img_convert(int argc, char **argv)
     compress = 0;
     skip_create = 0;
     for(;;) {
-        c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qn");
+        c = getopt(argc, argv, "f:O:B:s:hce6o:pS:t:qnl:");
         if (c == -1) {
             break;
         }
@@ -1183,6 +1189,18 @@ static int img_convert(int argc, char **argv)
         case 's':
             snapshot_name = optarg;
             break;
+        case 'l':
+            if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
+                sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
+                if (!sn_opts) {
+                    error_report("Failed in parsing snapshot param '%s'",
+                                 optarg);
+                    return 1;
+                }
+            } else {
+                snapshot_name = optarg;
+            }
+            break;
         case 'S':
         {
             int64_t sval;
@@ -1254,7 +1272,12 @@ static int img_convert(int argc, char **argv)
         total_sectors += bs_sectors;
     }
 
-    if (snapshot_name != NULL) {
+    if (sn_opts) {
+        ret = bdrv_snapshot_load_tmp(bs[0],
+                                     qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
+                                     qemu_opt_get(sn_opts, SNAPSHOT_OPT_NAME),
+                                     &local_err);
+    } else if (snapshot_name != NULL) {
         if (bs_n > 1) {
             error_report("No support for concatenating multiple snapshot");
             ret = -1;
@@ -1262,13 +1285,13 @@ static int img_convert(int argc, char **argv)
         }
 
         bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
-        if (error_is_set(&local_err)) {
-            error_report("Failed to load snapshot: %s",
-                         error_get_pretty(local_err));
-            error_free(local_err);
-            ret = -1;
-            goto out;
-        }
+    }
+    if (error_is_set(&local_err)) {
+        error_report("Failed to load snapshot: %s",
+                     error_get_pretty(local_err));
+        error_free(local_err);
+        ret = -1;
+        goto out;
     }
 
     /* Find driver and parse its options */
@@ -1559,6 +1582,9 @@ out:
     free_option_parameters(create_options);
     free_option_parameters(param);
     qemu_vfree(buf);
+    if (sn_opts) {
+        qemu_opts_del(sn_opts);
+    }
     if (out_bs) {
         bdrv_unref(out_bs);
     }
diff --git a/qemu-img.texi b/qemu-img.texi
index 768054e..fbdbfa7 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -46,7 +46,11 @@ is the destination disk image filename
 is a comma separated list of format specific options in a
 name=value format. Use @code{-o ?} for an overview of the options supported
 by the used format or see the format descriptions below for details.
-
+@item snapshot_param
+is param used for internal snapshot, format is
+'snapshot.id=[ID],snapshot.name=[NAME]' or '[ID_OR_NAME]'
+@item snapshot_id_or_name
+is deprecated, use snapshot_param instead
 
 @item -c
 indicates that target image must be compressed (qcow format only)
@@ -179,10 +183,10 @@ Error on reading data
 
 @end table
 
-@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 
-Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to disk image @var{output_filename}
-using format @var{output_fmt}. It can be optionally compressed (@code{-c}
+Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated)
+to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
 option) or use any format specific options like encryption (@code{-o} option).
 
 Only the formats @code{qcow} and @code{qcow2} support compression. The
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH V6 5/6] qemu-iotests: add test for snapshot in qemu-img convert
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
                   ` (3 preceding siblings ...)
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 4/6] qemu-img: add -l for snapshot in convert Wenchao Xia
@ 2013-11-22  4:27 ` Wenchao Xia
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 6/6] qemu-nbd: add doc for option -f Wenchao Xia
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-11-22  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/qemu-iotests/058     |   19 ++++++++++++++++++-
 tests/qemu-iotests/058.out |   12 ++++++++++++
 2 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
index 0b48386..19d8a8c 100755
--- a/tests/qemu-iotests/058
+++ b/tests/qemu-iotests/058
@@ -1,6 +1,6 @@
 #!/bin/bash
 #
-# Test export internal snapshot by qemu-nbd.
+# Test export internal snapshot by qemu-nbd, convert it by qemu-img.
 #
 # Copyright (C) 2013 IBM, Inc.
 #
@@ -33,6 +33,8 @@ status=1	# failure is the default!
 nbd_snapshot_port=10850
 nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
 
+converted_image=$TEST_IMG.converted
+
 _export_nbd_snapshot()
 {
     $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
@@ -53,6 +55,7 @@ _cleanup()
         kill $NBD_SNAPSHOT_PID
     fi
     _cleanup_test_img
+    rm -f "$converted_image"
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -97,6 +100,20 @@ echo "== verifying the exported snapshot with patterns =="
 $QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
 $QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$nbd_snapshot_img" | _filter_qemu_io
 
+$QEMU_IMG convert "$TEST_IMG" -l sn1 -O qcow2 "$converted_image"
+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$converted_image" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$converted_image" | _filter_qemu_io
+
+$QEMU_IMG convert "$TEST_IMG" -l snapshot.name=sn1 -O qcow2 "$converted_image"
+
+echo
+echo "== verifying the converted snapshot with patterns =="
+$QEMU_IO -c 'read -P 0xa 0x1000 0x1000' "$converted_image" | _filter_qemu_io
+$QEMU_IO -c 'read -P 0xb 0x2000 0x1000' "$converted_image" | _filter_qemu_io
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/058.out b/tests/qemu-iotests/058.out
index cc4b8ca..a8381b9 100644
--- a/tests/qemu-iotests/058.out
+++ b/tests/qemu-iotests/058.out
@@ -29,4 +29,16 @@ read 4096/4096 bytes at offset 4096
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 read 4096/4096 bytes at offset 8192
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== verifying the converted snapshot with patterns ==
+read 4096/4096 bytes at offset 4096
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 8192
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH V6 6/6] qemu-nbd: add doc for option -f
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
                   ` (4 preceding siblings ...)
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 5/6] qemu-iotests: add test for snapshot in qemu-img convert Wenchao Xia
@ 2013-11-22  4:27 ` Wenchao Xia
  2013-11-29  8:54 ` [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
  2013-12-03 13:46 ` Stefan Hajnoczi
  7 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-11-22  4:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 qemu-nbd.c    |    1 +
 qemu-nbd.texi |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index fe6053c..136e8c9 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -79,6 +79,7 @@ static void usage(const char *name)
 #endif
 "\n"
 "Block device options:\n"
+"  -f, --format=FORMAT  set image format (raw, qcow2, ...)\n"
 "  -r, --read-only      export read-only\n"
 "  -s, --snapshot       use FILE as an external snapshot, create a temporary\n"
 "                       file with backing_file=FILE, redirect the write to\n"
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 5b55f76..0a7e013 100644
--- a/qemu-nbd.texi
+++ b/qemu-nbd.texi
@@ -22,6 +22,8 @@ Export QEMU disk image using NBD protocol.
   interface to bind to (default @samp{0.0.0.0})
 @item -k, --socket=@var{path}
   Use a unix socket with path @var{path}
+@item -f, --format=@var{format}
+  Set image format as @var{format}
 @item -r, --read-only
   export read-only
 @item -P, --partition=@var{num}
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
                   ` (5 preceding siblings ...)
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 6/6] qemu-nbd: add doc for option -f Wenchao Xia
@ 2013-11-29  8:54 ` Wenchao Xia
  2013-12-03  3:33   ` Wenchao Xia
  2013-12-03 13:46 ` Stefan Hajnoczi
  7 siblings, 1 reply; 14+ messages in thread
From: Wenchao Xia @ 2013-11-29  8:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

> v6:
>    Address Kevin's comments:
>    1: typo fix, remove device and snapshot info in error message.
>    2: use strstart().
>    3: use _require_command(), limit proto to file, since when proto=nbd
> it can't work. also changed _require_command() to tip better.
>    4: use strstart().
>    6: new patch, since I found the doc missing in debugging.
> 
  Kevin, I found there is exsiting _require_command() in common.rc, and
snapshot create can't work when PROTO=NBD for EOF write request, so
limited the work condition to file and added _require_command QEMU_NBD.
Could u take a look at it?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd
  2013-11-29  8:54 ` [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
@ 2013-12-03  3:33   ` Wenchao Xia
  0 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-12-03  3:33 UTC (permalink / raw)
  To: qemu-devel; +Cc: kwolf, jcody, stefanha, pbonzini, Wenchao Xia

于 2013/11/29 16:54, Wenchao Xia 写道:
>> v6:
>>     Address Kevin's comments:
>>     1: typo fix, remove device and snapshot info in error message.
>>     2: use strstart().
>>     3: use _require_command(), limit proto to file, since when proto=nbd
>> it can't work. also changed _require_command() to tip better.
>>     4: use strstart().
>>     6: new patch, since I found the doc missing in debugging.
>>
>    Kevin, I found there is exsiting _require_command() in common.rc, and
> snapshot create can't work when PROTO=NBD for EOF write request, so
> limited the work condition to file and added _require_command QEMU_NBD.
> Could u take a look at it?
> 
> 
Hello, is this version OK to be merged?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH V6 4/6] qemu-img: add -l for snapshot in convert
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 4/6] qemu-img: add -l for snapshot in convert Wenchao Xia
@ 2013-12-03 11:02   ` Stefan Hajnoczi
  2013-12-04  2:50     ` Wenchao Xia
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Hajnoczi @ 2013-12-03 11:02 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, pbonzini, jcody, qemu-devel, stefanha

On Fri, Nov 22, 2013 at 12:27:10PM +0800, Wenchao Xia wrote:
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index da1d965..9a8153b 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -34,7 +34,7 @@ STEXI
>  ETEXI
>  
>  DEF("convert", img_convert,
> -    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
> +    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
>  STEXI
>  @item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}

Does this line need to be updated too?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH V6 3/6] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
  2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 3/6] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
@ 2013-12-03 13:45   ` Stefan Hajnoczi
  2013-12-04  2:52     ` Wenchao Xia
  0 siblings, 1 reply; 14+ messages in thread
From: Stefan Hajnoczi @ 2013-12-03 13:45 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, pbonzini, jcody, qemu-devel, stefanha

On Fri, Nov 22, 2013 at 12:27:09PM +0800, Wenchao Xia wrote:
> +nbd_snapshot_port=10850
> +nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
> +
> +_export_nbd_snapshot()
> +{
> +    $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &

Please use a UNIX domain socket so that multiple qemu-iotests instances
can run on the same host in parallel.  The socket should be in the
scratch directory just like the temporary image files.

> +    NBD_SNAPSHOT_PID=$!
> +    sleep 1

qemu-nbd might not start within 1 second on a heavily loaded host.

To make this test case reliable please use a loop (e.g. wait for UNIX
domain socket to appear for up to 30 seconds).

If you feel qemu-nbd doesn't offer a good interface for reliable
startup, feel free to extend it.  Test cases must be reliable and
ideally shouldn't waste time sleeping.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd
  2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
                   ` (6 preceding siblings ...)
  2013-11-29  8:54 ` [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
@ 2013-12-03 13:46 ` Stefan Hajnoczi
  7 siblings, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2013-12-03 13:46 UTC (permalink / raw)
  To: Wenchao Xia; +Cc: kwolf, pbonzini, jcody, qemu-devel, stefanha

On Fri, Nov 22, 2013 at 12:27:06PM +0800, Wenchao Xia wrote:
> This series allow user to read internal snapshot's contents without qemu-img
> convert.
> 
> V2:
>   Address Stefan's comments:
>   02: add 'fall through' comments in the case statement.
>   03: add doc about the difference of internal snapshot and backing chain
> snapshot, which is used in previous '--snapshot' parameter.
>   Other:
>   01,04: rebased on upstream with conflict resolved.
> 
> v3:
>   Address Paolo's comments:
>   02: add parameter "-l snapshot_id_or_name", rename options
> snapshot-load to load-snapshot, use QemuOpts.
>   03: rename snapshot-load to load-snapshot.
>   04: related change to test both -l and -L case.
>   05-07: add similar parameter for qemu-img convert.
>   Other:
>   01: foldered original snapshot logic into function
> bdrv_snapshot_load_tmp_by_id_or_name(), since multiple
> caller present in this version. Refined error message from
> ", reason: %s" to ": %s".
>   02: Refined error message from ", reason: %s" to ": %s".
>   03: Rename PARAM to SNAPSHOT_PARAM.
> 
> v4:
>   Address Eric's comments:
>   01: typo fix.
>   02: squashed patch. Use only one option -l and distiguish
> the cases by the starting string.
>   03: remove 'eval', remove '_supported_os Linux', remove the
> comments that have typo.
>   04: squashed patch. Add only one option -l and distiguish
> the cases by the starting string.
>   05: remove indentation.
>   Address Paolo's comments:
>   02: Use only one option -l and distiguish the cases by
> the starting string.
>   04: Use only one option -l and distiguish the cases by
> the starting string, mark old -s as deprecated, added doc
> for them.
> 
> v5:
>   Address Jeff's comments:
>   Quote image name in test cases.
> 
> v6:
>   Address Kevin's comments:
>   1: typo fix, remove device and snapshot info in error message.
>   2: use strstart().
>   3: use _require_command(), limit proto to file, since when proto=nbd
> it can't work. also changed _require_command() to tip better.
>   4: use strstart().
>   6: new patch, since I found the doc missing in debugging.
> 
> Wenchao Xia (6):
>   1 snapshot: distinguish id and name in load_tmp
>   2 qemu-nbd: support internal snapshot export
>   3 qemu-iotests: add 058 internal snapshot export with qemu-nbd case
>   4 qemu-img: add -l for snapshot in convert
>   5 qemu-iotests: add test for snapshot in qemu-img convert
>   6 qemu-nbd: add doc for option -f
> 
>  block/qcow2-snapshot.c       |   10 +++-
>  block/qcow2.h                |    5 ++-
>  block/snapshot.c             |   77 ++++++++++++++++++++++++++-
>  include/block/block_int.h    |    4 +-
>  include/block/snapshot.h     |   15 +++++-
>  qemu-img-cmds.hx             |    2 +-
>  qemu-img.c                   |   44 +++++++++++++---
>  qemu-img.texi                |   12 +++--
>  qemu-nbd.c                   |   47 ++++++++++++++++-
>  qemu-nbd.texi                |   10 +++-
>  tests/qemu-iotests/058       |  120 ++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/058.out   |   44 +++++++++++++++
>  tests/qemu-iotests/check     |    1 +
>  tests/qemu-iotests/common.rc |    3 +-
>  tests/qemu-iotests/group     |    1 +
>  15 files changed, 371 insertions(+), 24 deletions(-)
>  create mode 100755 tests/qemu-iotests/058
>  create mode 100644 tests/qemu-iotests/058.out

I'm happy overall but the test case must be reliable.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH V6 4/6] qemu-img: add -l for snapshot in convert
  2013-12-03 11:02   ` Stefan Hajnoczi
@ 2013-12-04  2:50     ` Wenchao Xia
  0 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-12-04  2:50 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kwolf, pbonzini, jcody, qemu-devel, stefanha

于 2013/12/3 19:02, Stefan Hajnoczi 写道:
> On Fri, Nov 22, 2013 at 12:27:10PM +0800, Wenchao Xia wrote:
>> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
>> index da1d965..9a8153b 100644
>> --- a/qemu-img-cmds.hx
>> +++ b/qemu-img-cmds.hx
>> @@ -34,7 +34,7 @@ STEXI
>>   ETEXI
>>
>>   DEF("convert", img_convert,
>> -    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
>> +    "convert [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] filename [filename2 [...]] output_filename")
>>   STEXI
>>   @item convert [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} [@var{filename2} [...]] @var{output_filename}
>
> Does this line need to be updated too?
>
   Yes, thanks for catching that.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH V6 3/6] qemu-iotests: add 058 internal snapshot export with qemu-nbd case
  2013-12-03 13:45   ` Stefan Hajnoczi
@ 2013-12-04  2:52     ` Wenchao Xia
  0 siblings, 0 replies; 14+ messages in thread
From: Wenchao Xia @ 2013-12-04  2:52 UTC (permalink / raw)
  To: Stefan Hajnoczi; +Cc: kwolf, pbonzini, jcody, qemu-devel, stefanha

于 2013/12/3 21:45, Stefan Hajnoczi 写道:
> On Fri, Nov 22, 2013 at 12:27:09PM +0800, Wenchao Xia wrote:
>> +nbd_snapshot_port=10850
>> +nbd_snapshot_img="nbd:127.0.0.1:$nbd_snapshot_port"
>> +
>> +_export_nbd_snapshot()
>> +{
>> +    $QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port "$TEST_IMG" -l $1 &
>
> Please use a UNIX domain socket so that multiple qemu-iotests instances
> can run on the same host in parallel.  The socket should be in the
> scratch directory just like the temporary image files.
>
>> +    NBD_SNAPSHOT_PID=$!
>> +    sleep 1
>
> qemu-nbd might not start within 1 second on a heavily loaded host.
>
> To make this test case reliable please use a loop (e.g. wait for UNIX
> domain socket to appear for up to 30 seconds).
>
> If you feel qemu-nbd doesn't offer a good interface for reliable
> startup, feel free to extend it.  Test cases must be reliable and
> ideally shouldn't waste time sleeping.
>
   I will improve this case.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2013-12-04  2:52 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-22  4:27 [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 1/6] snapshot: distinguish id and name in load_tmp Wenchao Xia
2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 2/6] qemu-nbd: support internal snapshot export Wenchao Xia
2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 3/6] qemu-iotests: add 058 internal snapshot export with qemu-nbd case Wenchao Xia
2013-12-03 13:45   ` Stefan Hajnoczi
2013-12-04  2:52     ` Wenchao Xia
2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 4/6] qemu-img: add -l for snapshot in convert Wenchao Xia
2013-12-03 11:02   ` Stefan Hajnoczi
2013-12-04  2:50     ` Wenchao Xia
2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 5/6] qemu-iotests: add test for snapshot in qemu-img convert Wenchao Xia
2013-11-22  4:27 ` [Qemu-devel] [PATCH V6 6/6] qemu-nbd: add doc for option -f Wenchao Xia
2013-11-29  8:54 ` [Qemu-devel] [PATCH V6 0/6] export internal snapshot by qemu-nbd Wenchao Xia
2013-12-03  3:33   ` Wenchao Xia
2013-12-03 13:46 ` 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).