From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56631) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VU9Ic-000492-NY for qemu-devel@nongnu.org; Thu, 10 Oct 2013 02:00:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VU9IV-0003ZH-Af for qemu-devel@nongnu.org; Thu, 10 Oct 2013 02:00:38 -0400 Received: from e28smtp09.in.ibm.com ([122.248.162.9]:44210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VU9IU-0003Yy-P1 for qemu-devel@nongnu.org; Thu, 10 Oct 2013 02:00:31 -0400 Received: from /spool/local by e28smtp09.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 10 Oct 2013 11:30:19 +0530 Received: from d28relay03.in.ibm.com (d28relay03.in.ibm.com [9.184.220.60]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id 94A46E004A for ; Thu, 10 Oct 2013 11:31:35 +0530 (IST) Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay03.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r9A62oDl35389502 for ; Thu, 10 Oct 2013 11:32:50 +0530 Received: from d28av04.in.ibm.com (localhost [127.0.0.1]) by d28av04.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r9A60Fic011623 for ; Thu, 10 Oct 2013 11:30:15 +0530 Message-ID: <52564275.4050500@linux.vnet.ibm.com> Date: Thu, 10 Oct 2013 14:00:21 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1380154568-5339-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1380154568-5339-3-git-send-email-xiawenc@linux.vnet.ibm.com> <524AF367.3000304@redhat.com> In-Reply-To: <524AF367.3000304@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V3 2/7] qemu-nbd: support internal snapshot export List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: kwolf@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org 于 2013/10/2 0:08, Paolo Bonzini 写道: > 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 >> --- >> 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"? > Later it is used by code: qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID), The macro is used to avoid type it twice in the codes, shouldn't it be used? Another reason not using "id" is because string "id" is treated as special case in opts_parse() so I choosed string "snapshot.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..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 >> #include >> @@ -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 > OK, I will change as: { NULL, 1, NULL, 'l' }, { "load-snapshot", 1, NULL, 'L' }, >> { "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); >> >