From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52208) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UEBOb-0007F0-D1 for qemu-devel@nongnu.org; Fri, 08 Mar 2013 23:28:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UEBOW-0008M5-8X for qemu-devel@nongnu.org; Fri, 08 Mar 2013 23:28:33 -0500 Received: from e28smtp07.in.ibm.com ([122.248.162.7]:41625) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UEBOV-0008Lr-Nc for qemu-devel@nongnu.org; Fri, 08 Mar 2013 23:28:28 -0500 Received: from /spool/local by e28smtp07.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Sat, 9 Mar 2013 09:55:02 +0530 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id E4AC91258023 for ; Sat, 9 Mar 2013 09:59:22 +0530 (IST) Received: from d28av02.in.ibm.com (d28av02.in.ibm.com [9.184.220.64]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r294SJUO26804338 for ; Sat, 9 Mar 2013 09:58:20 +0530 Received: from d28av02.in.ibm.com (loopback [127.0.0.1]) by d28av02.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r294SMv7016879 for ; Sat, 9 Mar 2013 15:28:22 +1100 Message-ID: <513ABA61.5040809@linux.vnet.ibm.com> Date: Sat, 09 Mar 2013 12:28:17 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1362636445-7188-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1362636445-7188-11-git-send-email-xiawenc@linux.vnet.ibm.com> <513A74AE.8030907@redhat.com> In-Reply-To: <513A74AE.8030907@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH V8 10/20] qmp: add interface query-snapshots List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: kwolf@redhat.com, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, armbru@redhat.com, pbonzini@redhat.com 于 2013-3-9 7:30, Eric Blake 写道: > On 03/06/2013 11:07 PM, Wenchao Xia wrote: >> This interface now return valid internal snapshots for whole vm. > > s/now return/returns/ > OK. >> >> Signed-off-by: Wenchao Xia >> --- >> block/qapi.c | 22 +++++++++++++++++++++ >> qapi-schema.json | 14 +++++++++++++ >> qmp-commands.hx | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 91 insertions(+), 0 deletions(-) >> > >> + >> +SnapshotInfoList *qmp_query_snapshots(Error **errp) >> +{ >> + BlockDriverState *bs; >> + SnapshotInfoList *list = NULL; >> + int ret; >> + >> + /* internal snapshot for whole vm */ >> + bs = bdrv_snapshots(); >> + if (!bs) { >> + error_setg(errp, "No available block device supports snapshots\n"); >> + return NULL; >> + } >> + > > list is NULL here, > >> + ret = bdrv_query_snapshot_info_list(bs, &list, true, errp); >> + if (ret < 0) { >> + qapi_free_SnapshotInfoList(list); > > and you documented that bdrv_query_snapshot_info_list leaves list > untouched on error, so why do you have to clean it up? > >> + list = NULL; >> + } >> + return list; > > In fact, you could write this as: > > bdrv_query_snapshot_info_list(bs, &list, true, errp); > return list; > > without needing 'ret'. > Yep, you are right. >> ## >> +# @query-snapshots: >> +# >> +# Get a list of internal snapshots for whole virtual machine, only valid > > s/for/for the/; s/machine, only/machine. Only/ > >> +# internal snapshot will be returned, inconsistent ones will be ignored > > s/snapshot/snapshots/ > OK. > >> SQMP >> +query-snapshots >> +----------- > > Common practice is to make the divider line match the line length of the > line above (you were short by '----') > >> + >> +Show the internal consistent snapshot information >> + >> +Each snapshot is represented by a json-object. The returned value >> +is a json-array of all snapshots >> + >> +Each json-object contain the following: >> + >> +- "id": unique snapshot id (json-string) >> +- "name": internal snapshot name (json-string) >> +- "vm-state-size": size of the VM state in bytes (json-int) >> +- "date-sec": UTC date of the snapshot in seconds (json-int) >> +- "date-nsec": fractional part in nano seconds to be used with > > s/nano seconds/nanoseconds/ > >> + date-sec(json-int) >> +- "vm-clock-sec": VM clock relative to boot in seconds (json-int) >> +- "vm-clock-nsec": fractional part in nano seconds to be used with > > and again > OK. -- Best Regards Wenchao Xia