[Qemu-devel] [PATCH V2 0/4] export internal snapshot by qemu-nbd

[Qemu-devel] [PATCH V2 0/4] export internal snapshot by qemu-nbd

[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.

Wenchao Xia (4):
  1 snapshot: distinguish id and name in load_tmp
  2 qemu-nbd: support internal snapshot export
  3 qemu-nbd: add doc for internal snapshot export
  4 qemu-iotests: add 058 internal snapshot export with qemu-nbd case

 block/qcow2-snapshot.c     |   16 +++++++-
 block/qcow2.h              |    5 ++-
 block/snapshot.c           |   37 ++++++++++++++++++-
 include/block/block_int.h  |    4 ++-
 include/block/snapshot.h   |    4 ++-
 qemu-img.c                 |   14 ++++++-
 qemu-nbd.c                 |   62 ++++++++++++++++++++++++++++++-
 qemu-nbd.texi              |    8 ++++-
 tests/qemu-iotests/058     |   87 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/058.out |   26 +++++++++++++
 tests/qemu-iotests/group   |    1 +
 11 files changed, 252 insertions(+), 12 deletions(-)
 create mode 100755 tests/qemu-iotests/058
 create mode 100644 tests/qemu-iotests/058.out

[Qemu-devel] [PATCH V2 1/4] snapshot: distinguish id and name in load_tmp

[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 call the function twice to keep
old search logic.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qcow2-snapshot.c    |   16 ++++++++++++++--
 block/qcow2.h             |    5 ++++-
 block/snapshot.c          |   37 +++++++++++++++++++++++++++++++++++--
 include/block/block_int.h |    4 +++-
 include/block/snapshot.h  |    4 +++-
 qemu-img.c                |   14 ++++++++++++--
 6 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 7d14420..fe8f0eb 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -669,7 +669,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;
@@ -677,12 +680,17 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_name)
     uint64_t *new_l1_table;
     int new_l1_bytes;
     int ret;
+    const char *device = bdrv_get_device_name(bs);
 
     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 a snapshot with ID '%s' and name '%s' "
+                   "on device '%s'",
+                   STR_OR_NULL(snapshot_id), STR_OR_NULL(name), device);
         return -ENOENT;
     }
     sn = &s->snapshots[snapshot_index];
@@ -693,6 +701,10 @@ 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 with ID '%s' and name "
+                   "'%s' on device '%s'",
+                   sn->id_str, sn->name, device);
         g_free(new_l1_table);
         return ret;
     }
diff --git a/block/qcow2.h b/block/qcow2.h
index c90e5d6..12cfeaf 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -472,7 +472,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..0dab05d 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -265,18 +265,51 @@ 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 -ENINVAL.
+ *
+ * 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 one matching @id and
+ * @name, return -ENOENT. If @bs do not support parameter @snapshot_id or
+ * @name, return -EINVAL. 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;
+    const char *device = bdrv_get_device_name(bs);
     if (!drv) {
+        error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
         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 '%s' is not readonly", device);
         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, device,
+              "temporarily load internal snapshot");
     return -ENOTSUP;
 }
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 3eeb6fe..554cadd 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -160,7 +160,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);
 
     int (*bdrv_save_vmstate)(BlockDriverState *bs, QEMUIOVector *qiov,
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index 012bf22..e50c3bf 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -61,5 +61,7 @@ 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);
 #endif
diff --git a/qemu-img.c b/qemu-img.c
index 926f0a0..d1f0e11 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1260,8 +1260,18 @@ 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");
+
+        ret = bdrv_snapshot_load_tmp(bs[0], snapshot_name, NULL, &local_err);
+        if (ret == -ENOENT || ret == -EINVAL) {
+            error_free(local_err);
+            local_err = NULL;
+            ret = bdrv_snapshot_load_tmp(bs[0], NULL, snapshot_name,
+                                         &local_err);
+        }
+        if (ret < 0) {
+            error_report("Failed to load snapshot, reason: %s",
+                         error_get_pretty(local_err));
+            error_free(local_err);
             ret = -1;
             goto out;
         }
-- 
1.7.1

[Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export

[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>
---
 qemu-nbd.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index c26c98e..e450d04 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>
@@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
     }
 }
 
+#define SNAPSHOT_OPT_ID         "id"
+#define SNAPSHOT_OPT_NAME       "name"
+
+static QEMUOptionParameter snapshot_options[] = {
+    {
+        .name = SNAPSHOT_OPT_ID,
+        .type = OPT_STRING,
+        .help = "snapshot id"
+    },
+    {
+        .name = SNAPSHOT_OPT_NAME,
+        .type = OPT_STRING,
+        .help = "snapshot name"
+    },
+    { NULL }
+};
+
 int main(int argc, char **argv)
 {
     BlockDriverState *bs;
@@ -315,7 +333,10 @@ 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";
+    QEMUOptionParameter *sn_param = NULL;
+    const QEMUOptionParameter *sn_param_id, *sn_param_name;
+    const char *sn_id = NULL, *sn_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 +349,7 @@ int main(int argc, char **argv)
         { "connect", 1, NULL, 'c' },
         { "disconnect", 0, NULL, 'd' },
         { "snapshot", 0, NULL, 's' },
+        { "snapshot-load", 1, NULL, 'l' },
         { "nocache", 0, NULL, 'n' },
         { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
 #ifdef CONFIG_LINUX_AIO
@@ -428,6 +450,14 @@ int main(int argc, char **argv)
                 errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
             }
             break;
+        case 'l':
+            sn_param = parse_option_parameters(optarg,
+                                               snapshot_options, sn_param);
+            if (!sn_param) {
+                errx(EXIT_FAILURE,
+                     "Invalid snapshot-load options '%s'", optarg);
+            }
+            /* fall through */
         case 'r':
             nbdflags |= NBD_FLAG_READ_ONLY;
             flags &= ~BDRV_O_RDWR;
@@ -581,6 +611,24 @@ int main(int argc, char **argv)
             error_get_pretty(local_err));
     }
 
+    if (sn_param) {
+        sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
+        sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
+        if (sn_param_id) {
+            sn_id = sn_param_id->value.s;
+        }
+        if (sn_param_name) {
+            sn_name = sn_param_name->value.s;
+        }
+        ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name, &local_err);
+        if (ret < 0) {
+            errno = -ret;
+            err(EXIT_FAILURE,
+                "Failed to load snapshot, reason:\n%s",
+                error_get_pretty(local_err));
+        }
+    }
+
     fd_size = bdrv_getlength(bs);
 
     if (partition != -1) {
@@ -641,6 +689,10 @@ int main(int argc, char **argv)
         unlink(sockpath);
     }
 
+    if (sn_param) {
+        free_option_parameters(sn_param);
+    }
+
     if (device) {
         void *ret;
         pthread_join(client_thread, &ret);
-- 
1.7.1

[Qemu-devel] [PATCH V2 3/4] qemu-nbd: add doc for internal snapshot export

[Wenchao Xia]

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

diff --git a/qemu-nbd.c b/qemu-nbd.c
index e450d04..8cb4bf1 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -80,7 +80,13 @@ 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, --snapshot-load=PARAM\n"
+"                       load an internal snapshot inside FILE and export it\n"
+"                       as an read-only device, PARAM format is\n"
+"                       'id=[ID],name=[NAME]'\n"
 "  -n, --nocache        disable host cache\n"
 "      --cache=MODE     set cache mode (none, writeback, ...)\n"
 #ifdef CONFIG_LINUX_AIO
diff --git a/qemu-nbd.texi b/qemu-nbd.texi
index 6055ec6..69e9e9a 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, --snapshot-load=@var{param}
+  load an internal snapshot inside @var{filename} and export it
+  as an read-only device, @var{param} format is
+  'id=[ID],name=[NAME]'
 @item -n, --nocache
 @itemx --cache=@var{cache}
   set cache mode to be used with the file.  See the documentation of
-- 
1.7.1

[Qemu-devel] [PATCH V2 4/4] qemu-iotests: add 058 internal snapshot export with qemu-nbd case

[Wenchao Xia]

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 tests/qemu-iotests/058     |   87 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/058.out |   26 +++++++++++++
 tests/qemu-iotests/group   |    1 +
 3 files changed, 114 insertions(+), 0 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..301ef1f
--- /dev/null
+++ b/tests/qemu-iotests/058
@@ -0,0 +1,87 @@
+#!/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()
+{
+    eval "$QEMU_NBD -v -t -b 127.0.0.1 -p $nbd_snapshot_port $TEST_IMG -l 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
+
+# Any format supporting intenal snapshots
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+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 foo $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 foo
+
+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..b174f3b
--- /dev/null
+++ b/tests/qemu-iotests/058.out
@@ -0,0 +1,26 @@
+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)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1ad02e5..2793e1d 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

Re: [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export

[Paolo Bonzini]

Il 22/09/2013 11:39, Wenchao Xia ha scritto:
> 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>
> ---
>  qemu-nbd.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 53 insertions(+), 1 deletions(-)
> 
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c26c98e..e450d04 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>
> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>      }
>  }
>  
> +#define SNAPSHOT_OPT_ID         "id"
> +#define SNAPSHOT_OPT_NAME       "name"
> +
> +static QEMUOptionParameter snapshot_options[] = {
> +    {
> +        .name = SNAPSHOT_OPT_ID,
> +        .type = OPT_STRING,
> +        .help = "snapshot id"
> +    },
> +    {
> +        .name = SNAPSHOT_OPT_NAME,
> +        .type = OPT_STRING,
> +        .help = "snapshot name"
> +    },
> +    { NULL }
> +};

I think whatever mechanism you use here to pick a snapshot id or name
should be implemented in qemu-img too.

Also, I think QEMUOptionParameter is being phased out.

>  int main(int argc, char **argv)
>  {
>      BlockDriverState *bs;
> @@ -315,7 +333,10 @@ 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";
> +    QEMUOptionParameter *sn_param = NULL;
> +    const QEMUOptionParameter *sn_param_id, *sn_param_name;
> +    const char *sn_id = NULL, *sn_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 +349,7 @@ int main(int argc, char **argv)
>          { "connect", 1, NULL, 'c' },
>          { "disconnect", 0, NULL, 'd' },
>          { "snapshot", 0, NULL, 's' },
> +        { "snapshot-load", 1, NULL, 'l' },

Please call this option "load-snapshot".

Paolo

>          { "nocache", 0, NULL, 'n' },
>          { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>  #ifdef CONFIG_LINUX_AIO
> @@ -428,6 +450,14 @@ int main(int argc, char **argv)
>                  errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>              }
>              break;
> +        case 'l':
> +            sn_param = parse_option_parameters(optarg,
> +                                               snapshot_options, sn_param);
> +            if (!sn_param) {
> +                errx(EXIT_FAILURE,
> +                     "Invalid snapshot-load options '%s'", optarg);
> +            }
> +            /* fall through */
>          case 'r':
>              nbdflags |= NBD_FLAG_READ_ONLY;
>              flags &= ~BDRV_O_RDWR;
> @@ -581,6 +611,24 @@ int main(int argc, char **argv)
>              error_get_pretty(local_err));
>      }
>  
> +    if (sn_param) {
> +        sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
> +        sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
> +        if (sn_param_id) {
> +            sn_id = sn_param_id->value.s;
> +        }
> +        if (sn_param_name) {
> +            sn_name = sn_param_name->value.s;
> +        }
> +        ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name, &local_err);
> +        if (ret < 0) {
> +            errno = -ret;
> +            err(EXIT_FAILURE,
> +                "Failed to load snapshot, reason:\n%s",
> +                error_get_pretty(local_err));
> +        }
> +    }
> +
>      fd_size = bdrv_getlength(bs);
>  
>      if (partition != -1) {
> @@ -641,6 +689,10 @@ int main(int argc, char **argv)
>          unlink(sockpath);
>      }
>  
> +    if (sn_param) {
> +        free_option_parameters(sn_param);
> +    }
> +
>      if (device) {
>          void *ret;
>          pthread_join(client_thread, &ret);
> 

Re: [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export

[Wenchao Xia]

于 2013/9/23 18:25, Paolo Bonzini 写道:
> Il 22/09/2013 11:39, Wenchao Xia ha scritto:
>> 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>
>> ---
>>   qemu-nbd.c |   54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 files changed, 53 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>> index c26c98e..e450d04 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>
>> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>>       }
>>   }
>>
>> +#define SNAPSHOT_OPT_ID         "id"
>> +#define SNAPSHOT_OPT_NAME       "name"
>> +
>> +static QEMUOptionParameter snapshot_options[] = {
>> +    {
>> +        .name = SNAPSHOT_OPT_ID,
>> +        .type = OPT_STRING,
>> +        .help = "snapshot id"
>> +    },
>> +    {
>> +        .name = SNAPSHOT_OPT_NAME,
>> +        .type = OPT_STRING,
>> +        .help = "snapshot name"
>> +    },
>> +    { NULL }
>> +};
> I think whatever mechanism you use here to pick a snapshot id or name
> should be implemented in qemu-img too.
qemu-img already pick up snapshot by mixed id and name, do you like to 
add a new
interface like the above(Keep old interface untouched for compatiablity)?

> Also, I think QEMUOptionParameter is being phased out.
>
Is QemuOptsList the recommanded method?

>>   int main(int argc, char **argv)
>>   {
>>       BlockDriverState *bs;
>> @@ -315,7 +333,10 @@ 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";
>> +    QEMUOptionParameter *sn_param = NULL;
>> +    const QEMUOptionParameter *sn_param_id, *sn_param_name;
>> +    const char *sn_id = NULL, *sn_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 +349,7 @@ int main(int argc, char **argv)
>>           { "connect", 1, NULL, 'c' },
>>           { "disconnect", 0, NULL, 'd' },
>>           { "snapshot", 0, NULL, 's' },
>> +        { "snapshot-load", 1, NULL, 'l' },
> Please call this option "load-snapshot".
>
OK.


> Paolo
>
>>           { "nocache", 0, NULL, 'n' },
>>           { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>>   #ifdef CONFIG_LINUX_AIO
>> @@ -428,6 +450,14 @@ int main(int argc, char **argv)
>>                   errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>>               }
>>               break;
>> +        case 'l':
>> +            sn_param = parse_option_parameters(optarg,
>> +                                               snapshot_options, sn_param);
>> +            if (!sn_param) {
>> +                errx(EXIT_FAILURE,
>> +                     "Invalid snapshot-load options '%s'", optarg);
>> +            }
>> +            /* fall through */
>>           case 'r':
>>               nbdflags |= NBD_FLAG_READ_ONLY;
>>               flags&= ~BDRV_O_RDWR;
>> @@ -581,6 +611,24 @@ int main(int argc, char **argv)
>>               error_get_pretty(local_err));
>>       }
>>
>> +    if (sn_param) {
>> +        sn_param_id = get_option_parameter(sn_param, SNAPSHOT_OPT_ID);
>> +        sn_param_name = get_option_parameter(sn_param, SNAPSHOT_OPT_NAME);
>> +        if (sn_param_id) {
>> +            sn_id = sn_param_id->value.s;
>> +        }
>> +        if (sn_param_name) {
>> +            sn_name = sn_param_name->value.s;
>> +        }
>> +        ret = bdrv_snapshot_load_tmp(bs, sn_id, sn_name,&local_err);
>> +        if (ret<  0) {
>> +            errno = -ret;
>> +            err(EXIT_FAILURE,
>> +                "Failed to load snapshot, reason:\n%s",
>> +                error_get_pretty(local_err));
>> +        }
>> +    }
>> +
>>       fd_size = bdrv_getlength(bs);
>>
>>       if (partition != -1) {
>> @@ -641,6 +689,10 @@ int main(int argc, char **argv)
>>           unlink(sockpath);
>>       }
>>
>> +    if (sn_param) {
>> +        free_option_parameters(sn_param);
>> +    }
>> +
>>       if (device) {
>>           void *ret;
>>           pthread_join(client_thread,&ret);
>>

Re: [Qemu-devel] [PATCH V2 2/4] qemu-nbd: support internal snapshot export

[Paolo Bonzini]

Il 24/09/2013 04:56, Wenchao Xia ha scritto:
> 于 2013/9/23 18:25, Paolo Bonzini 写道:
>> Il 22/09/2013 11:39, Wenchao Xia ha scritto:
>>> 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>
>>> ---
>>>   qemu-nbd.c |   54
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>   1 files changed, 53 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/qemu-nbd.c b/qemu-nbd.c
>>> index c26c98e..e450d04 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>
>>> @@ -304,6 +305,23 @@ static void nbd_accept(void *opaque)
>>>       }
>>>   }
>>>
>>> +#define SNAPSHOT_OPT_ID         "id"
>>> +#define SNAPSHOT_OPT_NAME       "name"
>>> +
>>> +static QEMUOptionParameter snapshot_options[] = {
>>> +    {
>>> +        .name = SNAPSHOT_OPT_ID,
>>> +        .type = OPT_STRING,
>>> +        .help = "snapshot id"
>>> +    },
>>> +    {
>>> +        .name = SNAPSHOT_OPT_NAME,
>>> +        .type = OPT_STRING,
>>> +        .help = "snapshot name"
>>> +    },
>>> +    { NULL }
>>> +};
>> I think whatever mechanism you use here to pick a snapshot id or name
>> should be implemented in qemu-img too.
> qemu-img already pick up snapshot by mixed id and name, do you like to
> add a new
> interface like the above(Keep old interface untouched for compatiablity)?

Yes, please.  And also implement the "mixed" method here.

>> Also, I think QEMUOptionParameter is being phased out.
>>
> Is QemuOptsList the recommanded method?

Yes.

Paolo