From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58784) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VU9Ts-00022Q-JU for qemu-devel@nongnu.org; Thu, 10 Oct 2013 02:12:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VU9Tl-0007HO-4Z for qemu-devel@nongnu.org; Thu, 10 Oct 2013 02:12:16 -0400 Received: from e28smtp01.in.ibm.com ([122.248.162.1]:47284) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VU9Tk-0007Gy-F4 for qemu-devel@nongnu.org; Thu, 10 Oct 2013 02:12:09 -0400 Received: from /spool/local by e28smtp01.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 10 Oct 2013 11:42:05 +0530 Received: from d28relay05.in.ibm.com (d28relay05.in.ibm.com [9.184.220.62]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id AABBF1258053 for ; Thu, 10 Oct 2013 11:42:27 +0530 (IST) Received: from d28av04.in.ibm.com (d28av04.in.ibm.com [9.184.220.66]) by d28relay05.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r9A6C1sY45023278 for ; Thu, 10 Oct 2013 11:42:02 +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 r9A6C2kw001665 for ; Thu, 10 Oct 2013 11:42:03 +0530 Message-ID: <52564539.2070401@linux.vnet.ibm.com> Date: Thu, 10 Oct 2013 14:12:09 +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> <52564275.4050500@linux.vnet.ibm.com> In-Reply-To: <52564275.4050500@linux.vnet.ibm.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/10 14:00, Wenchao Xia 写道: > 于 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' }, > From Eric's suggestion, I think simply one item: { "load-snapshot", 1, NULL, 'l' } would be engough to handle both cases. >>> { "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); >>> >> > >