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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 26/09/2013 02:16, 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>
> ---
>  block/snapshot.c         |   18 ++++++++++++++++++
>  include/block/snapshot.h |    6 ++++++
>  qemu-nbd.c               |   35 ++++++++++++++++++++++++++++++++++-
>  3 files changed, 58 insertions(+), 1 deletions(-)
> 
> diff --git a/block/snapshot.c b/block/snapshot.c
> index 2ae3099..b371c27 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,

Why not just use "id" and "name"?

> +            .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..c524a49 100644
> --- a/include/block/snapshot.h
> +++ b/include/block/snapshot.h
> @@ -27,6 +27,12 @@
>  
>  #include "qemu-common.h"
>  #include "qapi/error.h"
> +#include "qemu/option.h"
> +
> +#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..6588a1f 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>
> @@ -315,7 +316,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:L:";
>      struct option lopt[] = {
>          { "help", 0, NULL, 'h' },
>          { "version", 0, NULL, 'V' },
> @@ -328,6 +331,8 @@ int main(int argc, char **argv)
>          { "connect", 1, NULL, 'c' },
>          { "disconnect", 0, NULL, 'd' },
>          { "snapshot", 0, NULL, 's' },
> +        { "load-snapshot", 1, NULL, 'l' },

Just omit the long option here...

> +        { "load-snapshot1", 1, NULL, 'L' },

... and call this "load-snapshot".

Paolo

>          { "nocache", 0, NULL, 'n' },
>          { "cache", 1, NULL, QEMU_NBD_OPT_CACHE },
>  #ifdef CONFIG_LINUX_AIO
> @@ -428,6 +433,14 @@ int main(int argc, char **argv)
>                  errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg);
>              }
>              break;
> +        case 'l':
> +            sn_id_or_name = optarg;
> +            nbdflags |= NBD_FLAG_READ_ONLY;
> +            flags &= ~BDRV_O_RDWR;
> +            break;
> +        case 'L':
> +            sn_opts = qemu_opts_parse(&internal_snapshot_opts, optarg, 0);
> +            /* fall through */
>          case 'r':
>              nbdflags |= NBD_FLAG_READ_ONLY;
>              flags &= ~BDRV_O_RDWR;
> @@ -581,6 +594,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 +670,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);
>