Re: [Qemu-devel] [PATCH 5/6] snapshot: qmp interface

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013-1-5 0:22, Stefan Hajnoczi 写道:
> On Mon, Dec 17, 2012 at 02:25:08PM +0800, Wenchao Xia wrote:
>> @@ -1478,16 +1497,39 @@
>>   #
>>   # @device:  the name of the device to generate the snapshot from.
>>   #
>> -# @snapshot-file: the target of the new image. A new file will be created.
>> +# @snapshot-file: the target name of the snapshot. In external case, it is
>> +#                 the new file's name, A new file will be created. In internal
>> +#                 case, it is the internal snapshot record's name and if it is
>> +#                 'blank' name will be generated according to time.
>>   #
>>   # @format: #optional the format of the snapshot image, default is 'qcow2'.
>>   #
>> -# @mode: #optional whether and how QEMU should create a new image, default is
>> -#        'absolute-paths'.
>> +# @mode: #optional whether QEMU should create a new snapshot or use existing
>> +#        one, default is 'absolute-paths'.
>> +#
>> +# @type: #optional internal snapshot or external, default is 'external'.
>> +#
>>   ##
>>   { 'type': 'BlockdevSnapshot',
>>     'data': { 'device': 'str', 'snapshot-file': 'str', '*format': 'str',
>> -            '*mode': 'NewImageMode' } }
>> +            '*mode': 'NewImageMode', '*type': 'SnapshotType'} }
>
> It feels cleaner to define a new BlockdevInternalSnapshot type instead
> of overloading the field semantics.  The naming doesn't fit for internal
> snapshots (e.g. "snapshot-file", "absolute-paths", etc).
>
> Treat internal snapshots as a new transaction action.  That way we can
> cleanly add internal snapshot specific fields in the future.  The
> documentation will be clearer because users won't have to consider the
> "if type is 'external', then ..., otherwise ...".
>
> Stefan
>
   After struggling about the parameter meaning, I guess this is what
make things better.



-- 
Best Regards

Wenchao Xia