From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39975) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaSBs-0001dF-1i for qemu-devel@nongnu.org; Thu, 09 May 2013 10:51:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UaSBn-0008Ee-Qv for qemu-devel@nongnu.org; Thu, 09 May 2013 10:51:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:18435) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UaSBn-0008EW-JW for qemu-devel@nongnu.org; Thu, 09 May 2013 10:51:23 -0400 Message-ID: <518BB7D7.5050002@redhat.com> Date: Thu, 09 May 2013 08:51:03 -0600 From: Eric Blake MIME-Version: 1.0 References: <1367911007-13990-1-git-send-email-qiaonuohan@cn.fujitsu.com> <1367911007-13990-10-git-send-email-qiaonuohan@cn.fujitsu.com> <5189362E.3030707@redhat.com> <004301ce4bc9$11040970$330c1c50$@cn.fujitsu.com> <518A886D.6080105@redhat.com> <518B33BB.9010906@cn.fujitsu.com> In-Reply-To: <518B33BB.9010906@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2CWBVSTGGVJUVFJHGKKKP" Subject: Re: [Qemu-devel] [PATCH 9/9] Make monitor command 'dump-guest-memory' dump in kdump-compressed format List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Zhang Xiaohe Cc: qiaonuohan , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2CWBVSTGGVJUVFJHGKKKP Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/08/2013 11:27 PM, Zhang Xiaohe wrote: > =E4=BA=8E 2013=E5=B9=B405=E6=9C=8809=E6=97=A5 01:16, Eric Blake =E5=86=99= =E9=81=93: >> 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 explic= it >> 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. >=20 > The definion is like below: > { 'enum': 'DumpGuestMemoryFormat', > 'data': [ 'uncompressed', 'zlib', 'lzo', 'snappy' ] } >=20 > { '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'. >=20 > '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. >=20 > 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2CWBVSTGGVJUVFJHGKKKP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRi7fXAAoJEKeha0olJ0NqlFMIAKEQHu7rzGvyq8OwjMLGmd1m RJNy93Hk7Fo0FYArrFQNa6caP+JJBtDK58jJGTrrfJ9OLTNxwo+UMEYC6DJScAxr 9fcAgzLDLmL654jEss6BaOiCMW04O6BfahcvWZkP91TBli2Sqm07lSpNg9uP03Is 6V3Hpbx2io01POgArjOwmgCG4fvnPfNqE+H2TchGdSCw6ByWxPxNYaZDz0MG9U7C H6P9EKLJNzHjruLzCb7IsXuFdX8FLN9x+XJcrPGzrH2JB/Ufv+SUBCq2kWc+iGij 2QLKNqyK7qcs/NZrfmGKJ64Wqh0y6/U+kvs9usvoo1ExKJH2YmwKHjvSFIRVpBc= =YK6U -----END PGP SIGNATURE----- ------enig2CWBVSTGGVJUVFJHGKKKP--