Patchwork [16/18] qapi: Convert info snapshots

login
register
mail settings
Submitter Pavel Hrdina
Date Aug. 15, 2012, 7:41 a.m.
Message ID <699332045f54bbea299eefd50f7198826e638170.1345016001.git.phrdina@redhat.com>
Download mbox | patch
Permalink /patch/177567/
State New
Headers show

Comments

Pavel Hrdina - Aug. 15, 2012, 7:41 a.m.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 hmp.c            | 33 +++++++++++++++++++++++++++++++++
 hmp.h            |  1 +
 monitor.c        |  2 +-
 qapi-schema.json | 34 ++++++++++++++++++++++++++++++++++
 qmp-commands.hx  | 30 ++++++++++++++++++++++++++++++
 savevm.c         | 51 ++++++++++++++++++++++++---------------------------
 sysemu.h         |  2 --
 7 files changed, 123 insertions(+), 30 deletions(-)
Eric Blake - Aug. 16, 2012, 4:36 a.m.
On 08/15/2012 01:41 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  hmp.c            | 33 +++++++++++++++++++++++++++++++++
>  hmp.h            |  1 +
>  monitor.c        |  2 +-
>  qapi-schema.json | 34 ++++++++++++++++++++++++++++++++++
>  qmp-commands.hx  | 30 ++++++++++++++++++++++++++++++
>  savevm.c         | 51 ++++++++++++++++++++++++---------------------------
>  sysemu.h         |  2 --
>  7 files changed, 123 insertions(+), 30 deletions(-)
> 

> @@ -1053,6 +1053,40 @@
>  { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
>  
>  ##
> +# @SnapshotInfo:
> +#

We've got competition here:
https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02961.html

These two patches need to converge into a single design.

> +# Snapshot list.  This structure contains list of snapshots for virtual machine.
> +#
> +# @id: id of the snapshot.
> +#
> +# @tag: human readable tag of the snapshot.
> +#
> +# @vm_size: size of the snapshot in Bytes.

As this is a new QMP command, you should use '-', not '_'.  Benoit's
patch named this @vm-state-size.

> +#
> +# @date: date and time of the snapshot as unix timestamp.
> +#
> +# @vm_clock: time in the guest in nsecs.

For online internal snapshots (those created by savevm), the vm clock
offset makes sense. But for offline snapshots (those created by
'qemu-img snapshot', with no VM state), there is no vm clock offset.
'qemu-img info' lists 00:00:00.000 as a result, but I wonder if it would
be better to leave this field (and vm-state-size) as #optional and omit
them from the JSON for offline snapshots.

And since qemu refuses to load offline snapshots, it might be worth an
additional field in the JSON that says whether a snapshot is online or
offline, to make it easier for the parsing application to determine
whether it can be loaded or must go through qemu-img while the guest is
offline (then again, keeping vm-state-size as non-optional, and checking
for 0 size, somewhat covers this).

> +#
> +# Since:  1.2

1.3 (entire series...)
Pavel Hrdina - Aug. 16, 2012, 10:17 a.m.
On 08/16/2012 06:36 AM, Eric Blake wrote:
> On 08/15/2012 01:41 AM, Pavel Hrdina wrote:
>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>> ---
>>   hmp.c            | 33 +++++++++++++++++++++++++++++++++
>>   hmp.h            |  1 +
>>   monitor.c        |  2 +-
>>   qapi-schema.json | 34 ++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  | 30 ++++++++++++++++++++++++++++++
>>   savevm.c         | 51 ++++++++++++++++++++++++---------------------------
>>   sysemu.h         |  2 --
>>   7 files changed, 123 insertions(+), 30 deletions(-)
>>
>> @@ -1053,6 +1053,40 @@
>>   { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
>>   
>>   ##
>> +# @SnapshotInfo:
>> +#
> We've got competition here:
> https://lists.gnu.org/archive/html/qemu-devel/2012-08/msg02961.html
>
> These two patches need to converge into a single design.
I can use his design.
>> +# Snapshot list.  This structure contains list of snapshots for virtual machine.
>> +#
>> +# @id: id of the snapshot.
>> +#
>> +# @tag: human readable tag of the snapshot.
>> +#
>> +# @vm_size: size of the snapshot in Bytes.
> As this is a new QMP command, you should use '-', not '_'.  Benoit's
> patch named this @vm-state-size.
>
>> +#
>> +# @date: date and time of the snapshot as unix timestamp.
>> +#
>> +# @vm_clock: time in the guest in nsecs.
> For online internal snapshots (those created by savevm), the vm clock
> offset makes sense. But for offline snapshots (those created by
> 'qemu-img snapshot', with no VM state), there is no vm clock offset.
> 'qemu-img info' lists 00:00:00.000 as a result, but I wonder if it would
> be better to leave this field (and vm-state-size) as #optional and omit
> them from the JSON for offline snapshots.
>
> And since qemu refuses to load offline snapshots, it might be worth an
> additional field in the JSON that says whether a snapshot is online or
> offline, to make it easier for the parsing application to determine
> whether it can be loaded or must go through qemu-img while the guest is
> offline (then again, keeping vm-state-size as non-optional, and checking
> for 0 size, somewhat covers this).
As you said, this is covered and I think that checking for 0 size is 
good enough.
>> +#
>> +# Since:  1.2
> 1.3 (entire series...)
>

Patch

diff --git a/hmp.c b/hmp.c
index a3fd8ca..cf0e036 100644
--- a/hmp.c
+++ b/hmp.c
@@ -613,6 +613,39 @@  void hmp_info_block_jobs(Monitor *mon)
     }
 }
 
+void hmp_info_snapshots(Monitor *mon)
+{
+    SnapshotInfoList *list;
+    Error *err = NULL;
+    char buf[256];
+    QEMUSnapshotInfo sn;
+
+    list = qmp_query_vm_snapshots(&err);
+
+    if (error_is_set(&err)) {
+        hmp_handle_error(mon, &err);
+        return;
+    }
+
+    if (!list) {
+        monitor_printf(mon, "There is no snapshot available.\n");
+        return;
+    }
+    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+    while (list) {
+        memcpy(&(sn.id_str), list->value->id, sizeof(sn.id_str));
+        memcpy(&(sn.name), list->value->tag, sizeof(sn.name));
+        sn.date_sec = list->value->date;
+        sn.date_nsec = sn.date_sec * 1000000000;
+        sn.vm_clock_nsec = list->value->vm_clock;
+        sn.vm_state_size = list->value->vm_size;
+        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+        list = list->next;
+    }
+
+    qapi_free_SnapshotInfoList(list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 738d9bc..9b67fc3 100644
--- a/hmp.h
+++ b/hmp.h
@@ -36,6 +36,7 @@  void hmp_info_spice(Monitor *mon);
 void hmp_info_balloon(Monitor *mon);
 void hmp_info_pci(Monitor *mon);
 void hmp_info_block_jobs(Monitor *mon);
+void hmp_info_snapshots(Monitor *mon);
 void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
diff --git a/monitor.c b/monitor.c
index b75b9e0..db9a8d0 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2541,7 +2541,7 @@  static mon_cmd_t info_cmds[] = {
         .args_type  = "",
         .params     = "",
         .help       = "show the currently saved VM snapshots",
-        .mhandler.info = do_info_snapshots,
+        .mhandler.info = hmp_info_snapshots,
     },
     {
         .name       = "status",
diff --git a/qapi-schema.json b/qapi-schema.json
index feea992..763b51e 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1053,6 +1053,40 @@ 
 { 'command': 'query-block-jobs', 'returns': ['BlockJobInfo'] }
 
 ##
+# @SnapshotInfo:
+#
+# Snapshot list.  This structure contains list of snapshots for virtual machine.
+#
+# @id: id of the snapshot.
+#
+# @tag: human readable tag of the snapshot.
+#
+# @vm_size: size of the snapshot in Bytes.
+#
+# @date: date and time of the snapshot as unix timestamp.
+#
+# @vm_clock: time in the guest in nsecs.
+#
+# Since:  1.2
+##
+{ 'type': 'SnapshotInfo',
+  'data': {'id': 'str', 'tag': 'str', 'vm_size': 'int',
+           'date': 'int', 'vm_clock': 'int'} }
+
+##
+# @query-vm-snapshots:
+#
+# List available snapshots for VM.
+#
+# Returns: an array of @SnapshotInfo describing each snapshot or an empty array
+#            on success
+#          If an error occurs, GenericError with error message
+#
+# Since: 1.2
+##
+{ 'command': 'query-vm-snapshots', 'returns': ['SnapshotInfo'] }
+
+##
 # @quit:
 #
 # This command will cause the QEMU process to exit gracefully.  While every
diff --git a/qmp-commands.hx b/qmp-commands.hx
index bca4267..3609de8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2465,3 +2465,33 @@  EQMP
         .mhandler.cmd_new = qmp_marshal_input_query_cpu_definitions,
     },
 
+SQMP
+query-vm-snapshots
+----------
+
+List available snapshots for VM.
+
+Return an array of json-object, each with the following information:
+
+- "id": id of the snapshot.
+
+- "tag": human readable tag of the snapshot.
+
+- "vm_size": size of the snapshot in Bytes.
+
+- "date": date and time of the snapshot as unix timestamp.
+
+- "vm_clock": time in the guest in nsecs.
+
+Example:
+
+-> { "execute": "query-vm-snapshots" }
+<- {"return": [{"vm_size": 212309067, "tag": "my_snapshot",
+    "vm_clock": 630176706190, "id": "1", "date": 1341999856}]}
+
+EQMP
+    {
+        .name       = "query-vm-snapshots",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_vm_snapshots,
+    },
diff --git a/savevm.c b/savevm.c
index 3f7ec44..ca7448d 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2332,34 +2332,28 @@  void qmp_vm_snapshot_delete(const char *name, Error **errp)
     }
 }
 
-void do_info_snapshots(Monitor *mon)
+SnapshotInfoList *qmp_query_vm_snapshots(Error **errp)
 {
+    SnapshotInfoList *snapshot_list = NULL, *last = NULL;
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
     int nb_sns, i, ret, available;
-    int total;
-    int *available_snapshots;
-    char buf[256];
 
     bs = bdrv_snapshots();
     if (!bs) {
-        monitor_printf(mon, "No available block device supports snapshots\n");
-        return;
+        error_set(errp, QERR_NOT_SUPPORTED);
+        return NULL;
     }
 
-    nb_sns = bdrv_snapshot_list(bs, &sn_tab, NULL);
-    if (nb_sns < 0) {
-        monitor_printf(mon, "bdrv_snapshot_list: error %d\n", nb_sns);
-        return;
+    nb_sns = bdrv_snapshot_list(bs, &sn_tab, errp);
+    if (error_is_set(errp)) {
+        return NULL;
     }
 
     if (nb_sns == 0) {
-        monitor_printf(mon, "There is no snapshot available.\n");
-        return;
+        return NULL;
     }
 
-    available_snapshots = g_malloc0(sizeof(int) * nb_sns);
-    total = 0;
     for (i = 0; i < nb_sns; i++) {
         sn = &sn_tab[i];
         available = 1;
@@ -2376,24 +2370,27 @@  void do_info_snapshots(Monitor *mon)
         }
 
         if (available) {
-            available_snapshots[total] = i;
-            total++;
-        }
-    }
-
-    if (total > 0) {
-        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
-        for (i = 0; i < total; i++) {
-            sn = &sn_tab[available_snapshots[i]];
-            monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+            SnapshotInfoList *info = g_malloc0(sizeof(*info));
+            info->value = g_malloc0(sizeof(*info->value));
+            info->value->id = g_strdup(sn->id_str);
+            info->value->tag = g_strdup(sn->name);
+            info->value->vm_size = sn->vm_state_size;
+            info->value->date = sn->date_sec;
+            info->value->vm_clock = sn->vm_clock_nsec;
+
+            if (!snapshot_list) {
+                snapshot_list = info;
+                last = info;
+            } else {
+                last->next = info;
+                last = info;
+            }
         }
-    } else {
-        monitor_printf(mon, "There is no suitable snapshot available\n");
     }
 
     g_free(sn_tab);
-    g_free(available_snapshots);
 
+    return snapshot_list;
 }
 
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
diff --git a/sysemu.h b/sysemu.h
index 44afe61..a77dad2 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -69,8 +69,6 @@  void qemu_remove_exit_notifier(Notifier *notify);
 
 void qemu_add_machine_init_done_notifier(Notifier *notify);
 
-void do_info_snapshots(Monitor *mon);
-
 void qemu_announce_self(void);
 
 bool qemu_savevm_state_blocked(Error **errp);