[libvirt] [PATCH v2 0/2] 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 v1 to v2: 1. address Daniel P. Berrange's comment about using a new parameter to replace flags like VIR_DUMP_COMPRESS_ZLIB. qiaonuohan (2): make qemu dump memory in kdump-compressed format add dump_memory_format in qemu.conf include/libvirt/libvirt.h.in | 15 +++++++++- src/driver.h | 3 +- src/libvirt.c | 10 +++++-- 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 | 61 +++++++++++++++++++++++++++++++++----- src/qemu/qemu_monitor.c | 6 ++-- 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_protocol.x | 1 + src/test/test_driver.c | 12 +++++++- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 +++++++++++++++++++++++++++- 17 files changed, 163 insertions(+), 22 deletions(-)
--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 used to add "--compress" and "[--compression-format] <string>" to "virsh dump --memory-only" and send dump-guest-memory command to qemu with dump format specified to one of elf, kdump-zlib, kdump-lzo and kdump-snappy. Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 15 ++++++++++++++- src/driver.h | 3 ++- src/libvirt.c | 10 ++++++++-- src/qemu/qemu_driver.c | 30 +++++++++++++++++++++-------- src/qemu/qemu_monitor.c | 6 +++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- src/remote/remote_protocol.x | 1 + src/test/test_driver.c | 12 +++++++++++- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 +++++++++++++++++++++++++++++++++++++++++++- 12 files changed, 113 insertions(+), 21 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..aae3c49 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1180,6 +1180,18 @@ typedef enum { VIR_DUMP_MEMORY_ONLY = (1 << 4), /* use dump-guest-memory */ } virDomainCoreDumpFlags; +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 */ @@ -1728,7 +1740,8 @@ int virDomainManagedSaveRemove(virDomainPtr dom, */ int virDomainCoreDump (virDomainPtr domain, const char *to, - unsigned int flags); + unsigned int flags, + const unsigned int memory_dump_format); /* * Screenshot of current domain console diff --git a/src/driver.h b/src/driver.h index 5f4cd8d..c228fd0 100644 --- a/src/driver.h +++ b/src/driver.h @@ -303,7 +303,8 @@ typedef int typedef int (*virDrvDomainCoreDump)(virDomainPtr domain, const char *to, - unsigned int flags); + unsigned int flags, + const unsigned int memory_dump_format); typedef char * (*virDrvDomainScreenshot)(virDomainPtr domain, diff --git a/src/libvirt.c b/src/libvirt.c index 9cc5b1c..0659baf 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2916,6 +2916,7 @@ error: * @domain: a domain object * @to: path for the core file * @flags: bitwise-OR of virDomainCoreDumpFlags + * @memory_dump_format: format of guest memory's dump * * This method will dump the core of a domain on a given file for analysis. * Note that for remote Xen Daemon the file path will be interpreted in @@ -2934,10 +2935,14 @@ error: * 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. * + * when @flags includes VIR_DUMP_MEMORY_ONLY and memory_dump_format is set, + * libvirt will ask qemu dump guest's memory in kdump-compressed format. + * * Returns 0 in case of success and -1 in case of failure. */ int -virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags) +virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags, + const unsigned int memory_dump_format) { virConnectPtr conn; @@ -2980,7 +2985,8 @@ virDomainCoreDump(virDomainPtr domain, const char *to, unsigned int flags) goto error; } - ret = conn->driver->domainCoreDump(domain, absolute_to, flags); + ret = conn->driver->domainCoreDump(domain, absolute_to, flags, + memory_dump_format); VIR_FREE(absolute_to); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 59e018d..7b6fa43 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3383,7 +3383,8 @@ cleanup: } static int qemuDumpToFd(virQEMUDriverPtr driver, virDomainObjPtr vm, - int fd, enum qemuDomainAsyncJob asyncJob) + int fd, enum qemuDomainAsyncJob asyncJob, + const char* dump_format) { qemuDomainObjPrivatePtr priv = vm->privateData; int ret = -1; @@ -3403,7 +3404,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, dump_format); qemuDomainObjExitMonitor(driver, vm); return ret; @@ -3414,13 +3415,15 @@ doCoreDump(virQEMUDriverPtr driver, virDomainObjPtr vm, const char *path, virQEMUSaveFormat compress, - unsigned int dump_flags) + unsigned int dump_flags, + const unsigned int memory_dump_format) { int fd = -1; int ret = -1; virFileWrapperFdPtr wrapperFd = NULL; int directFlag = 0; unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; + const char *dump_format; /* Create an empty file with appropriate ownership. */ if (dump_flags & VIR_DUMP_BYPASS_CACHE) { @@ -3444,7 +3447,16 @@ doCoreDump(virQEMUDriverPtr driver, goto cleanup; if (dump_flags & VIR_DUMP_MEMORY_ONLY) { - ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP); + if (memory_dump_format == VIR_MEMORY_DUMP_COMPRESS_ZLIB) + dump_format = "kdump-zlib"; + else if (memory_dump_format == VIR_MEMORY_DUMP_COMPRESS_LZO) + dump_format = "kdump-lzo"; + else if (memory_dump_format == VIR_MEMORY_DUMP_COMPRESS_SNAPPY) + dump_format = "kdump-snappy"; + else + dump_format = "elf"; + ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, + dump_format); } else { ret = qemuMigrationToFile(driver, vm, fd, 0, path, qemuCompressProgramName(compress), false, @@ -3509,7 +3521,8 @@ cleanup: static int qemuDomainCoreDump(virDomainPtr dom, const char *path, - unsigned int flags) + unsigned int flags, + const unsigned int memory_dump_format) { virQEMUDriverPtr driver = dom->conn->privateData; virDomainObjPtr vm; @@ -3557,7 +3570,8 @@ static int qemuDomainCoreDump(virDomainPtr dom, } } - ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags); + ret = doCoreDump(driver, vm, path, getCompressionType(driver), flags, + memory_dump_format); if (ret < 0) goto endjob; @@ -3734,7 +3748,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")); @@ -3798,7 +3812,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")); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a2769db..2722781 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2345,10 +2345,10 @@ int qemuMonitorMigrateCancel(qemuMonitorPtr mon) } int -qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd) +qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd, const char *dump_format) { int ret; - VIR_DEBUG("mon=%p fd=%d", mon, fd); + VIR_DEBUG("mon=%p fd=%d dump_format=%s", mon, fd, dump_format); if (!mon) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -2368,7 +2368,7 @@ qemuMonitorDumpToFd(qemuMonitorPtr mon, int fd) if (qemuMonitorSendFileHandle(mon, "dump", fd) < 0) return -1; - ret = qemuMonitorJSONDump(mon, "fd:dump"); + ret = qemuMonitorJSONDump(mon, "fd:dump", 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..f2e5763 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 *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..7c9625f 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 *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", 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..7691356 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 *dump_format); int qemuMonitorJSONGraphicsRelocate(qemuMonitorPtr mon, int type, diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f1f2359..cc6f68b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -902,6 +902,7 @@ struct remote_domain_core_dump_args { remote_nonnull_domain dom; remote_nonnull_string to; unsigned int flags; + unsigned int memory_dump_format; }; struct remote_domain_screenshot_args { diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..559933f 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -2429,7 +2429,8 @@ testDomainRestore(virConnectPtr conn, static int testDomainCoreDump(virDomainPtr domain, const char *to, - unsigned int flags) + unsigned int flags, + const unsigned int memory_dump_format) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,15 @@ static int testDomainCoreDump(virDomainPtr domain, } } + if (!(flags & VIR_DUMP_MEMORY_ONLY) && (memory_dump_format == + VIR_MEMORY_DUMP_COMPRESS_ZLIB || memory_dump_format == + VIR_MEMORY_DUMP_COMPRESS_LZO || memory_dump_format == + VIR_MEMORY_DUMP_COMPRESS_SNAPPY)) { + virReportSystemError(errno, + _("%s"), "memory_dump_format is available when " + "VIR_DUMP_MEMORY_ONLY is set"); + } + ret = 0; cleanup: VIR_FORCE_CLOSE(fd); diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index d7da5a8..c02016f 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_dump_format") GEN_TEST_FUNC(qemuMonitorJSONGraphicsRelocate, VIR_DOMAIN_GRAPHICS_TYPE_SPICE, "localhost", 12345, 12346, NULL) GEN_TEST_FUNC(qemuMonitorJSONAddNetdev, "some_dummy_netdevstr") diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index c3db94c..ba17d10 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4519,6 +4519,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} }; @@ -4533,6 +4541,9 @@ doDump(void *opaque) sigset_t sigmask, oldsigmask; const char *name = NULL; const char *to = NULL; + bool optCompress; + const char *compression_format = NULL; + unsigned int memory_dump_format = 0; unsigned int flags = 0; sigemptyset(&sigmask); @@ -4557,7 +4568,39 @@ doDump(void *opaque) if (vshCommandOptBool(cmd, "memory-only")) flags |= VIR_DUMP_MEMORY_ONLY; - if (virDomainCoreDump(dom, to, flags) < 0) { + 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 (virDomainCoreDump(dom, to, flags, memory_dump_format) < 0) { vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); goto out; }
On Tue, Feb 25, 2014 at 17:54:30 +0800, Qiao Nuohan 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 used to add "--compress" and "[--compression-format] <string>" to "virsh dump --memory-only" and send dump-guest-memory command to qemu with dump format specified to one of elf, kdump-zlib, kdump-lzo and kdump-snappy.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 15 ++++++++++++++- src/driver.h | 3 ++- src/libvirt.c | 10 ++++++++-- src/qemu/qemu_driver.c | 30 +++++++++++++++++++++-------- src/qemu/qemu_monitor.c | 6 +++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- src/remote/remote_protocol.x | 1 + src/test/test_driver.c | 12 +++++++++++- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 +++++++++++++++++++++++++++++++++++++++++++- 12 files changed, 113 insertions(+), 21 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..aae3c49 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1180,6 +1180,18 @@ typedef enum { VIR_DUMP_MEMORY_ONLY = (1 << 4), /* use dump-guest-memory */ } virDomainCoreDumpFlags;
+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 */ @@ -1728,7 +1740,8 @@ int virDomainManagedSaveRemove(virDomainPtr dom, */ int virDomainCoreDump (virDomainPtr domain, const char *to, - unsigned int flags); + unsigned int flags, + const unsigned int memory_dump_format);
This is not allowed and I believe "make syntax-check" should fail on the corresponding change to remote_protocol.x. If you want to add a new parameter to an existing API, you have keep the API as is and instead create a new API with a different name that will have all the parameters you need. Jirka
On 02/25/2014 06:56 PM, Jiri Denemark wrote:
On Tue, Feb 25, 2014 at 17:54:30 +0800, Qiao Nuohan 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 used to add "--compress" and "[--compression-format]<string>" to "virsh dump --memory-only" and send dump-guest-memory command to qemu with dump format specified to one of elf, kdump-zlib, kdump-lzo and kdump-snappy.
Signed-off-by: Qiao Nuohan<qiaonuohan@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 15 ++++++++++++++- src/driver.h | 3 ++- src/libvirt.c | 10 ++++++++-- src/qemu/qemu_driver.c | 30 +++++++++++++++++++++-------- src/qemu/qemu_monitor.c | 6 +++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- src/remote/remote_protocol.x | 1 + src/test/test_driver.c | 12 +++++++++++- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 +++++++++++++++++++++++++++++++++++++++++++- 12 files changed, 113 insertions(+), 21 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..aae3c49 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1180,6 +1180,18 @@ typedef enum { VIR_DUMP_MEMORY_ONLY = (1<< 4), /* use dump-guest-memory */ } virDomainCoreDumpFlags;
+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 */ @@ -1728,7 +1740,8 @@ int virDomainManagedSaveRemove(virDomainPtr dom, */ int virDomainCoreDump (virDomainPtr domain, const char *to, - unsigned int flags); + unsigned int flags, + const unsigned int memory_dump_format);
This is not allowed and I believe "make syntax-check" should fail on the corresponding change to remote_protocol.x. If you want to add a new parameter to an existing API, you have keep the API as is and instead create a new API with a different name that will have all the parameters you need.
Hello Jirka, Thanks for pointing it out for me, I finally found it's libvirt's policy never to change a committed public API. But driver handling methods are allowed to be changed, am I right? -- Regards Qiao Nuohan
On 02/26/2014 01:08 AM, Qiao Nuohan wrote:
int virDomainCoreDump (virDomainPtr domain, const char *to, - unsigned int flags); + unsigned int flags, + const unsigned int memory_dump_format);
This is not allowed and I believe "make syntax-check" should fail on the corresponding change to remote_protocol.x. If you want to add a new
I don't know if syntax-check would catch it, but the fact that you would have had to modify src/remote_protocol-structs is indeed a red flag that reviewers will catch.
parameter to an existing API, you have keep the API as is and instead create a new API with a different name that will have all the parameters you need.
Hello Jirka,
Thanks for pointing it out for me, I finally found it's libvirt's policy never to change a committed public API. But driver handling methods are allowed to be changed, am I right?
Changing driver handling methods is fine. That said, how are you going to provide the extra parameter to the driver handler without also having a public API? We have a script that validates that all the driver function names are close derivatives of the public API names, because that's the only way to programmatically enforce that we have proper ACL checks in all APIs. So really, the easiest would be adding a new API and new driver methods, with a better signature. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Wed, Feb 26, 2014 at 04:08:11PM +0800, Qiao Nuohan wrote:
On 02/25/2014 06:56 PM, Jiri Denemark wrote:
On Tue, Feb 25, 2014 at 17:54:30 +0800, Qiao Nuohan 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 used to add "--compress" and "[--compression-format]<string>" to "virsh dump --memory-only" and send dump-guest-memory command to qemu with dump format specified to one of elf, kdump-zlib, kdump-lzo and kdump-snappy.
Signed-off-by: Qiao Nuohan<qiaonuohan@cn.fujitsu.com> --- include/libvirt/libvirt.h.in | 15 ++++++++++++++- src/driver.h | 3 ++- src/libvirt.c | 10 ++++++++-- src/qemu/qemu_driver.c | 30 +++++++++++++++++++++-------- src/qemu/qemu_monitor.c | 6 +++--- src/qemu/qemu_monitor.h | 3 ++- src/qemu/qemu_monitor_json.c | 4 +++- src/qemu/qemu_monitor_json.h | 3 ++- src/remote/remote_protocol.x | 1 + src/test/test_driver.c | 12 +++++++++++- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 +++++++++++++++++++++++++++++++++++++++++++- 12 files changed, 113 insertions(+), 21 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..aae3c49 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1180,6 +1180,18 @@ typedef enum { VIR_DUMP_MEMORY_ONLY = (1<< 4), /* use dump-guest-memory */ } virDomainCoreDumpFlags;
+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 */ @@ -1728,7 +1740,8 @@ int virDomainManagedSaveRemove(virDomainPtr dom, */ int virDomainCoreDump (virDomainPtr domain, const char *to, - unsigned int flags); + unsigned int flags, + const unsigned int memory_dump_format);
This is not allowed and I believe "make syntax-check" should fail on the corresponding change to remote_protocol.x. If you want to add a new parameter to an existing API, you have keep the API as is and instead create a new API with a different name that will have all the parameters you need.
Hello Jirka,
Thanks for pointing it out for me, I finally found it's libvirt's policy never to change a committed public API. But driver handling methods are allowed to be changed, am I right?
Yes & no. There should be a 1-1 mapping of APIs in libvirt.h to driver callbacks defined in src/driver.h. The driver.h maps into the remote RPC protocol and that is considered part of the ABI and thus must not change. What is permitted is in the implementation of the new APIs is to share code with the impl of the old API. ie in qemu_driver.c 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 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..148a6bc 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 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 ecaaf81..7d225da 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 1f44a76..97b0f67 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 7b6fa43..99df954 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3410,6 +3410,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, @@ -3423,7 +3438,9 @@ doCoreDump(virQEMUDriverPtr driver, virFileWrapperFdPtr wrapperFd = NULL; int directFlag = 0; unsigned int flags = VIR_FILE_WRAPPER_NON_BLOCKING; - const char *dump_format; + const char *dump_format = "elf"; + virQEMUDriverConfigPtr cfg = NULL; + int dump_memory_format; /* Create an empty file with appropriate ownership. */ if (dump_flags & VIR_DUMP_BYPASS_CACHE) { @@ -3453,8 +3470,22 @@ doCoreDump(virQEMUDriverPtr driver, dump_format = "kdump-lzo"; else if (memory_dump_format == VIR_MEMORY_DUMP_COMPRESS_SNAPPY) dump_format = "kdump-snappy"; - else - 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) + dump_format = "kdump-zlib"; + else if (dump_memory_format == + QEMU_DUMP_MEMORY_FORMAT_KDUMP_LZO) + dump_format = "kdump-lzo"; + else if (dump_memory_format == + QEMU_DUMP_MEMORY_FORMAT_KDUMP_SNAPPY) + dump_format = "kdump-snappy"; + } + } ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, dump_format); } else { diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 81fedd6..9d0bcec 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -51,6 +51,7 @@ module Test_libvirtd_qemu = } { "save_image_format" = "raw" } { "dump_image_format" = "raw" } +{ "dump_memory_format" = "elf" } { "snapshot_image_format" = "raw" } { "auto_dump_path" = "/var/lib/libvirt/qemu/dump" } { "auto_dump_bypass_cache" = "0" }
On 02/25/2014 05:55 PM, Qiao Nuohan 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>
Sorry, the title should be [PATCH v2 2/2] add dump_memory_format in qemu.conf -- Regards Qiao Nuohan
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jiri Denemark -
Qiao Nuohan