Patchwork [V7,14/14] hmp: show snapshots on single block device

login
register
mail settings
Submitter Wayne Xia
Date Feb. 26, 2013, 10:40 a.m.
Message ID <1361875228-15769-15-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/223195/
State New
Headers show

Comments

Wayne Xia - Feb. 26, 2013, 10:40 a.m.
This patch added the support of showing internal snapshots on an
image in the backing chain of a block device in hmp layer, by
calling a qmp function.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 hmp.c     |   81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 monitor.c |    6 ++--
 2 files changed, 83 insertions(+), 4 deletions(-)
Eric Blake - Feb. 26, 2013, 4:13 p.m.
On 02/26/2013 03:40 AM, Wenchao Xia wrote:
>   This patch added the support of showing internal snapshots on an
> image in the backing chain of a block device in hmp layer, by
> calling a qmp function.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  hmp.c     |   81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  monitor.c |    6 ++--
>  2 files changed, 83 insertions(+), 4 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 5e68b2f..ffa92ff 100644

> +static ImageInfo *find_image_info(const DeviceImageInfoList *device_image_list,
> +                                  const char *image_name)
> +{
> +    ImageInfo *image_info;
> +

> +
> +    /* search the chain */
> +    while (device_image_list) {
> +        if (device_image_list->value->has_image) {
> +            image_info = device_image_list->value->image;
> +            if (!strcmp(image_info->filename, image_name)) {
> +                return image_info;
> +            }
> +        }

Is this loop smart enough, or should it be borrowing from some of the
additional complexity in bdrv_find_backing_image for properly chasing
down relative names?


> +    if (image_info) {
> +        if (image_info->has_snapshots) {
> +            list = image_info->snapshots;
> +            monitor_printf(mon, "Device '%s', Image name '%s':\n",
> +                           device_name, image_info->filename);
> +            monitor_dump_snapshotinfolist(mon, list);
> +        } else {
> +            monitor_printf(mon, "Device '%s' have no valid "

s/have/has/

> +                           "internal snapshot.\n",
> +                           device_name);
> +        }
> +    } else {
> +            monitor_printf(mon, "Device '%s' have no correspond image now.\n",

s/have no correspond image now/has no corresponding image/
Wayne Xia - Feb. 27, 2013, 2:29 a.m.
于 2013-2-27 0:13, Eric Blake 写道:
> On 02/26/2013 03:40 AM, Wenchao Xia wrote:
>>    This patch added the support of showing internal snapshots on an
>> image in the backing chain of a block device in hmp layer, by
>> calling a qmp function.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   hmp.c     |   81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   monitor.c |    6 ++--
>>   2 files changed, 83 insertions(+), 4 deletions(-)
>>
>> diff --git a/hmp.c b/hmp.c
>> index 5e68b2f..ffa92ff 100644
>
>> +static ImageInfo *find_image_info(const DeviceImageInfoList *device_image_list,
>> +                                  const char *image_name)
>> +{
>> +    ImageInfo *image_info;
>> +
>
>> +
>> +    /* search the chain */
>> +    while (device_image_list) {
>> +        if (device_image_list->value->has_image) {
>> +            image_info = device_image_list->value->image;
>> +            if (!strcmp(image_info->filename, image_name)) {
>> +                return image_info;
>> +            }
>> +        }
>
> Is this loop smart enough, or should it be borrowing from some of the
> additional complexity in bdrv_find_backing_image for properly chasing
> down relative names?
>
   OK, it will be enhanced.

>
>> +    if (image_info) {
>> +        if (image_info->has_snapshots) {
>> +            list = image_info->snapshots;
>> +            monitor_printf(mon, "Device '%s', Image name '%s':\n",
>> +                           device_name, image_info->filename);
>> +            monitor_dump_snapshotinfolist(mon, list);
>> +        } else {
>> +            monitor_printf(mon, "Device '%s' have no valid "
>
> s/have/has/
>
OK.

>> +                           "internal snapshot.\n",
>> +                           device_name);
>> +        }
>> +    } else {
>> +            monitor_printf(mon, "Device '%s' have no correspond image now.\n",
>
> s/have no correspond image now/has no corresponding image/
>
OK.
Wayne Xia - Feb. 27, 2013, 8:35 a.m.
于 2013-2-27 10:29, Wenchao Xia 写道:
> 于 2013-2-27 0:13, Eric Blake 写道:
>> On 02/26/2013 03:40 AM, Wenchao Xia wrote:
>>>    This patch added the support of showing internal snapshots on an
>>> image in the backing chain of a block device in hmp layer, by
>>> calling a qmp function.
>>>
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> ---
>>>   hmp.c     |   81
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>>   monitor.c |    6 ++--
>>>   2 files changed, 83 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hmp.c b/hmp.c
>>> index 5e68b2f..ffa92ff 100644
>>
>>> +static ImageInfo *find_image_info(const DeviceImageInfoList
>>> *device_image_list,
>>> +                                  const char *image_name)
>>> +{
>>> +    ImageInfo *image_info;
>>> +
>>
>>> +
>>> +    /* search the chain */
>>> +    while (device_image_list) {
>>> +        if (device_image_list->value->has_image) {
>>> +            image_info = device_image_list->value->image;
>>> +            if (!strcmp(image_info->filename, image_name)) {
>>> +                return image_info;
>>> +            }
>>> +        }
>>
>> Is this loop smart enough, or should it be borrowing from some of the
>> additional complexity in bdrv_find_backing_image for properly chasing
>> down relative names?
>>
>    OK, it will be enhanced.
>
   To make things more straight, I guess a new hmp API is better:
info image <device> -b
which may show all image info on a device, including internal snapshot.
In this case, hmp/qmp command are connected more closely and saves
the trouble of find correct one in hmp level. If you agree I'll
discard info snapshots <device> in next version.


>>
>>> +    if (image_info) {
>>> +        if (image_info->has_snapshots) {
>>> +            list = image_info->snapshots;
>>> +            monitor_printf(mon, "Device '%s', Image name '%s':\n",
>>> +                           device_name, image_info->filename);
>>> +            monitor_dump_snapshotinfolist(mon, list);
>>> +        } else {
>>> +            monitor_printf(mon, "Device '%s' have no valid "
>>
>> s/have/has/
>>
> OK.
>
>>> +                           "internal snapshot.\n",
>>> +                           device_name);
>>> +        }
>>> +    } else {
>>> +            monitor_printf(mon, "Device '%s' have no correspond
>>> image now.\n",
>>
>> s/have no correspond image now/has no corresponding image/
>>
> OK.
>
>

Patch

diff --git a/hmp.c b/hmp.c
index 5e68b2f..ffa92ff 100644
--- a/hmp.c
+++ b/hmp.c
@@ -629,6 +629,73 @@  static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list)
     }
 }
 
+/* Find the ImageInfo with correct image_name. If image_name = NULL, then
+   return the top one in the chain. */
+static ImageInfo *find_image_info(const DeviceImageInfoList *device_image_list,
+                                  const char *image_name)
+{
+    ImageInfo *image_info;
+
+    /* return top one */
+    if (!image_name) {
+        if (device_image_list->value->has_image) {
+            return device_image_list->value->image;
+        } else {
+            return NULL;
+        }
+    }
+
+    /* search the chain */
+    while (device_image_list) {
+        if (device_image_list->value->has_image) {
+            image_info = device_image_list->value->image;
+            if (!strcmp(image_info->filename, image_name)) {
+                return image_info;
+            }
+        }
+        device_image_list = device_image_list->next;
+    }
+    return NULL;
+}
+
+static void hmp_info_snapshots_device(Monitor *mon,
+                                      const char *device_name,
+                                      const char *image_name)
+{
+    Error *err = NULL;
+    SnapshotInfoList *list;
+
+    DeviceImageInfoList *device_image_list = qmp_query_images(true,
+                                            device_name, true, true, &err);
+    if (error_is_set(&err)) {
+        hmp_handle_error(mon, &err);
+        return;
+    }
+    if (!device_image_list) {
+        monitor_printf(mon, "Device '%s' can't get image info list.\n",
+                       device_name);
+        return;
+    }
+
+    ImageInfo *image_info = find_image_info(device_image_list, image_name);
+    if (image_info) {
+        if (image_info->has_snapshots) {
+            list = image_info->snapshots;
+            monitor_printf(mon, "Device '%s', Image name '%s':\n",
+                           device_name, image_info->filename);
+            monitor_dump_snapshotinfolist(mon, list);
+        } else {
+            monitor_printf(mon, "Device '%s' have no valid "
+                           "internal snapshot.\n",
+                           device_name);
+        }
+    } else {
+            monitor_printf(mon, "Device '%s' have no correspond image now.\n",
+                           device_name);
+    }
+    qapi_free_DeviceImageInfoList(device_image_list);
+}
+
 static void hmp_info_snapshots_vm(Monitor *mon)
 {
     Error *err = NULL;
@@ -651,7 +718,19 @@  static void hmp_info_snapshots_vm(Monitor *mon)
 
 void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
 {
-    hmp_info_snapshots_vm(mon);
+    const char *device_name = qdict_get_try_str(qdict, "device");
+    const char *image_name = qdict_get_try_str(qdict, "image");
+
+    if (device_name) {
+        hmp_info_snapshots_device(mon, device_name, image_name);
+    } else {
+        if (image_name) {
+            monitor_printf(mon, "Parameter [image] is valid only when "
+                                "device is specified.\n");
+            return;
+        }
+        hmp_info_snapshots_vm(mon);
+    }
 }
 
 void hmp_quit(Monitor *mon, const QDict *qdict)
diff --git a/monitor.c b/monitor.c
index 5112375..1dca36f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2591,9 +2591,9 @@  static mon_cmd_t info_cmds[] = {
     },
     {
         .name       = "snapshots",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show the currently saved VM snapshots",
+        .args_type  = "device:O?",
+        .params     = "[device=str] [,image=str]",
+        .help       = "show snapshots of whole vm or a single device",
         .mhandler.cmd = hmp_info_snapshots,
     },
     {