From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:43194) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcRsQ-0006a5-Ot for qemu-devel@nongnu.org; Tue, 14 May 2013 22:55:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UcRsP-0002Lk-2Y for qemu-devel@nongnu.org; Tue, 14 May 2013 22:55:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59064) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UcRsO-0002Lg-QM for qemu-devel@nongnu.org; Tue, 14 May 2013 22:55:36 -0400 Message-ID: <5192F91A.7090904@redhat.com> Date: Tue, 14 May 2013 20:55:22 -0600 From: Eric Blake MIME-Version: 1.0 References: <1368584998-20053-1-git-send-email-qiaonuohan@cn.fujitsu.com> <1368584998-20053-10-git-send-email-qiaonuohan@cn.fujitsu.com> In-Reply-To: <1368584998-20053-10-git-send-email-qiaonuohan@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2KERKBELWCMOWLUESHRMF" Subject: Re: [Qemu-devel] [PATCH 9/9 v2] Make monitor command 'dump-guest-memory' dump in kdump-compressed format List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Qiao Nuohan Cc: qemu-devel@nongnu.org, d.hatayama@jp.fujitsu.com, kumagai-atsushi@mxc.nes.nec.co.jp, zhangxh@cn.fujitsu.com, anderson@redhat.com, afaerber@suse.de This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2KERKBELWCMOWLUESHRMF Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 05/14/2013 08:29 PM, Qiao Nuohan wrote: > Make monitor command 'dump-guest-memory' dump in kdump-compressed forma= t. > The command's usage: > dump [-p] protocol [begin] [length] [format] > 'format' is used to specified the format of vmcore and can be: > 1. 'elf': ELF format, without compression > 2. 'zlib': kdump-compressed format, with zlib-compressed > 3. 'lzo': kdump-compressed format, with lzo-compressed > 4. 'snappy': kdump-compressed format, with snappy-compressed > And without 'format' being set, vmcore will be in ELF format. >=20 > Note: > 1. The kdump-compressed format is readable only with the crash utilit= y, and it > can be smaller than the ELF format because of the compression supp= ort. > 2. The kdump-compressed format is the 5th edition. >=20 > Signed-off-by: Qiao Nuohan > Signed-off-by: Zhang Xiaohe > --- > -static int dump_init(DumpState *s, int fd, bool paging, bool has_filte= r, > - int64_t begin, int64_t length, Error **errp) > +static int dump_init(DumpState *s, int fd, bool compress_format, > + DumpGuestMemoryFormat format, bool paging, > + bool has_filter, int64_t begin, int64_t length, Er= ror **errp) > { Why do you need compress_format as a separate parameter, when that information is redundant with the contents of format? > +void qmp_dump_guest_memory(bool paging, const char *protocol, bool has= _begin, > + int64_t begin, bool has_length, > + int64_t length, bool has_format, > + DumpGuestMemoryFormat format, Error **errp= ) > { > const char *p; > int fd =3D -1; > DumpState *s; > int ret; > + int compress_format =3D 0; You are using this as a bool, so type it as a bool rather than int. > +++ b/include/sysemu/dump.h > @@ -37,6 +37,13 @@ > #endif > =20 > /* > + * dump format > + */ > +#define FLAG_DUMP_COMPRESS_ZLIB (0x1) /* compressed with zlib */= > +#define FLAG_DUMP_COMPRESS_LZO (0x2) /* compressed with lzo */ > +#define FLAG_DUMP_COMPRESS_SNAPPY (0x4) /* compressed with snappy = */ Why are you skipping 3? Besides, these aren't flags that can be bitwise-or'd together, so the name FLAG_ is misleading. Why not just make it an enum, with value 0, 1, 2 (or with 1, 2, 3)? In fact, why even declare this at all, instead of reusing the DumpGuestMemoryFormat enum created for you by the QMP code generation from the schema? > + > +/* > * flag used in page desc of kdump-compressed format > */ > #define DUMP_DH_COMPRESSED_ZLIB (0x1) > @@ -46,6 +53,7 @@ > #define KDUMP_SIGNATURE "KDUMP " > #define SIG_LEN (sizeof(KDUMP_SIGNATURE) - 1) > #define DISKDUMP_HEADER_BLOCKS (1) > +#define PAGE_SIZE (4096) Is that true for all platforms? > +++ b/qapi-schema.json > @@ -2381,6 +2381,18 @@ > { 'command': 'device_del', 'data': {'id': 'str'} } > =20 > ## > +# @DumpGuestMemoryFormat > +# > +# Format of Guest memory dump > +# > +# Each represents a format of guest memory dump. > +# > +# Since: 1.6 > +## > +{ 'enum': 'DumpGuestMemoryFormat', > + 'data': [ 'elf', 'zlib', 'lzo', 'snappy' ] } For several other enums, we document what each enum value means here. That way, if the enum gets used more than once, you won't have to repeat.= =2E. > + > +## > # @dump-guest-memory > # > # Dump guest's memory to vmcore. It is a synchronous operation that ca= n take > @@ -2416,13 +2428,20 @@ > # want to dump all guest's memory, please specify the start @= begin > # and @length > # > +# @format: #optional if specified, the format of guest memory dump. > +# > +# 1. elf: dump file will be in elf format > +# 2. zlib: dump file will be in kdump format with zlib-comp= ressed > +# 3. lzo: dump file will be in kdump format with lzo-compre= ssed > +# 4. snappy: dump file will be in kdump format with snappy-= compressed =2E..this text in each other use of the enum. Also, if we extend the enu= m in the future, you don't have to revisit quite as much documentation. > +# > # Returns: nothing on success > # > -# Since: 1.2 > +# Since: 1.6 Not quite right. The dump-guest-memory command is still Since: 1.2. Rather, it is the @format: line that should add "(since 1.6)" to show that one additional option was added later. Look at drive-mirror's @buf-size for an example. Interface is looking better, though; thanks for folding in the review suggestions. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2KERKBELWCMOWLUESHRMF 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/ iQEcBAEBCAAGBQJRkvkaAAoJEKeha0olJ0Nqce4H/22UrPHHm5XXHOiKr6CpkCJK iacaAXNvbbz5anFuM+nGyrIc7ELzKIcxZo/zyTLPexhQCGb0lnnV7KyAESrpoZl6 xzzDT8uFaZ+ZMpa35zowFthQYGsZHCFxnFofkCt5k+ewRDO8JKGSPlsM8AGpwalG eJZ/fq9JC5KpQydQUdSigJ42mKZO9f8/ZFOEGUsa1dgsf2ccxk5ej4Q0X9olRzF4 avAIKUaVorhuuNrNyfH/FohqnGKqGkmd9FXIpXlQy5sK6Ny2Kw9aTaAQMwiIia15 yz5VKHb4ayoLjr/A8d8/wPCkicC0A8fQ860zNR2GrA4fPsK7G90fjdaf+hcqwik= =lXtX -----END PGP SIGNATURE----- ------enig2KERKBELWCMOWLUESHRMF--