On 04.07.2016 11:31, Lin Ma wrote:
> Currently, the output of 'info snapshots' shows fully available snapshots.
> It's opaque, hides some snapshot information to users. It's not convenient
> if users want to know more about all of snapshot information on every block
> device via monitor.
> 
> Follow Kevin's and Max's proposals, The patch makes the output more detailed:
> (qemu) info snapshots
> List of snapshots present on all disks:
>  ID        TAG                 VM SIZE                DATE       VM CLOCK
>  --        checkpoint-1           165M 2016-05-22 16:58:07   00:02:06.813
> 
> List of partial (non-loadable) snapshots on 'drive_image1':
>  ID        TAG                 VM SIZE                DATE       VM CLOCK
>  1         snap1                     0 2016-05-22 16:57:31   00:01:30.567
> 
> Signed-off-by: Lin Ma <lma@suse.com>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

I said it looked good apart from what I said, but I didn't give an R-b,
so this shouldn't be here.

> ---
>  migration/savevm.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 90 insertions(+), 7 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index a8f22da..95e9efa 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c

[...]

> @@ -2249,11 +2308,35 @@ void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
>              monitor_printf(mon, "\n");
>          }
>      } else {
> -        monitor_printf(mon, "There is no suitable snapshot available\n");
> +        monitor_printf(mon, "None\n");
>      }
>  
> +    QTAILQ_FOREACH(image_entry, &image_list, next) {
> +        if (QTAILQ_EMPTY(&image_entry->snapshots)) {
> +            continue;
> +        }
> +        monitor_printf(mon,
> +                       "\nList of partial (non-loadable) snapshots on '%s':\n",
> +                       image_entry->imagename);
> +        bdrv_snapshot_dump((fprintf_function)monitor_printf, mon, NULL);
> +        monitor_printf(mon, "\n");
> +        QTAILQ_FOREACH(snapshot_entry, &image_entry->snapshots, next) {
> +            bdrv_snapshot_dump((fprintf_function)monitor_printf, mon,
> +                               &snapshot_entry->sn);
> +            monitor_printf(mon, "\n");
> +        }
> +    }
> +
> +    QTAILQ_FOREACH(image_entry, &image_list, next) {

This needs to be a QTAILQ_FOREACH_SAFE(), too.

Max

> +        SnapshotEntry *next_sn;
> +        QTAILQ_FOREACH_SAFE(snapshot_entry, &image_entry->snapshots, next,
> +                            next_sn) {
> +            g_free(snapshot_entry);
> +        }
> +        g_free(image_entry);
> +    }
>      g_free(sn_tab);
> -    g_free(available_snapshots);
> +    g_free(global_snapshots);
>  
>  }
>  
>