On Thu, Feb 27, 2014 at 03:56:45PM +0800, qiaonuohan wrote:
> This patch is used to add dump_memory_format to qemu.conf and libvirt will use
> it to specify the default format in which qemu dumps guest's memory. But when
> "--compress" is specified with "virsh dump --memory-only", the format configured
> by dump_memory_format will be overrided. dump_memory_format can one of elf,
> kdump-zlib, kdump-lzo and kdump-snappy. And elf is the default value.
>
> Signed-off-by: Qiao Nuohan <qiaonuohan@xxxxxxxxxxxxxx>
> ---
> src/qemu/libvirtd_qemu.aug | 1 +
> src/qemu/qemu.conf | 13 ++++++++++++-
> src/qemu/qemu_conf.c | 3 +++
> src/qemu/qemu_conf.h | 2 ++
> src/qemu/qemu_driver.c | 37 ++++++++++++++++++++++++++++++++++---
> src/qemu/test_libvirtd_qemu.aug.in | 1 +
> 6 files changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> index a9ff421..d3704fc 100644
> --- a/src/qemu/libvirtd_qemu.aug
> +++ b/src/qemu/libvirtd_qemu.aug
> @@ -61,6 +61,7 @@ module Libvirtd_qemu =
> let save_entry = str_entry "save_image_format"
> | str_entry "dump_image_format"
> | str_entry "snapshot_image_format"
> + | str_entry "dump_memory_format"
> | str_entry "auto_dump_path"
> | bool_entry "auto_dump_bypass_cache"
> | bool_entry "auto_start_bypass_cache"
> diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> index e436084..06b1c5e 100644
> --- a/src/qemu/qemu.conf
> +++ b/src/qemu/qemu.conf
> @@ -292,7 +292,8 @@
> # dump_image_format is used when you use 'virsh dump' at emergency
> # crashdump, and if the specified dump_image_format is not valid, or
> # the requested compression program can't be found, this falls
> -# back to "raw" compression.
> +# back to "raw" compression. Note, dump_image_format doesn't work yet when you
> +# use "virsh dump --memory-only".
> #
> # snapshot_image_format specifies the compression algorithm of the memory save
> # image when an external snapshot of a domain is taken. This does not apply
> @@ -303,6 +304,16 @@
> #dump_image_format = "raw"
> #snapshot_image_format = "raw"
>
> +# Qemu can dump guest's memory in elf format or in kdump-compressed format, and
> +# the compression of kdump_compressed format can be zlib/lzo/snappy.
> +#
> +# dump_memory_format is used to specify the format in which qemu dumps guest's
> +# memory. However, if "--compress" is specified with 'virsh dump --memory-only',
> +# dump_memory_format will not work.
> +# dump_memory_format can be elf, kdump-zlib, kdump-lzo and kdump-snappy.
> +#
> +#dump_memory_format = "elf"
> +
> # When a domain is configured to be auto-dumped when libvirtd receives a
> # watchdog event from qemu guest, libvirtd will save dump files in directory
> # specified by auto_dump_path. Default value is /var/lib/libvirt/qemu/dump
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 2c397b0..c5014fc 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -296,6 +296,7 @@ static void virQEMUDriverConfigDispose(void *obj)
>
> VIR_FREE(cfg->saveImageFormat);
> VIR_FREE(cfg->dumpImageFormat);
> + VIR_FREE(cfg->dumpMemoryFormat);
> VIR_FREE(cfg->autoDumpPath);
>
> virStringFreeList(cfg->securityDriverNames);
> @@ -548,6 +549,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
> GET_VALUE_STR("dump_image_format", cfg->dumpImageFormat);
> GET_VALUE_STR("snapshot_image_format", cfg->snapshotImageFormat);
>
> + GET_VALUE_STR("dump_memory_format", cfg->dumpMemoryFormat);
> +
> GET_VALUE_STR("auto_dump_path", cfg->autoDumpPath);
> GET_VALUE_BOOL("auto_dump_bypass_cache", cfg->autoDumpBypassCache);
> GET_VALUE_BOOL("auto_start_bypass_cache", cfg->autoStartBypassCache);
> diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
> index c181dc2..7173e1a 100644
> --- a/src/qemu/qemu_conf.h
> +++ b/src/qemu/qemu_conf.h
> @@ -149,6 +149,8 @@ struct _virQEMUDriverConfig {
> char *dumpImageFormat;
> char *snapshotImageFormat;
>
> + char *dumpMemoryFormat;
> +
> char *autoDumpPath;
> bool autoDumpBypassCache;
> bool autoStartBypassCache;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e063a42..fd26e06 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3418,6 +3418,21 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm,
> return ret;
> }
>
> +typedef enum {
> + QEMU_DUMP_MEMORY_FORMAT_ELF = 0,
> + QEMU_DUMP_MEMORY_FORMAT_KDUMP_ZLIB = 1,
> + QEMU_DUMP_MEMORY_FORMAT_KDUMP_LZO = 2,
> + QEMU_DUMP_MEMORY_FORMAT_KDUMP_SNAPPY = 3,
> + QEMU_DUMP_MEMORY_FORMAT_LAST
> +} virQEMUDumpMemoryFormat;
> +
> +VIR_ENUM_DECL(qemuDumpMemoryFormat)
> +VIR_ENUM_IMPL(qemuDumpMemoryFormat, QEMU_DUMP_MEMORY_FORMAT_LAST,
> + "elf",
> + "kdump-zlib",
> + "kdump-lzo",
> + "kdump-snappy")
> +
> static int
> doCoreDump(virQEMUDriverPtr driver,
> virDomainObjPtr vm,
> @@ -3431,7 +3446,9 @@ doCoreDump(virQEMUDriverPtr driver,
> virFileWrapperFdPtr wrapperFd = NULL;
> int directFlag = 0;
> unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING;
> - const char *memory_dump_format;
> + const char *memory_dump_format = "elf";
> + virQEMUDriverConfigPtr cfg = NULL;
> + int dump_memory_format;
>
> /* Create an empty file with appropriate ownership. */
> if (dump_flags & VIR_DUMP_BYPASS_CACHE) {
> @@ -3461,8 +3478,22 @@ doCoreDump(virQEMUDriverPtr driver,
> memory_dump_format = "kdump-lzo";
> else if (dumpformat == VIR_MEMORY_DUMP_COMPRESS_SNAPPY)
> memory_dump_format = "kdump-snappy";
> - else
> - memory_dump_format = "elf";
> + else {
> + /* when --compress option is specified, qemu.conf will not work */
> + cfg = virQEMUDriverGetConfig(driver);
> + if (cfg->dumpMemoryFormat) {
> + dump_memory_format = qemuDumpMemoryFormatTypeFromString(
> + cfg->dumpMemoryFormat);
> + if (dump_memory_format == QEMU_DUMP_MEMORY_FORMAT_KDUMP_ZLIB)
> + memory_dump_format = "kdump-zlib";
> + else if (dump_memory_format ==
> + QEMU_DUMP_MEMORY_FORMAT_KDUMP_LZO)
> + memory_dump_format = "kdump-lzo";
> + else if (dump_memory_format ==
> + QEMU_DUMP_MEMORY_FORMAT_KDUMP_SNAPPY)
> + memory_dump_format = "kdump-snappy";
> + }
> + }
NACK to this change. We shouldn't invisibly change the behaviour of
public APIs based on qemu.conf configuration parameters. The app should
always be explicit about the format they want to produce.
Where a qemu.conf parameter could be valid is wrt automatically
triggered core dumps, which don't involve public API calls.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list