Patchwork [V10,12/17] hmp: add function hmp_info_snapshots()

login
register
mail settings
Submitter Wayne Xia
Date March 22, 2013, 2:19 p.m.
Message ID <1363961953-13561-13-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/230101/
State New
Headers show

Comments

Wayne Xia - March 22, 2013, 2:19 p.m.
This function will simply call qmp interface qmp_query_snapshots()
added in last commit and then dump information in monitor console.
  To get snapshot info, Now qemu and qemu-img both call block layer
function bdrv_query_snapshot_info_list() in their calling path, and
then they just translate the qmp object got to strings in stdout or
monitor console.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 hmp.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 hmp.h |    1 +
 2 files changed, 43 insertions(+), 0 deletions(-)
Eric Blake - March 29, 2013, 11:04 p.m.
On 03/22/2013 08:19 AM, Wenchao Xia wrote:
>   This function will simply call qmp interface qmp_query_snapshots()
> added in last commit and then dump information in monitor console.
>   To get snapshot info, Now qemu and qemu-img both call block layer
> function bdrv_query_snapshot_info_list() in their calling path, and
> then they just translate the qmp object got to strings in stdout or
> monitor console.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  hmp.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  hmp.h |    1 +
>  2 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index b0a861c..c475d65 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -651,6 +651,48 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>      qapi_free_TPMInfoList(info_list);
>  }
>  
> +/* assume list is valid */
> +static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
> +{
> +    SnapshotInfoList *elem;
> +    char buf[256];
> +
> +    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));

Are you sure that won't ever be truncated?  I'm pretty sure that I could
come up with a scenario where I cause bdrv_snapshot_dump() to want to
output more than 256 bytes of details.
Wayne Xia - March 30, 2013, 12:38 p.m.
于 2013-3-30 7:04, Eric Blake 写道:
> On 03/22/2013 08:19 AM, Wenchao Xia wrote:
>>    This function will simply call qmp interface qmp_query_snapshots()
>> added in last commit and then dump information in monitor console.
>>    To get snapshot info, Now qemu and qemu-img both call block layer
>> function bdrv_query_snapshot_info_list() in their calling path, and
>> then they just translate the qmp object got to strings in stdout or
>> monitor console.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   hmp.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>   hmp.h |    1 +
>>   2 files changed, 43 insertions(+), 0 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index b0a861c..c475d65 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -651,6 +651,48 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
>>       qapi_free_TPMInfoList(info_list);
>>   }
>>
>> +/* assume list is valid */
>> +static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
>> +{
>> +    SnapshotInfoList *elem;
>> +    char buf[256];
>> +
>> +    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
>
> Are you sure that won't ever be truncated?  I'm pretty sure that I could
> come up with a scenario where I cause bdrv_snapshot_dump() to want to
> output more than 256 bytes of details.
>
   I hope not use dynamic buff for strings, how about check it as
following:

char buf[254] = 0;
monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
if (buf[254]) {
      monitor_printf(mon, "'string truncated'");
}
Kevin Wolf - April 2, 2013, 7:39 a.m.
Am 30.03.2013 um 13:38 hat Wenchao Xia geschrieben:
> 于 2013-3-30 7:04, Eric Blake 写道:
> >On 03/22/2013 08:19 AM, Wenchao Xia wrote:
> >>   This function will simply call qmp interface qmp_query_snapshots()
> >>added in last commit and then dump information in monitor console.
> >>   To get snapshot info, Now qemu and qemu-img both call block layer
> >>function bdrv_query_snapshot_info_list() in their calling path, and
> >>then they just translate the qmp object got to strings in stdout or
> >>monitor console.
> >>
> >>Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> >>---
> >>  hmp.c |   42 ++++++++++++++++++++++++++++++++++++++++++
> >>  hmp.h |    1 +
> >>  2 files changed, 43 insertions(+), 0 deletions(-)
> >>
> >>diff --git a/hmp.c b/hmp.c
> >>index b0a861c..c475d65 100644
> >>--- a/hmp.c
> >>+++ b/hmp.c
> >>@@ -651,6 +651,48 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
> >>      qapi_free_TPMInfoList(info_list);
> >>  }
> >>
> >>+/* assume list is valid */
> >>+static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
> >>+{
> >>+    SnapshotInfoList *elem;
> >>+    char buf[256];
> >>+
> >>+    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> >
> >Are you sure that won't ever be truncated?  I'm pretty sure that I could
> >come up with a scenario where I cause bdrv_snapshot_dump() to want to
> >output more than 256 bytes of details.
> >
>   I hope not use dynamic buff for strings, how about check it as
> following:
> 
> char buf[254] = 0;
> monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> if (buf[254]) {
>      monitor_printf(mon, "'string truncated'");
> }

About as useless as the truncated string alone. That something is
missing will be noticed by the user; the fix isn't to tell him that the
output is broken, but to give the full output.

Kevin

Patch

diff --git a/hmp.c b/hmp.c
index b0a861c..c475d65 100644
--- a/hmp.c
+++ b/hmp.c
@@ -651,6 +651,48 @@  void hmp_info_tpm(Monitor *mon, const QDict *qdict)
     qapi_free_TPMInfoList(info_list);
 }
 
+/* assume list is valid */
+static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
+{
+    SnapshotInfoList *elem;
+    char buf[256];
+
+    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+
+    for (elem = list; elem; elem = elem->next) {
+        QEMUSnapshotInfo sn = {
+            .vm_state_size = elem->value->vm_state_size,
+            .date_sec = elem->value->date_sec,
+            .date_nsec = elem->value->date_nsec,
+            .vm_clock_nsec = elem->value->vm_clock_sec * 1000000000ULL +
+                             elem->value->vm_clock_nsec,
+        };
+        pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
+        pstrcpy(sn.name, sizeof(sn.name), elem->value->name);
+        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), &sn));
+    }
+}
+
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+    SnapshotInfoList *list;
+
+    list = qmp_query_snapshots(&err);
+    if (error_is_set(&err)) {
+        hmp_handle_error(mon, &err);
+        return;
+    }
+
+    if (list == NULL) {
+        monitor_printf(mon, "There is no suitable snapshot available\n");
+        return;
+    }
+
+    monitor_dump_snapshotinfolist(mon, list);
+    qapi_free_SnapshotInfoList(list);
+}
+
 void hmp_quit(Monitor *mon, const QDict *qdict)
 {
     monitor_suspend(mon);
diff --git a/hmp.h b/hmp.h
index 95fe76e..106d8d5 100644
--- a/hmp.h
+++ b/hmp.h
@@ -37,6 +37,7 @@  void hmp_info_balloon(Monitor *mon, const QDict *qdict);
 void hmp_info_pci(Monitor *mon, const QDict *qdict);
 void hmp_info_block_jobs(Monitor *mon, const QDict *qdict);
 void hmp_info_tpm(Monitor *mon, const QDict *qdict);
+void hmp_info_snapshots(Monitor *mon, const QDict *qdict);
 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);