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

["Lin Ma" <lma@suse.com>]

>>> Max Reitz <mreitz@redhat.com> 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 <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.
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);
>>  
>>  }
>>  
>>