Re: [Qemu-devel] [PATCH 2/2 V3] hmp: show all of snapshot info on every block dev in output of 'info snapshots'

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]

On 29.06.2016 09:43, 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>
> ---
>  migration/savevm.c | 95 ++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 88 insertions(+), 7 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index a8f22da..e5a5536 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c

[...]

> @@ -2249,11 +2308,33 @@ 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) {
> +        QTAILQ_FOREACH(snapshot_entry, &image_entry->snapshots, next) {
> +            g_free(snapshot_entry);
> +        }
> +        g_free(image_entry);
> +    }

Just as in the for loop removing snapshot entries that are available on
all images, you need to use QTAILQ_FOREACH_SAFE() if you free list
elements inside of the loop (so both of these QTAILQ_FOREACH() need to
be QTAILQ_FOREACH_SAFE()).

Looks good apart from that, though.

Max

>      g_free(sn_tab);
> -    g_free(available_snapshots);
> +    g_free(global_snapshots);
>  
>  }
>  
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]