[libvirt] [PATCH v3 0/4] support dumping guest memory in compressed format

dumping guest's memroy is introduced without compression supported, and this is a freature regression of 'virsh dump --memory-only'. This patchset is used to add support in libvirt side to make qemu dump guest's memory in kdump-compressed format and please refer the following address to see implementation of the qemu side, the lastest version of qemu side is v9(ready for being queued). http://lists.nongnu.org/archive/html/qemu-devel/2014-02/msg03016.html ChangLog: Changes from v2 to v3: 1. address Jiri Denemark's comment about adding a new public API instead of changing an old one. Changes from v1 to v2: 1. address Daniel P. Berrange's comment about using a new parameter to replace flags like VIR_DUMP_COMPRESS_ZLIB. qiaonuohan (4): add new virDomainMemoryDump API wire up qemu agent to virDomainMemoryDump API allow "virsh dump --memory-only" specify dump format add dump_memory_format in qemu.conf include/libvirt/libvirt.h.in | 21 +++++++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 +++ src/driver.h | 7 +++ src/libvirt.c | 95 ++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++ 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 | 73 +++++++++++++++++++++++++---- src/qemu/qemu_monitor.c | 7 +-- src/qemu/qemu_monitor.h | 3 +- src/qemu/qemu_monitor_json.c | 4 +- src/qemu/qemu_monitor_json.h | 3 +- src/qemu/test_libvirtd_qemu.aug.in | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 +++++- src/remote_protocol-structs | 7 +++ src/test/test_driver.c | 17 ++++++- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 55 +++++++++++++++++++++- 22 files changed, 319 insertions(+), 24 deletions(-) -- 1.8.5.3

--memory-only option is introduced without compression supported. Therefore, this is a freature regression of virsh dump. Now qemu has support dumping memory in kdump-compressed format. This patch is adding new virDomainMemoryDump API, so that the format in which qemu dump domain's memory can be specified. Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>_ --- include/libvirt/libvirt.h.in | 21 ++++++++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 +++ src/driver.h | 7 ++++ src/libvirt.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++++++- src/remote_protocol-structs | 7 ++++ src/test/test_driver.c | 17 +++++++- 10 files changed, 172 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..ab6efca 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1180,6 +1180,19 @@ typedef enum { VIR_DUMP_MEMORY_ONLY = (1 << 4), /* use dump-guest-memory */ } virDomainCoreDumpFlags; +/* Domain memory dump's format */ +typedef enum { + VIR_MEMORY_DUMP_COMPRESS_ZLIB = 1, /* dump guest memory in + kdump-compressed format, with + zlib-compressed */ + VIR_MEMORY_DUMP_COMPRESS_LZO = 2, /* dump guest memory in + kdump-compressed format, with + lzo-compressed */ + VIR_MEMORY_DUMP_COMPRESS_SNAPPY = 3, /* dump guest memory in + kdump-compressed format, with + snappy-compressed */ +} virMemoryDumpFormat; + /* Domain migration flags. */ typedef enum { VIR_MIGRATE_LIVE = (1 << 0), /* live migration */ @@ -1731,6 +1744,14 @@ int virDomainCoreDump (virDomainPtr domain, unsigned int flags); /* + * Domain core dump + */ +int virDomainMemoryDump (virDomainPtr domain, + const char *to, + unsigned int flags, + unsigned int dumpformat); + +/* * Screenshot of current domain console */ char * virDomainScreenshot (virDomainPtr domain, diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index d517c66..af4f05e 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -42,7 +42,7 @@ VIR_ENUM_IMPL(virAccessPermDomain, "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", "block_read", "block_write", "mem_read", "open_graphics", "open_device", "screenshot", - "open_namespace"); + "open_namespace", "memory_dump"); VIR_ENUM_IMPL(virAccessPermInterface, VIR_ACCESS_PERM_INTERFACE_LAST, diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 6d14f05..2537fbf 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -205,6 +205,12 @@ typedef enum { VIR_ACCESS_PERM_DOMAIN_CORE_DUMP, /* Dump guest core */ /** + * @desc: Dump domain's memory only + * @message: Dumping domain's memory requires authorization + */ + VIR_ACCESS_PERM_DOMAIN_MEMORY_DUMP, /* Dump guest's memory only */ + + /** * @desc: Use domain power management * @message: Using domain power management requires authoriation */ diff --git a/src/driver.h b/src/driver.h index fbfaac4..7c001e6 100644 --- a/src/driver.h +++ b/src/driver.h @@ -306,6 +306,12 @@ typedef int const char *to, unsigned int flags); +typedef int +(*virDrvDomainMemoryDump)(virDomainPtr domain, + const char *to, + unsigned int flags, + unsigned int dumpformat); + typedef char * (*virDrvDomainScreenshot)(virDomainPtr domain, virStreamPtr stream, @@ -1200,6 +1206,7 @@ struct _virDriver { virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; virDrvDomainSaveImageDefineXML domainSaveImageDefineXML; virDrvDomainCoreDump domainCoreDump; + virDrvDomainMemoryDump domainMemoryDump; virDrvDomainScreenshot domainScreenshot; virDrvDomainSetVcpus domainSetVcpus; virDrvDomainSetVcpusFlags domainSetVcpusFlags; diff --git a/src/libvirt.c b/src/libvirt.c index dcf6b53..ec106a6 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2999,6 +2999,101 @@ error: return -1; } +/** + * virDomainMemoryDump: + * @domain: a domain object + * @to: path for the core file + * @flags: bitwise-OR of virDomainCoreDumpFlags + * @dumpformat: format of domain memory's dump + * + * This method will only dump the memory of a domain on a given file for + * analysis, so @flag must includes VIR_DUMP_MEMORY_ONLY. + * + * If @flags includes VIR_DUMP_CRASH, then leave the guest shut off with + * a crashed state after the dump completes. If @flags includes + * VIR_DUMP_LIVE, then make the core dump while continuing to allow + * the guest to run; otherwise, the guest is suspended during the dump. + * VIR_DUMP_RESET flag forces reset of the quest after dump. + * The above three flags are mutually exclusive. + * + * when @flags includes VIR_DUMP_MEMORY_ONLY and dumpformat is set, libvirt + * will ask qemu dump domain's memory in kdump-compressed format. + * + * Additionally, if @flags includes VIR_DUMP_BYPASS_CACHE, then libvirt + * will attempt to bypass the file system cache while creating the file, + * or fail if it cannot do so for the given system; this can allow less + * pressure on file system cache, but also risks slowing saves to NFS. + * + * Returns 0 in case of success and -1 in case of failure. + */ +int +virDomainMemoryDump(virDomainPtr domain, const char *to, unsigned int flags, + unsigned int dumpformat) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "to=%s, flags=%x", to, flags); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckNonNullArgGoto(to, error); + + if (!(flags & VIR_DUMP_MEMORY_ONLY)) { + virReportInvalidArg(flags, "%s", + _("memory-only flag is needed to dump domain's memory only")); + goto error; + } + + if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_LIVE)) { + virReportInvalidArg(flags, "%s", + _("crash and live flags are mutually exclusive")); + goto error; + } + + if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_RESET)) { + virReportInvalidArg(flags, "%s", + _("crash and reset flags are mutually exclusive")); + goto error; + } + + if ((flags & VIR_DUMP_LIVE) && (flags & VIR_DUMP_RESET)) { + virReportInvalidArg(flags, "%s", + _("live and reset flags are mutually exclusive")); + goto error; + } + + if (conn->driver->domainMemoryDump) { + int ret; + char *absolute_to; + + /* We must absolutize the file path as the save is done out of process */ + if (virFileAbsPath(to, &absolute_to) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("could not build absolute core file path")); + goto error; + } + + ret = conn->driver->domainMemoryDump(domain, absolute_to, flags, + dumpformat); + + VIR_FREE(absolute_to); + + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + +error: + virDispatchError(domain->conn); + return -1; +} + /** * virDomainScreenshot: diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 6ed6ce6..7135242 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -645,5 +645,10 @@ LIBVIRT_1.2.1 { virConnectNetworkEventDeregisterAny; } LIBVIRT_1.1.3; +LIBVIRT_1.2.3 { + global: + virDomainMemoryDump; +} LIBVIRT_1.2.1; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 955465a..c5772ed 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7516,6 +7516,7 @@ static virDriver remote_driver = { .domainSaveImageGetXMLDesc = remoteDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = remoteDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = remoteDomainCoreDump, /* 0.3.0 */ + .domainMemoryDump = remoteDomainMemoryDump, /* 1.2.3 */ .domainScreenshot = remoteDomainScreenshot, /* 0.9.2 */ .domainSetVcpus = remoteDomainSetVcpus, /* 0.3.0 */ .domainSetVcpusFlags = remoteDomainSetVcpusFlags, /* 0.8.5 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f1f2359..680456f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -904,6 +904,13 @@ struct remote_domain_core_dump_args { unsigned int flags; }; +struct remote_domain_memory_dump_args { + remote_nonnull_domain dom; + remote_nonnull_string to; + unsigned int flags; + unsigned int dumpformat; +}; + struct remote_domain_screenshot_args { remote_nonnull_domain dom; unsigned int screen; @@ -5262,5 +5269,11 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333 + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333, + + /** + * @generate: both + * @acl: domain:memory_dump + */ + REMOTE_PROC_DOMAIN_MEMORY_DUMP = 334 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 5636d55..01a9dbe 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -557,6 +557,12 @@ struct remote_domain_core_dump_args { remote_nonnull_string to; u_int flags; }; +struct remote_domain_memory_dump_args { + remote_nonnull_domain dom; + remote_nonnull_string to; + u_int flags; + u_int dumpformat; +}; struct remote_domain_screenshot_args { remote_nonnull_domain dom; u_int screen; @@ -2755,4 +2761,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_BALLOON_CHANGE = 331, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_PMSUSPEND_DISK = 332, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVED = 333, + REMOTE_PROC_DOMAIN_MEMORY_DUMP = 334, }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..cf93f97 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2427,9 +2427,10 @@ testDomainRestore(virConnectPtr conn, return testDomainRestoreFlags(conn, path, NULL, 0); } -static int testDomainCoreDump(virDomainPtr domain, +static int testDomainMemoryDump(virDomainPtr domain, const char *to, - unsigned int flags) + unsigned int flags, + unsigned int dumpformat) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,12 @@ static int testDomainCoreDump(virDomainPtr domain, } } + if (dumpformat > VIR_MEMORY_DUMP_COMPRESS_SNAPPY) { + virReportSystemError(errno, + _("invalid value of dumpformat: %d"), dumpformat); + goto cleanup; + } + ret = 0; cleanup: VIR_FORCE_CLOSE(fd); @@ -2490,6 +2497,12 @@ cleanup: return ret; } +static int testDomainCoreDump(virDomainPtr domain, + const char *to, + unsigned int flags) { + return testDomainMemoryDump(domain, to, flags, 0); +} + static char *testDomainGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) { char *ret; -- 1.8.5.3

On Thu, Feb 27, 2014 at 03:56:42PM +0800, qiaonuohan wrote:
--memory-only option is introduced without compression supported. Therefore, this is a freature regression of virsh dump. Now qemu has support dumping memory in kdump-compressed format. This patch is adding new virDomainMemoryDump API, so that the format in which qemu dump domain's memory can be specified.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>_ --- include/libvirt/libvirt.h.in | 21 ++++++++++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 6 +++ src/driver.h | 7 ++++ src/libvirt.c | 95 ++++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++++++- src/remote_protocol-structs | 7 ++++ src/test/test_driver.c | 17 +++++++- 10 files changed, 172 insertions(+), 4 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..ab6efca 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1180,6 +1180,19 @@ typedef enum { VIR_DUMP_MEMORY_ONLY = (1 << 4), /* use dump-guest-memory */ } virDomainCoreDumpFlags;
+/* Domain memory dump's format */ +typedef enum { + VIR_MEMORY_DUMP_COMPRESS_ZLIB = 1, /* dump guest memory in + kdump-compressed format, with + zlib-compressed */ + VIR_MEMORY_DUMP_COMPRESS_LZO = 2, /* dump guest memory in + kdump-compressed format, with + lzo-compressed */ + VIR_MEMORY_DUMP_COMPRESS_SNAPPY = 3, /* dump guest memory in + kdump-compressed format, with + snappy-compressed */ +} virMemoryDumpFormat;
s/virMemoryDumpFormat/virDomainCoreDumpFormat/ No need to assign values to each entry here since this is a plain enum, not bitflags. Also s/MEMORY_COMPRESS/DOMAIN_CORE_FORMAT/ for every member And add VIR_MEMORY_DUMP_FORMAT_RAW for the non-compressed format, which probably ought to be the default (ie first).
+ /* Domain migration flags. */ typedef enum { VIR_MIGRATE_LIVE = (1 << 0), /* live migration */ @@ -1731,6 +1744,14 @@ int virDomainCoreDump (virDomainPtr domain, unsigned int flags);
/* + * Domain core dump + */ +int virDomainMemoryDump (virDomainPtr domain, + const char *to, + unsigned int flags, + unsigned int dumpformat);
Convention is that the 'flags' parameter should always be the very last one. The name would be better as virDomainCoreDumpWithFormat IMHO to show it is an extension of virDomainCoreDump.
diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 6d14f05..2537fbf 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -205,6 +205,12 @@ typedef enum { VIR_ACCESS_PERM_DOMAIN_CORE_DUMP, /* Dump guest core */
/** + * @desc: Dump domain's memory only + * @message: Dumping domain's memory requires authorization + */ + VIR_ACCESS_PERM_DOMAIN_MEMORY_DUMP, /* Dump guest's memory only */ + + /**
There's no point in inventing a new permission - the same permission as the existing API is more than OK.
+/** + * virDomainMemoryDump: + * @domain: a domain object + * @to: path for the core file + * @flags: bitwise-OR of virDomainCoreDumpFlags + * @dumpformat: format of domain memory's dump + * + * This method will only dump the memory of a domain on a given file for + * analysis, so @flag must includes VIR_DUMP_MEMORY_ONLY.
This restricton seems rather dubious. This new method's functionality should be a super-set of virDomainCoreDump - ie everything that the old method could do, should be possible in the new method. All we do is add the ability to choose between formats. 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 :|

On 02/27/2014 07:43 AM, Daniel P. Berrange wrote:
+/* Domain memory dump's format */
/* */ comments don't make it through to the html doc pages. Here, you want something like: /** * virDomainCoreDumpFormat: * Values for specifying different formats of domain core dumps. */
+typedef enum { + VIR_MEMORY_DUMP_COMPRESS_ZLIB = 1, /* dump guest memory in + kdump-compressed format, with + zlib-compressed */ + VIR_MEMORY_DUMP_COMPRESS_LZO = 2, /* dump guest memory in + kdump-compressed format, with + lzo-compressed */ + VIR_MEMORY_DUMP_COMPRESS_SNAPPY = 3, /* dump guest memory in + kdump-compressed format, with + snappy-compressed */ +} virMemoryDumpFormat;
Missing a *_LAST enum value, under proper #ifdef.
s/virMemoryDumpFormat/virDomainCoreDumpFormat/
No need to assign values to each entry here since this is a plain enum, not bitflags.
Also s/MEMORY_COMPRESS/DOMAIN_CORE_FORMAT/ for every member
And add VIR_MEMORY_DUMP_FORMAT_RAW for the non-compressed format, which probably ought to be the default (ie first).
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

This patch makes use of the QEMU guest agent to implement the virDomainMemoryDump API. Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++---------- src/qemu/qemu_monitor.c | 7 ++++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9a865e..e063a42 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3391,7 +3391,8 @@ cleanup: } static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, - int fd, enum qemuDomainAsyncJob asyncJob) + int fd, enum qemuDomainAsyncJob asyncJob, + const char* memory_dump_format) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; @@ -3411,7 +3412,7 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1; - ret = qemuMonitorDumpToFd(priv->mon, fd); + ret = qemuMonitorDumpToFd(priv->mon, fd, memory_dump_format); qemuDomainObjExitMonitor(driver, vm); return ret; @@ -3422,13 +3423,15 @@ doCoreDump(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *path, virQEMUSaveFormat compress, - unsigned int dump_flags) + unsigned int dump_flags, + unsigned int dumpformat) { int fd = -1; int ret = -1; virFileWrapperFdPtr wrapperFd = NULL; int directFlag = 0; unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; + const char *memory_dump_format; /* Create an empty file with appropriate ownership. */ if (dump_flags & VIR_DUMP_BYPASS_CACHE) { @@ -3452,7 +3455,16 @@ doCoreDump(virQEMUDriverPtr driver, goto cleanup; if (dump_flags & VIR_DUMP_MEMORY_ONLY) { - ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP); + if (dumpformat == VIR_MEMORY_DUMP_COMPRESS_ZLIB) + memory_dump_format = "kdump-zlib"; + else if (dumpformat == VIR_MEMORY_DUMP_COMPRESS_LZO) + memory_dump_format = "kdump-lzo"; + else if (dumpformat == VIR_MEMORY_DUMP_COMPRESS_SNAPPY) + memory_dump_format = "kdump-snappy"; + else + memory_dump_format = "elf"; + ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, + memory_dump_format); } else { ret = qemuMigrationToFile(driver, vm, fd, 0, path, qemuCompressProgramName(compress), false, @@ -3515,9 +3527,10 @@ cleanup: return ret; } -static int qemuDomainCoreDump(virDomainPtr dom, +static int qemuDomainMemoryDump(virDomainPtr dom, const char *path, - unsigned int flags) + unsigned int flags, + unsigned int dumpformat) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3533,7 +3546,7 @@ static int qemuDomainCoreDump(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) return -1; - if (virDomainCoreDumpEnsureACL(dom->conn, vm->def) < 0) + if (virDomainMemoryDumpEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (qemuDomainObjBeginAsyncJob(driver, vm, @@ -3565,7 +3578,8 @@ static int qemuDomainCoreDump(virDomainPtr dom, } } - ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags); + ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags, + dumpformat); if (ret < 0) goto endjob; @@ -3619,6 +3633,13 @@ cleanup: return ret; } +static int qemuDomainCoreDump(virDomainPtr dom, + const char *path, + unsigned int flags) +{ + return qemuDomainMemoryDump(dom, path, flags, 0); +} + static char * qemuDomainScreenshot(virDomainPtr dom, virStreamPtr st, @@ -3742,7 +3763,7 @@ static void processWatchdogEvent(virQEMUDriverPtr driver, virDomainObjPtr vm, in flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0; ret = doCoreDump(driver, vm, dumpfile, - getCompressionType(driver), flags); + getCompressionType(driver), flags, 0); if (ret < 0) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Dump failed")); @@ -3806,7 +3827,7 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0; ret = doCoreDump(driver, vm, dumpfile, - getCompressionType(driver), flags); + getCompressionType(driver), flags, 0); if (ret < 0) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Dump failed")); @@ -16610,6 +16631,7 @@ static virDriver qemuDriver = { .domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = qemuDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = qemuDomainCoreDump, /* 0.7.0 */ + .domainMemoryDump = qemuDomainMemoryDump, /* 1.2.3 */ .domainScreenshot = qemuDomainScreenshot, /* 0.9.2 */ .domainSetVcpus = qemuDomainSetVcpus, /* 0.4.4 */ .domainSetVcpusFlags = qemuDomainSetVcpusFlags, /* 0.8.5 */ diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a2769db..bcb457f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2345,10 +2345,11 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon) } int -qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd) +qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, const char* memory_dump_format) { int ret; - VIR_DEBUG("mon=%p fd=%d", mon, fd); + VIR_DEBUG("mon=%p fd=%d memory_dump_format=%s", mon, fd, + memory_dump_format); if (!mon) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -2368,7 +2369,7 @@ qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd) if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0) return -1; - ret = qemuMonitorJSONDump(mon, "fd:dump"); + ret = qemuMonitorJSONDump(mon, "fd:dump", memory_dump_format); if (ret < 0) { if (qemuMonitorCloseFileHandle(mon, "dump") < 0) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index eabf000..a6fc8ee 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -495,7 +495,8 @@ int qemuMonitorMigrateToUnix(qemuMonitorPtr mon, int qemuMonitorMigrateCancel(qemuMonitorPtr mon); int qemuMonitorDumpToFd(qemuMonitorPtr mon, - int fd); + int fd, + const char* memory_dump_format); int qemuMonitorGraphicsRelocate(qemuMonitorPtr mon, int type, diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5e825ac..f77b35a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2636,7 +2636,8 @@ int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon) int qemuMonitorJSONDump(qemuMonitorPtr mon, - const char *protocol) + const char *protocol, + const char *memory_dump_format) { int ret; virJSONValuePtr cmd = NULL; @@ -2645,6 +2646,7 @@ qemuMonitorJSONDump(qemuMonitorPtr mon, cmd = qemuMonitorJSONMakeCommand("dump-guest-memory", "b:paging", false, "s:protocol", protocol, + "s:format", memory_dump_format, NULL); if (!cmd) return -1; diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index a93c51e..af6f666 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -148,7 +148,8 @@ int qemuMonitorJSONGetSpiceMigrationStatus(qemuMonitorPtr mon, int qemuMonitorJSONMigrateCancel(qemuMonitorPtr mon); int qemuMonitorJSONDump(qemuMonitorPtr mon, - const char *protocol); + const char *protocol, + const char *memory_dump_format); int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon, int type, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index d7da5a8..f21de84 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -1154,7 +1154,7 @@ GEN_TEST_FUNC(qemuMonitorJSONSetMigrationDowntime, 1) GEN_TEST_FUNC(qemuMonitorJSONMigrate, QEMU_MONITOR_MIGRATE_BACKGROUND | QEMU_MONITOR_MIGRATE_NON_SHARED_DISK | QEMU_MONITOR_MIGRATE_NON_SHARED_INC, "tcp:localhost:12345") -GEN_TEST_FUNC(qemuMonitorJSONDump, "dummy_protocol") +GEN_TEST_FUNC(qemuMonitorJSONDump, "dummy_protocol", "dummy_memory_dump_format") GEN_TEST_FUNC(qemuMonitorJSONGraphicsRelocate, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, "localhost", 12345, 12346, NULL) GEN_TEST_FUNC(qemuMonitorJSONAddNetdev, "some_dummy_netdevstr") -- 1.8.5.3

On Thu, Feb 27, 2014 at 03:56:43PM +0800, qiaonuohan wrote:
This patch makes use of the QEMU guest agent to implement the virDomainMemoryDump API.
Unless I'm missing something this doesn't appear to involve the QEMU guest agent, at least not from libvirt's POV. This code is using the QEMU monitor APIs.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- src/qemu/qemu_driver.c | 42 ++++++++++++++++++++++++++++++++---------- src/qemu/qemu_monitor.c | 7 ++++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- tests/qemumonitorjsontest.c | 2 +- 6 files changed, 44 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9a865e..e063a42 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3391,7 +3391,8 @@ cleanup: }
static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, - int fd, enum qemuDomainAsyncJob asyncJob) + int fd, enum qemuDomainAsyncJob asyncJob, + const char* memory_dump_format) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; @@ -3411,7 +3412,7 @@ static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) return -1;
- ret = qemuMonitorDumpToFd(priv->mon, fd); + ret = qemuMonitorDumpToFd(priv->mon, fd, memory_dump_format); qemuDomainObjExitMonitor(driver, vm);
return ret; @@ -3422,13 +3423,15 @@ doCoreDump(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *path, virQEMUSaveFormat compress, - unsigned int dump_flags) + unsigned int dump_flags, + unsigned int dumpformat) { int fd = -1; int ret = -1; virFileWrapperFdPtr wrapperFd = NULL; int directFlag = 0; unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; + const char *memory_dump_format;
/* Create an empty file with appropriate ownership. */ if (dump_flags & VIR_DUMP_BYPASS_CACHE) { @@ -3452,7 +3455,16 @@ doCoreDump(virQEMUDriverPtr driver, goto cleanup;
if (dump_flags & VIR_DUMP_MEMORY_ONLY) { - ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP); + if (dumpformat == VIR_MEMORY_DUMP_COMPRESS_ZLIB) + memory_dump_format = "kdump-zlib"; + else if (dumpformat == VIR_MEMORY_DUMP_COMPRESS_LZO) + memory_dump_format = "kdump-lzo"; + else if (dumpformat == VIR_MEMORY_DUMP_COMPRESS_SNAPPY) + memory_dump_format = "kdump-snappy"; + else + memory_dump_format = "elf";
There should be an explicit check for the ELF format, and the use virReportError(VIR_ERR_INVALID_ARG) if the user supplied a format we don't know about.
+ ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, + memory_dump_format); } else { ret = qemuMigrationToFile(driver, vm, fd, 0, path, qemuCompressProgramName(compress), false, @@ -3515,9 +3527,10 @@ cleanup: return ret; }
-static int qemuDomainCoreDump(virDomainPtr dom, +static int qemuDomainMemoryDump(virDomainPtr dom, const char *path, - unsigned int flags) + unsigned int flags, + unsigned int dumpformat) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3533,7 +3546,7 @@ static int qemuDomainCoreDump(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) return -1;
- if (virDomainCoreDumpEnsureACL(dom->conn, vm->def) < 0) + if (virDomainMemoryDumpEnsureACL(dom->conn, vm->def) < 0) goto cleanup;
if (qemuDomainObjBeginAsyncJob(driver, vm, @@ -3565,7 +3578,8 @@ static int qemuDomainCoreDump(virDomainPtr dom, } }
- ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags); + ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags, + dumpformat); if (ret < 0) goto endjob;
@@ -3619,6 +3633,13 @@ cleanup: return ret; }
+static int qemuDomainCoreDump(virDomainPtr dom, + const char *path, + unsigned int flags) +{ + return qemuDomainMemoryDump(dom, path, flags, 0); +}
This should use a enum constant from the public header rather than 0 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 :|

This patch is used to add "--compress" and "[--compression-format] <string>" to "virsh dump --memory-only". And "virsh dump --memory-only" is going be implemented by new virDomainMemoryDump API. Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- tools/virsh-domain.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2e3f0ed..14ef612 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4486,6 +4486,14 @@ static const vshCmdOptDef opts_dump[] = { .type = VSH_OT_BOOL, .help = N_("dump domain's memory only") }, + {.name = "compress", + .type = VSH_OT_BOOL, + .help = N_("make qemu dump domain's memory in kdump-compressed format") + }, + {.name = "compression-format", + .type = VSH_OT_DATA, + .help = N_("specify the compression format of kdump-compressed format") + }, {.name = NULL} }; @@ -4501,6 +4509,9 @@ doDump(void *opaque) const char *name = NULL; const char *to = NULL; unsigned int flags = 0; + bool optCompress; + const char *compression_format = NULL; + unsigned int memory_dump_format = 0; sigemptyset(&sigmask); sigaddset(&sigmask, SIGINT); @@ -4524,11 +4535,51 @@ doDump(void *opaque) if (vshCommandOptBool(cmd, "memory-only")) flags |= VIR_DUMP_MEMORY_ONLY; - if (virDomainCoreDump(dom, to, flags) < 0) { - vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); + optCompress = vshCommandOptBool(cmd, "compress"); + if (optCompress && !(flags & VIR_DUMP_MEMORY_ONLY)) { + vshError(ctl, "%s", + _("compress flag cannot be set without memory-only flag")); goto out; } + if (vshCommandOptString(cmd, "compression-format", &compression_format)) { + if (!optCompress) { + vshError(ctl, "%s", + _("compression-format cannot be set without compress " + "flag")); + goto out; + } + + if (STREQ(compression_format, "zlib")) + memory_dump_format = VIR_MEMORY_DUMP_COMPRESS_ZLIB; + else if (STREQ(compression_format, "lzo")) + memory_dump_format = VIR_MEMORY_DUMP_COMPRESS_LZO; + else if (STREQ(compression_format, "snappy")) + memory_dump_format = VIR_MEMORY_DUMP_COMPRESS_SNAPPY; + else { + vshError(ctl, _("compression format '%s' is not supported, " + "expecting 'zlib', 'lzo' or 'snappy'."), + compression_format); + goto out; + } + } else { + if (optCompress) + memory_dump_format = VIR_MEMORY_DUMP_COMPRESS_ZLIB; + } + + if (flags & VIR_DUMP_MEMORY_ONLY) { + if (virDomainMemoryDump(dom, to, flags, memory_dump_format) < 0) { + vshError(ctl, _("Failed to dump the memory of domain %s to %s"), + name, to); + goto out; + } + } else { + if (virDomainCoreDump(dom, to, flags) < 0) { + vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); + goto out; + } + } + ret = '0'; out: pthread_sigmask(SIG_SETMASK, &oldsigmask, NULL); -- 1.8.5.3

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@cn.fujitsu.com> --- 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"; + } + } ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, memory_dump_format); } else { diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 81fedd6..f35bf10 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -52,6 +52,7 @@ module Test_libvirtd_qemu = { "save_image_format" = "raw" } { "dump_image_format" = "raw" } { "snapshot_image_format" = "raw" } +{ "dump_memory_format" = "elf" } { "auto_dump_path" = "/var/lib/libvirt/qemu/dump" } { "auto_dump_bypass_cache" = "0" } { "auto_start_bypass_cache" = "0" } -- 1.8.5.3

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@cn.fujitsu.com> --- 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 :|

On 02/27/2014 07:55 AM, Daniel P. Berrange wrote:
+ 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.
Arguably, if you support value '0' as an explicit "use the default format from the .conf file", and '1' as "raw", then you could set it up so that applications have explicit control over all formats when desired, but can use the .conf file default when they don't care. But again, this would have to be explicit by having an enum value in the .h file that explicitly states the user wants to use a .conf file setting (without regards to its value) instead of their own explicit value.
Where a qemu.conf parameter could be valid is wrt automatically triggered core dumps, which don't involve public API calls.
And even though I suggested the possibility of an explicit "use the .conf file default", it sounds more complex; sometimes simpler is better, and Dan's suggestion of not allowing the explicit API to ever be influenced by the .conf file is also reasonable. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Thu, Feb 27, 2014 at 08:52:19AM -0700, Eric Blake wrote:
On 02/27/2014 07:55 AM, Daniel P. Berrange wrote:
+ 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.
Arguably, if you support value '0' as an explicit "use the default format from the .conf file", and '1' as "raw", then you could set it up so that applications have explicit control over all formats when desired, but can use the .conf file default when they don't care. But again, this would have to be explicit by having an enum value in the .h file that explicitly states the user wants to use a .conf file setting (without regards to its value) instead of their own explicit value.
While you could do that, I just don't see the value in having an option where the data you get back is in an arbitrary format you can't predict
Where a qemu.conf parameter could be valid is wrt automatically triggered core dumps, which don't involve public API calls.
And even though I suggested the possibility of an explicit "use the .conf file default", it sounds more complex; sometimes simpler is better, and Dan's suggestion of not allowing the explicit API to ever be influenced by the .conf file is also reasonable.
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 :|

On 02/27/2014 08:55 AM, Daniel P. Berrange wrote:
Arguably, if you support value '0' as an explicit "use the default format from the .conf file", and '1' as "raw", then you could set it up so that applications have explicit control over all formats when desired, but can use the .conf file default when they don't care. But again, this would have to be explicit by having an enum value in the .h file that explicitly states the user wants to use a .conf file setting (without regards to its value) instead of their own explicit value.
While you could do that, I just don't see the value in having an option where the data you get back is in an arbitrary format you can't predict
If we support this mode, then the return value has to be the enum value of what format was actually chosen, rather than a simpler -1/0 return. The more complex this sounds, the more I like Dan's preference for simpler usage. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange
-
Eric Blake
-
qiaonuohan