On 05/14/2013 08:29 PM, Qiao Nuohan wrote: > Make monitor command 'dump-guest-memory' dump in kdump-compressed format. > 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. > > Note: > 1. The kdump-compressed format is readable only with the crash utility, and > it > can be smaller than the ELF format because of the compression support. > 2. The kdump-compressed format is the 5th edition. > > Signed-off-by: Qiao Nuohan <qiaonuo...@cn.fujitsu.com> > Signed-off-by: Zhang Xiaohe <zhan...@cn.fujitsu.com> > ---
> -static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
> - 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, Error
> **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 = -1;
> DumpState *s;
> int ret;
> + int compress_format = 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
>
> /*
> + * 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'} }
>
> ##
> +# @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...
> +
> +##
> # @dump-guest-memory
> #
> # Dump guest's memory to vmcore. It is a synchronous operation that can 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-compressed
> +# 3. lzo: dump file will be in kdump format with lzo-compressed
> +# 4. snappy: dump file will be in kdump format with
> snappy-compressed
...this text in each other use of the enum. Also, if we extend the enum
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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
