Re: [Qemu-devel] [PATCH V8 10/20] qmp: add interface query-snapshots

[Wenchao Xia]

于 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 <xiaw...@linux.vnet.ibm.com>
---
  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