diff mbox

[V8,10/20] qmp: add interface query-snapshots

Message ID 1362636445-7188-11-git-send-email-xiawenc@linux.vnet.ibm.com
State New
Headers show

Commit Message

Wayne Xia March 7, 2013, 6:07 a.m. UTC
This interface now return valid internal snapshots for whole vm.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 block/qapi.c     |   22 +++++++++++++++++++++
 qapi-schema.json |   14 +++++++++++++
 qmp-commands.hx  |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+), 0 deletions(-)

Comments

Eric Blake March 8, 2013, 11:30 p.m. UTC | #1
On 03/06/2013 11:07 PM, Wenchao Xia wrote:
>   This interface now return valid internal snapshots for whole vm.

s/now return/returns/

> 
> Signed-off-by: Wenchao Xia <xiawenc@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'.

>  ##
> +# @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/


>  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
Wayne Xia March 9, 2013, 4:28 a.m. UTC | #2
于 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 <xiawenc@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.
diff mbox

Patch

diff --git a/block/qapi.c b/block/qapi.c
index 0c3055f..b903dd8 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -14,6 +14,7 @@ 
 #include "block/qapi.h"
 #include "block/snapshot.h"
 #include "block/block_int.h"
+#include "qmp-commands.h"
 
 /*
  * check whether the snapshot is valid for whole vm.
@@ -180,3 +181,24 @@  int bdrv_query_image_info(BlockDriverState *bs,
     *p_info = info;
     return 0;
 }
+
+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;
+    }
+
+    ret = bdrv_query_snapshot_info_list(bs, &list, true, errp);
+    if (ret < 0) {
+        qapi_free_SnapshotInfoList(list);
+        list = NULL;
+    }
+    return list;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index 28b070f..3710495 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -839,6 +839,20 @@ 
 { 'command': 'query-block', 'returns': ['BlockInfo'] }
 
 ##
+# @query-snapshots:
+#
+# Get a list of internal snapshots for whole virtual machine, only valid
+# internal snapshot will be returned, inconsistent ones will be ignored
+#
+# Returns: a list of @SnapshotInfo describing all consistent virtual machine
+#          snapshots
+#
+# Since: 1.5
+##
+{ 'command': 'query-snapshots',
+  'returns': ['SnapshotInfo'] }
+
+##
 # @BlockDeviceStats:
 #
 # Statistics of a virtual block device or a block backing device.
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 95022e2..d505209 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -1743,6 +1743,61 @@  EQMP
     },
 
 SQMP
+query-snapshots
+-----------
+
+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
+               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
+                   vm-clock-sec (json-int)
+
+Example:
+
+-> { "execute": "query-snapshots" }
+<- {
+      "return":[
+         {
+            "id": "1",
+            "name": "snapshot1",
+            "vm-state-size": 0,
+            "date-sec": 10000200,
+            "date-nsec": 12,
+            "vm-clock-sec": 206,
+            "vm-clock-nsec": 30
+         },
+         {
+            "id": "2",
+            "name": "snapshot2",
+            "vm-state-size": 34000000,
+            "date-sec": 13000200,
+            "date-nsec": 32,
+            "vm-clock-sec": 406,
+            "vm-clock-nsec": 31
+         }
+      ]
+   }
+
+EQMP
+
+    {
+        .name       = "query-snapshots",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_snapshots,
+    },
+
+SQMP
 query-blockstats
 ----------------