On 05/08/2013 11:27 PM, Zhang Xiaohe wrote:
> 于 2013年05月09日 01:16, Eric Blake 写道:
>> On 05/08/2013 02:50 AM, qiaonuohan wrote:
>>>
>>> Thanks for your suggestion. I will fix it like:
>>>
>>> { 'enum': 'DumpCompressionFormat',
>>> 'data': [ 'zlib', 'lzo', 'snappy' ] }
>>>
>>> For zlib is treated as the default compression format, and
>>> 'uncompressed' won't be an option.
>>
>> No, I was serious that you need to provide 'uncompressed' as an explicit
>> enum value. It is very annoying to toggle between four states (three
>> compression formats and a fourth state of no compression) when the
>> fourth is available only by omitting a parameter. The default MUST be
>> 'uncompressed' for backwards-compatibility, not 'zlib'.
>>
> We'd like to make sure that we understand you precisely.
>
> The definion is like below:
> { 'enum': 'DumpGuestMemoryFormat',
> 'data': [ 'uncompressed', 'zlib', 'lzo', 'snappy' ] }
>
> { 'command': 'dump-guest-memory',
> 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
> '*length': 'int', '*format': 'DumpCompressionFormat' } }Closer - make sure you use the same type name in both places (the enum name 'DumpGuestMemoryFormat' is slightly nicer than the command use of 'DumpCompressionFormat'. > > 'format' is optional: > 1. when 'format' is not specified, vmcore will be in ELF format. > 2. when 'format' is specified and its parameter is 'uncompressed', > vmcore will be in ELF format as well. > 3. when 'format' is specified and its parameter is 'zlib/lzo/snappy', > vmcore will be in kdump-compressed format. Correct. Or you could use the name 'elf' instead of 'uncompressed', if that makes more sense - as long as the enum calls out the names you are supporting. > > If this is what you suggest, then I don't think it is necessary to > add 'uncompressed'. The backwards-compatibility is assured by case 1, > in which the interface is exactly the same as before. So why do we > add another parameter to do the same thing again? Because it is nicer to apps to allow them to explicitly specify the default. Making 'format' optional allows older apps to still work, but for newer apps, it is easier to ALWAYS supply the format argument than it is to make the generation of the format argument conditional on whether the default behavior is desired. Trust me - I'm reviewing this as a potential user of your interface. When I ask for a fourth enum value, it's because I want to use it. When you keep coming back and complaining that you don't want to provide the fourth enum value because it is redundant, I feel like you aren't paying attention to your user base. I also think that implementation-wise, it will be easier to write your code if you have an enum supplying all four possibilities, since it is easier to write code that defaults a missing argument to a known enum value, and then have the rest of your code deal with enum values, than it is to write code that has to check everywhere whether the argument is missing (for one value) vs. one of three other enum values. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
