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

[Lin Ma]

>>> Max Reitz <address@hidden> 2016/7/6 星期三 上午 1:26 >>>
>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 <address@hidden>
>> Reviewed-by: Max Reitz <address@hidden>
>
>I said it looked good apart from what I said, but I didn't give an R-b,
>so this shouldn't be here.
Got it.

>
>> ---
>>  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
OK, I'd like to define such a var: 'ImageEntry *next_ie' for this 
QTAILQ_FOREACH_SAFE,
put it above this line.
How about it?
 
Thanks.

>>
>> +        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);
>>  
>>  }
>>  
>>