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