Thanks for addressing my earlier comments. Some new ones below:
On 01/17/14 08:46, qiaonuohan wrote:
> + /* check whether lzo/snappy is supported */
> +#ifndef CONFIG_LZO
> + if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_LZO) {
> + error_setg(errp, "kdump-lzo is not available now");
> + }
> +#endif
> +
> +#ifndef CONFIG_SNAPPY
> + if (has_format && format == DUMP_GUEST_MEMORY_FORMAT_KDUMP_SNAPPY) {
> + error_setg(errp, "kdump-snappy is not available now");
> + }
> +#endif
Ekaterina caught in testing that these two blocks must be complemented
with a return statement each -- when you detect these errors,
qmp_dump_guest_memory() must not proceed. Apologies for not noticing
them in v6.
> +##
> # @dump-guest-memory
> #
> # Dump guest's memory to vmcore. It is a synchronous operation that can take
> @@ -2712,13 +2730,18 @@
> # want to dump all guest's memory, please specify the start @begin
> # and @length
> #
> +# @format: #optional if specified, the format of guest memory dump. But
> non-elf
> +# format is conflict with paging and filter, ie. @paging, @begin and
> +# @length is not allowed to be specified with @format at the same
> time
Please make it more precise with "non-elf", as in:
... are not allowed to be specified with *non-elf* @format at the same
time
Not really relevant but since you'll respin anyway... :)
> +# (since 2.0)
> +#
> # Returns: nothing on success
> #
> # Since: 1.2
> ##
> { 'command': 'dump-guest-memory',
> 'data': { 'paging': 'bool', 'protocol': 'str', '*begin': 'int',
> - '*length': 'int' } }
> + '*length': 'int', '*format': 'DumpGuestMemoryFormat' } }
>
> ##
> # @netdev_add:
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 02cc815..9158f22 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -791,8 +791,8 @@ EQMP
>
> {
> .name = "dump-guest-memory",
> - .args_type = "paging:b,protocol:s,begin:i?,end:i?",
> - .params = "-p protocol [begin] [length]",
> + .args_type = "paging:b,protocol:s,begin:i?,end:i?,format:s?",
> + .params = "-p protocol [begin] [length] [format]",
> .help = "dump guest memory to file",
> .user_print = monitor_user_noop,
> .mhandler.cmd_new = qmp_marshal_input_dump_guest_memory,
> @@ -813,6 +813,9 @@ Arguments:
> with length together (json-int)
> - "length": the memory size, in bytes. It's optional, and should be specified
> with begin together (json-int)
> +- "format": the format of guest memory dump. It's optional, and can be
> + elf|kdump-zlib|kdump-lzo|kdump-snappy, but non-elf formats will
> + conflict with paging and filter, ie. begin and
> length(json-string)
Please add a space between "length" and "(json-string)".
Thank you!
Laszlo
