Patchwork [3/3] HMP: show internal snapshots on a single device

login
register
mail settings
Submitter Wayne Xia
Date Dec. 19, 2012, 10:17 a.m.
Message ID <1355912230-25132-4-git-send-email-xiawenc@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/207306/
State New
Headers show

Comments

Wayne Xia - Dec. 19, 2012, 10:17 a.m.
This patch add an option to show snapshots on a single block
device, so some snapshot do not exist on other block device
could be shown.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 monitor.c |    6 +++---
 savevm.c  |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 57 insertions(+), 4 deletions(-)
Markus Armbruster - Dec. 21, 2012, 2:07 p.m.
Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

>   This patch add an option to show snapshots on a single block
> device, so some snapshot do not exist on other block device
> could be shown.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  monitor.c |    6 +++---
>  savevm.c  |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index ce0e74d..b019618 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2613,9 +2613,9 @@ static mon_cmd_t info_cmds[] = {
>      },
>      {
>          .name       = "snapshots",
> -        .args_type  = "",
> -        .params     = "",
> -        .help       = "show the currently saved VM snapshots",
> +        .args_type  = "device:B?",
> +        .params     = "[device]",
> +        .help       = "show snapshots of whole vm or a single device",
>          .mhandler.info = do_info_snapshots,
>      },
>      {
> diff --git a/savevm.c b/savevm.c
> index fa32171..438eb24 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -2358,7 +2358,7 @@ void do_delvm(Monitor *mon, const QDict *qdict)
>      }
>  }
>  
> -void do_info_snapshots(Monitor *mon, const QDict *qdict)
> +static void do_info_snapshots_vm(Monitor *mon)
>  {
>      BlockDriverState *bs, *bs1;
>      QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
> @@ -2422,6 +2422,59 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict)
>  
>  }
>  
> +static void do_info_snapshots_blk(Monitor *mon, const char *device)
> +{
> +    BlockDriverState *bs;
> +    QEMUSnapshotInfo *sn_tab, *sn;
> +    int nb_sns, i;
> +    char buf[256];
> +
> +    /* find the target bs */
> +    bs = bdrv_find(device);
> +    if (!bs) {
> +        monitor_printf(mon, "Device '%s' not found.\n", device);
> +        return ;
> +    }
> +
> +    if (!bdrv_can_snapshot(bs)) {
> +        monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
> +        return ;
> +    }

Rest of the function is is essentially code copied from
do_info_snapshots_vm().  Could it be factored out?

> +
> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
> +    if (nb_sns < 0) {
> +        monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n",
> +                       device, nb_sns);
> +        return;
> +    }
> +
> +    if (nb_sns == 0) {
> +        monitor_printf(mon, "There is no snapshot available.\n");
> +        return;
> +    }
> +
> +    monitor_printf(mon, "Device %s:\n", device);
> +    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
> +    for (i = 0; i < nb_sns; i++) {
> +        sn = &sn_tab[i];
> +        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
> +    }
> +    g_free(sn_tab);
> +    return;
> +}
> +
> +void do_info_snapshots(Monitor *mon, const QDict *qdict)
> +{
> +    /* Todo, there should be a layer rebuild qdict before enter this func. */

I'm not sure I get this comment.

We usually capitalize TODO for easy grepping.

> +    const char *device = qdict_get_try_str(qdict, "device");
> +    if (!device) {
> +        do_info_snapshots_vm(mon);
> +    } else {
> +        do_info_snapshots_blk(mon, device);
> +    }
> +    return;
> +}
> +
>  void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
>  {
>      qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
Wayne Xia - Dec. 25, 2012, 3:11 a.m.
>>   
>> +static void do_info_snapshots_blk(Monitor *mon, const char *device)
>> +{
>> +    BlockDriverState *bs;
>> +    QEMUSnapshotInfo *sn_tab, *sn;
>> +    int nb_sns, i;
>> +    char buf[256];
>> +
>> +    /* find the target bs */
>> +    bs = bdrv_find(device);
>> +    if (!bs) {
>> +        monitor_printf(mon, "Device '%s' not found.\n", device);
>> +        return ;
>> +    }
>> +
>> +    if (!bdrv_can_snapshot(bs)) {
>> +        monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
>> +        return ;
>> +    }
> 
> Rest of the function is is essentially code copied from
> do_info_snapshots_vm().  Could it be factored out?
> 
  Sure, I forgot that before. I guess a better way is adding a QMP
interface first and using common code from it.

>> +
>> +    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
>> +    if (nb_sns < 0) {
>> +        monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n",
>> +                       device, nb_sns);
>> +        return;
>> +    }
>> +
>> +    if (nb_sns == 0) {
>> +        monitor_printf(mon, "There is no snapshot available.\n");
>> +        return;
>> +    }
>> +
>> +    monitor_printf(mon, "Device %s:\n", device);
>> +    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
>> +    for (i = 0; i < nb_sns; i++) {
>> +        sn = &sn_tab[i];
>> +        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
>> +    }
>> +    g_free(sn_tab);
>> +    return;
>> +}
>> +
>> +void do_info_snapshots(Monitor *mon, const QDict *qdict)
>> +{
>> +    /* Todo, there should be a layer rebuild qdict before enter this func. */
> 
> I'm not sure I get this comment.
> 
> We usually capitalize TODO for easy grepping.
> 
  My mistake, this comments should have been removed, it is used before
where qdict was not rebuilt as a new one.

>> +    const char *device = qdict_get_try_str(qdict, "device");
>> +    if (!device) {
>> +        do_info_snapshots_vm(mon);
>> +    } else {
>> +        do_info_snapshots_blk(mon, device);
>> +    }
>> +    return;
>> +}
>> +
>>   void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
>>   {
>>       qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,
>

Patch

diff --git a/monitor.c b/monitor.c
index ce0e74d..b019618 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2613,9 +2613,9 @@  static mon_cmd_t info_cmds[] = {
     },
     {
         .name       = "snapshots",
-        .args_type  = "",
-        .params     = "",
-        .help       = "show the currently saved VM snapshots",
+        .args_type  = "device:B?",
+        .params     = "[device]",
+        .help       = "show snapshots of whole vm or a single device",
         .mhandler.info = do_info_snapshots,
     },
     {
diff --git a/savevm.c b/savevm.c
index fa32171..438eb24 100644
--- a/savevm.c
+++ b/savevm.c
@@ -2358,7 +2358,7 @@  void do_delvm(Monitor *mon, const QDict *qdict)
     }
 }
 
-void do_info_snapshots(Monitor *mon, const QDict *qdict)
+static void do_info_snapshots_vm(Monitor *mon)
 {
     BlockDriverState *bs, *bs1;
     QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = &s;
@@ -2422,6 +2422,59 @@  void do_info_snapshots(Monitor *mon, const QDict *qdict)
 
 }
 
+static void do_info_snapshots_blk(Monitor *mon, const char *device)
+{
+    BlockDriverState *bs;
+    QEMUSnapshotInfo *sn_tab, *sn;
+    int nb_sns, i;
+    char buf[256];
+
+    /* find the target bs */
+    bs = bdrv_find(device);
+    if (!bs) {
+        monitor_printf(mon, "Device '%s' not found.\n", device);
+        return ;
+    }
+
+    if (!bdrv_can_snapshot(bs)) {
+        monitor_printf(mon, "Device '%s' can't have snapshot.\n", device);
+        return ;
+    }
+
+    nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+    if (nb_sns < 0) {
+        monitor_printf(mon, "Device %s bdrv_snapshot_list: error %d\n",
+                       device, nb_sns);
+        return;
+    }
+
+    if (nb_sns == 0) {
+        monitor_printf(mon, "There is no snapshot available.\n");
+        return;
+    }
+
+    monitor_printf(mon, "Device %s:\n", device);
+    monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), NULL));
+    for (i = 0; i < nb_sns; i++) {
+        sn = &sn_tab[i];
+        monitor_printf(mon, "%s\n", bdrv_snapshot_dump(buf, sizeof(buf), sn));
+    }
+    g_free(sn_tab);
+    return;
+}
+
+void do_info_snapshots(Monitor *mon, const QDict *qdict)
+{
+    /* Todo, there should be a layer rebuild qdict before enter this func. */
+    const char *device = qdict_get_try_str(qdict, "device");
+    if (!device) {
+        do_info_snapshots_vm(mon);
+    } else {
+        do_info_snapshots_blk(mon, device);
+    }
+    return;
+}
+
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
 {
     qemu_ram_set_idstr(memory_region_get_ram_addr(mr) & TARGET_PAGE_MASK,