[libvirt] [PATCH v4 0/3] 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 v3 to v4: 1. dropping patch "add dump_memory_format in qemu.conf" 2. fix to follow some conventions 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 (3): add new virDomainCoreDumpWithFormat API add qemu support to virDomainCoreDumpWithFormat API allow "virsh dump --memory-only" specify dump format include/libvirt/libvirt.h.in | 31 ++++++++++++++ src/driver.h | 7 +++ src/libvirt.c | 100 +++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/qemu/qemu_driver.c | 45 +++++++++++++++---- 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/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++++++- src/remote_protocol-structs | 7 +++ src/test/test_driver.c | 19 ++++++-- tests/qemumonitorjsontest.c | 2 +- tools/virsh-domain.c | 45 ++++++++++++++++++- 15 files changed, 273 insertions(+), 21 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 virDomainCoreDumpWithFormat 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 | 31 ++++++++++++++ src/driver.h | 7 +++ src/libvirt.c | 100 +++++++++++++++++++++++++++++++++++++++++++ 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 | 19 ++++++-- 8 files changed, 181 insertions(+), 4 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..12d64ab 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1180,6 +1180,29 @@ typedef enum { VIR_DUMP_MEMORY_ONLY = (1 << 4), /* use dump-guest-memory */ } virDomainCoreDumpFlags; +/** + * virDomainCoreDumpFormat: + * + * Values for specifying different formats of domain core dumps. + */ +typedef enum { + VIR_DUMP_FORMAT_RAW, /* dump guest memory in raw format */ + VIR_DUMP_FORMAT_KDUMP_ZLIB, /* dump guest memory in kdump-compressed + format, with zlib-compressed */ + VIR_DUMP_FORMAT_KDUMP_LZO, /* dump guest memory in kdump-compressed + format, with lzo-compressed */ + VIR_DUMP_FORMAT_KDUMP_SNAPPY, /* dump guest memory in kdump-compressed + format, with snappy-compressed */ +#ifdef VIR_ENUM_SENTINELS + VIR_DUMP_FORMAT_LAST + /* + * NB: this enum value will increase over time as new events are + * added to the libvirt API. It reflects the last state supported + * by this version of the libvirt API. + */ +#endif +} virDomainCoreDumpFormat; + /* Domain migration flags. */ typedef enum { VIR_MIGRATE_LIVE = (1 << 0), /* live migration */ @@ -1731,6 +1754,14 @@ int virDomainCoreDump (virDomainPtr domain, unsigned int flags); /* + * Domain core dump with format specified + */ +int virDomainCoreDumpWithFormat (virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags); + +/* * Screenshot of current domain console */ char * virDomainScreenshot (virDomainPtr domain, diff --git a/src/driver.h b/src/driver.h index fbfaac4..d613f64 100644 --- a/src/driver.h +++ b/src/driver.h @@ -306,6 +306,12 @@ typedef int const char *to, unsigned int flags); +typedef int +(*virDrvDomainCoreDumpWithFormat)(virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags); + typedef char * (*virDrvDomainScreenshot)(virDomainPtr domain, virStreamPtr stream, @@ -1200,6 +1206,7 @@ struct _virDriver { virDrvDomainSaveImageGetXMLDesc domainSaveImageGetXMLDesc; virDrvDomainSaveImageDefineXML domainSaveImageDefineXML; virDrvDomainCoreDump domainCoreDump; + virDrvDomainCoreDumpWithFormat domainCoreDumpWithFormat; virDrvDomainScreenshot domainScreenshot; virDrvDomainSetVcpus domainSetVcpus; virDrvDomainSetVcpusFlags domainSetVcpusFlags; diff --git a/src/libvirt.c b/src/libvirt.c index dcf6b53..5a6a576 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2999,6 +2999,106 @@ error: return -1; } +/** + * virDomainCoreDumpWithFormat: + * @domain: a domain object + * @to: path for the core file + * @dumpformat: format of domain memory's dump + * @flags: bitwise-OR of virDomainCoreDumpFlags + * + * 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 + * the remote host. Hypervisors may require the user to manually ensure + * proper permissions on the file named by @to. + * + * If @flags includes VIR_DUMP_MEMORY_ONLY and dumpformat is set, libvirt + * will ask qemu dump domain's memory in kdump-compressed format. + * + * 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. + * + * 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 +virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to, unsigned int + dumpformat, unsigned int flags) +{ + 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) && + (dumpformat == VIR_DUMP_FORMAT_KDUMP_ZLIB || + dumpformat == VIR_DUMP_FORMAT_KDUMP_LZO || + dumpformat == VIR_DUMP_FORMAT_KDUMP_SNAPPY)) { + virReportInvalidArg(flags, "%s", + _("compression format is only work with 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->domainCoreDumpWithFormat) { + 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->domainCoreDumpWithFormat(domain, absolute_to, + dumpformat, flags); + + 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..9ab0c92 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: + virDomainCoreDumpWithFormat; +} 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..0a15267 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 */ + .domainCoreDumpWithFormat = remoteDomainCoreDumpWithFormat, /* 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..6c445cc 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_core_dump_with_format_args { + remote_nonnull_domain dom; + remote_nonnull_string to; + unsigned int dumpformat; + unsigned int flags; +}; + 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:core_dump + */ + REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 5636d55..0e2101c 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_core_dump_with_format_args { + remote_nonnull_domain dom; + remote_nonnull_string to; + u_int dompformat; + u_int flags; +}; 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_CORE_DUMP_WITH_FORMAT = 334, }; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..605b0d1 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, - const char *to, - unsigned int flags) +static int testDomainCoreDumpWithFormat(virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,12 @@ static int testDomainCoreDump(virDomainPtr domain, } } + if (dumpformat > VIR_DUMP_FORMAT_KDUMP_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 testDomainCoreDumpWithFormat(domain, to, VIR_DUMP_FORMAT_RAW, flags); +} + static char *testDomainGetOSType(virDomainPtr dom ATTRIBUTE_UNUSED) { char *ret; -- 1.8.5.3
On Mon, Mar 03, 2014 at 10:27:24AM +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 virDomainCoreDumpWithFormat 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 | 31 ++++++++++++++ src/driver.h | 7 +++ src/libvirt.c | 100 +++++++++++++++++++++++++++++++++++++++++++ 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 | 19 ++++++-- 8 files changed, 181 insertions(+), 4 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 295d551..12d64ab 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in
diff --git a/src/libvirt.c b/src/libvirt.c index dcf6b53..5a6a576 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2999,6 +2999,106 @@ error: return -1; }
+/** + * virDomainCoreDumpWithFormat: + * @domain: a domain object + * @to: path for the core file + * @dumpformat: format of domain memory's dump + * @flags: bitwise-OR of virDomainCoreDumpFlags + * + * 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 + * the remote host. Hypervisors may require the user to manually ensure + * proper permissions on the file named by @to. + * + * If @flags includes VIR_DUMP_MEMORY_ONLY and dumpformat is set, libvirt + * will ask qemu dump domain's memory in kdump-compressed format. + * + * 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. + * + * 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 +virDomainCoreDumpWithFormat(virDomainPtr domain, const char *to, unsigned int + dumpformat, unsigned int flags) +{ + 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) && + (dumpformat == VIR_DUMP_FORMAT_KDUMP_ZLIB || + dumpformat == VIR_DUMP_FORMAT_KDUMP_LZO || + dumpformat == VIR_DUMP_FORMAT_KDUMP_SNAPPY)) { + virReportInvalidArg(flags, "%s", + _("compression format is only work with memory-only")); + goto error; + }
This should be something checked on the QEMU driver since it is an implmentation restriction, not an API restriction. This should validate that dumpformat < VIR_DUMP_FORMAT_LAST though
+ + 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->domainCoreDumpWithFormat) { + 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->domainCoreDumpWithFormat(domain, absolute_to, + dumpformat, flags); + + VIR_FREE(absolute_to); + + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + +error: + virDispatchError(domain->conn); + return -1; +}
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..605b0d1 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, - const char *to, - unsigned int flags) +static int testDomainCoreDumpWithFormat(virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,12 @@ static int testDomainCoreDump(virDomainPtr domain, } }
+ if (dumpformat > VIR_DUMP_FORMAT_KDUMP_SNAPPY) { + virReportSystemError(errno, + _("invalid value of dumpformat: %d"), dumpformat); + goto cleanup; + }
This should be done in the libvirt.c entry point, comparing against VIR_DUMP_FORMAT_LAST 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 03/04/2014 07:41 PM, Daniel P. Berrange wrote:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..605b0d1 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, - const char *to, - unsigned int flags) +static int testDomainCoreDumpWithFormat(virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,12 @@ static int testDomainCoreDump(virDomainPtr domain, } }
+ if (dumpformat> VIR_DUMP_FORMAT_KDUMP_SNAPPY) { + virReportSystemError(errno, + _("invalid value of dumpformat: %d"), dumpformat); + goto cleanup; + }
This should be done in the libvirt.c entry point, comparing against VIR_DUMP_FORMAT_LAST
Is it OK, if I change the check to following one + /* dump the core of "domain" to file "to" */ + if (virDomainCoreDumpWithFormat(domain, to, dumpformat, flags) < 0) { + goto cleanup; + }
Regards, Daniel
-- Regards Qiao Nuohan
On Wed, Mar 05, 2014 at 12:00:59PM +0000, qiaonuohan@cn.fujitsu.com wrote:
On 03/04/2014 07:41 PM, Daniel P. Berrange wrote:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..605b0d1 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, - const char *to, - unsigned int flags) +static int testDomainCoreDumpWithFormat(virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,12 @@ static int testDomainCoreDump(virDomainPtr domain, } }
+ if (dumpformat> VIR_DUMP_FORMAT_KDUMP_SNAPPY) { + virReportSystemError(errno, + _("invalid value of dumpformat: %d"), dumpformat); + goto cleanup; + }
This should be done in the libvirt.c entry point, comparing against VIR_DUMP_FORMAT_LAST
Is it OK, if I change the check to following one
+ /* dump the core of "domain" to file "to" */ + if (virDomainCoreDumpWithFormat(domain, to, dumpformat, flags) < 0) { + goto cleanup; + }
Huh, I don't really see what you mean here. 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 03/05/2014 08:42 PM, Daniel P. Berrange wrote:
On Wed, Mar 05, 2014 at 12:00:59PM +0000, qiaonuohan@cn.fujitsu.com wrote:
On 03/04/2014 07:41 PM, Daniel P. Berrange wrote:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..605b0d1 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, - const char *to, - unsigned int flags) +static int testDomainCoreDumpWithFormat(virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,12 @@ static int testDomainCoreDump(virDomainPtr domain, } }
+ if (dumpformat> VIR_DUMP_FORMAT_KDUMP_SNAPPY) { + virReportSystemError(errno, + _("invalid value of dumpformat: %d"), dumpformat); + goto cleanup; + }
This should be done in the libvirt.c entry point, comparing against VIR_DUMP_FORMAT_LAST
Is it OK, if I change the check to following one
+ /* dump the core of "domain" to file "to" */ + if (virDomainCoreDumpWithFormat(domain, to, dumpformat, flags)< 0) { + goto cleanup; + }
Huh, I don't really see what you mean here.
parameter of testXXX should be used, or it won't go through make. My computer output the following message. I just want to check is it OK to use virDomainCoreDumpWithFormat here. <cut> ... CC qemu/libvirt_driver_qemu_impl_la-qemu_capabilities.lo CC qemu/libvirt_driver_qemu_impl_la-qemu_command.lo CC qemu/libvirt_driver_qemu_impl_la-qemu_domain.lo cc1: warnings being treated as errors test/test_driver.c: In function 'testDomainCoreDumpWithFormat': test/test_driver.c:2432: error: unused parameter 'dumpformat' [-Wunused-parameter] make[3]: *** [test/libvirt_driver_test_la-test_driver.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... make[3]: Leaving directory `/work/qemu/libvirt/src' make[2]: *** [all] Error 2 make[2]: Leaving directory `/work/qemu/libvirt/src' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/work/qemu/libvirt' make: *** [all] Error 2 <cut>
Regards, Daniel
-- Regards Qiao Nuohan
On Wed, Mar 05, 2014 at 12:48:00PM +0000, qiaonuohan@cn.fujitsu.com wrote:
On 03/05/2014 08:42 PM, Daniel P. Berrange wrote:
On Wed, Mar 05, 2014 at 12:00:59PM +0000, qiaonuohan@cn.fujitsu.com wrote:
On 03/04/2014 07:41 PM, Daniel P. Berrange wrote:
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index b724f82..605b0d1 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, - const char *to, - unsigned int flags) +static int testDomainCoreDumpWithFormat(virDomainPtr domain, + const char *to, + unsigned int dumpformat, + unsigned int flags) { testConnPtr privconn = domain->conn->privateData; int fd = -1; @@ -2479,6 +2480,12 @@ static int testDomainCoreDump(virDomainPtr domain, } }
+ if (dumpformat> VIR_DUMP_FORMAT_KDUMP_SNAPPY) { + virReportSystemError(errno, + _("invalid value of dumpformat: %d"), dumpformat); + goto cleanup; + }
This should be done in the libvirt.c entry point, comparing against VIR_DUMP_FORMAT_LAST
Is it OK, if I change the check to following one
+ /* dump the core of "domain" to file "to" */ + if (virDomainCoreDumpWithFormat(domain, to, dumpformat, flags)< 0) { + goto cleanup; + }
Huh, I don't really see what you mean here.
parameter of testXXX should be used, or it won't go through make. My computer output the following message. I just want to check is it OK to use virDomainCoreDumpWithFormat here.
<cut> ... CC qemu/libvirt_driver_qemu_impl_la-qemu_capabilities.lo CC qemu/libvirt_driver_qemu_impl_la-qemu_command.lo CC qemu/libvirt_driver_qemu_impl_la-qemu_domain.lo cc1: warnings being treated as errors test/test_driver.c: In function 'testDomainCoreDumpWithFormat': test/test_driver.c:2432: error: unused parameter 'dumpformat' [-Wunused-parameter]
Ok, so for the test driver you should do something like this if (dumpformat != VIR_DUMP_FORMAT_RAW) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, ....); return -1; } ie, only accept raw format The check for format value being out of range should be in libvirt.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 makes qemu driver supprot virDomainCoreDumpWithFormat API. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++--------- 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, 48 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9a865e..f373f7c 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 = NULL; /* Create an empty file with appropriate ownership. */ if (dump_flags & VIR_DUMP_BYPASS_CACHE) { @@ -3452,7 +3455,20 @@ doCoreDump(virQEMUDriverPtr driver, goto cleanup; if (dump_flags & VIR_DUMP_MEMORY_ONLY) { - ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP); + if (dumpformat == VIR_DUMP_FORMAT_RAW) + memory_dump_format = "elf"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_ZLIB) + memory_dump_format = "kdump-zlib"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_LZO) + memory_dump_format = "kdump-lzo"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_SNAPPY) + memory_dump_format = "kdump-snappy"; + else { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown dumpformat '%d'"), dumpformat); + } + ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, + memory_dump_format); } else { ret = qemuMigrationToFile(driver, vm, fd, 0, path, qemuCompressProgramName(compress), false, @@ -3515,8 +3531,9 @@ cleanup: return ret; } -static int qemuDomainCoreDump(virDomainPtr dom, +static int qemuDomainCoreDumpWithFormat(virDomainPtr dom, const char *path, + unsigned int dumpformat, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -3533,7 +3550,7 @@ static int qemuDomainCoreDump(virDomainPtr dom, if (!(vm = qemuDomObjFromDomain(dom))) return -1; - if (virDomainCoreDumpEnsureACL(dom->conn, vm->def) < 0) + if (virDomainCoreDumpWithFormatEnsureACL(dom->conn, vm->def) < 0) goto cleanup; if (qemuDomainObjBeginAsyncJob(driver, vm, @@ -3565,7 +3582,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 +3637,13 @@ cleanup: return ret; } +static int qemuDomainCoreDump(virDomainPtr dom, + const char *path, + unsigned int flags) +{ + return qemuDomainCoreDumpWithFormat(dom, path, VIR_DUMP_FORMAT_RAW, flags); +} + static char * qemuDomainScreenshot(virDomainPtr dom, virStreamPtr st, @@ -3742,7 +3767,8 @@ 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, + VIR_DUMP_FORMAT_RAW); if (ret < 0) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Dump failed")); @@ -3806,7 +3832,7 @@ doCoreDumpToAutoDumpPath(virQEMUDriverPtr driver, flags |= cfg->autoDumpBypassCache ? VIR_DUMP_BYPASS_CACHE: 0; ret = doCoreDump(driver, vm, dumpfile, - getCompressionType(driver), flags); + getCompressionType(driver), flags, VIR_DUMP_FORMAT_RAW); if (ret < 0) virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Dump failed")); @@ -16610,6 +16636,7 @@ static virDriver qemuDriver = { .domainSaveImageGetXMLDesc = qemuDomainSaveImageGetXMLDesc, /* 0.9.4 */ .domainSaveImageDefineXML = qemuDomainSaveImageDefineXML, /* 0.9.4 */ .domainCoreDump = qemuDomainCoreDump, /* 0.7.0 */ + .domainCoreDumpWithFormat = qemuDomainCoreDumpWithFormat, /* 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 Mon, Mar 03, 2014 at 10:27:25AM +0800, qiaonuohan wrote:
This patch makes qemu driver supprot virDomainCoreDumpWithFormat API. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++--------- 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, 48 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9a865e..f373f7c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c
@@ -3452,7 +3455,20 @@ doCoreDump(virQEMUDriverPtr driver, goto cleanup;
if (dump_flags & VIR_DUMP_MEMORY_ONLY) { - ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP); + if (dumpformat == VIR_DUMP_FORMAT_RAW) + memory_dump_format = "elf"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_ZLIB) + memory_dump_format = "kdump-zlib"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_LZO) + memory_dump_format = "kdump-lzo"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_SNAPPY) + memory_dump_format = "kdump-snappy"; + else { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown dumpformat '%d'"), dumpformat); + } + ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, + memory_dump_format); } else { ret = qemuMigrationToFile(driver, vm, fd, 0, path, qemuCompressProgramName(compress), false,
The else branch here should raise VIR_ERR_OPERATION_UNSUPPORTED if dumpformat != VIR_DUMP_FORMAT_RAW
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)
The '*' should associate with the variable name, not the data type.
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);
Same comment about '*' whitespace 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 03/04/2014 07:44 PM, Daniel P. Berrange wrote:
On Mon, Mar 03, 2014 at 10:27:25AM +0800, qiaonuohan wrote:
This patch makes qemu driver supprot virDomainCoreDumpWithFormat API. --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++++++++--------- 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, 48 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9a865e..f373f7c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3452,7 +3455,20 @@ doCoreDump(virQEMUDriverPtr driver, goto cleanup;
if (dump_flags& VIR_DUMP_MEMORY_ONLY) { - ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP); + if (dumpformat == VIR_DUMP_FORMAT_RAW) + memory_dump_format = "elf"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_ZLIB) + memory_dump_format = "kdump-zlib"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_LZO) + memory_dump_format = "kdump-lzo"; + else if (dumpformat == VIR_DUMP_FORMAT_KDUMP_SNAPPY) + memory_dump_format = "kdump-snappy"; + else { + virReportError(VIR_ERR_INVALID_ARG, + _("unknown dumpformat '%d'"), dumpformat); + } + ret = qemuDumpToFd(driver, vm, fd, QEMU_ASYNC_JOB_DUMP, + memory_dump_format); } else { ret = qemuMigrationToFile(driver, vm, fd, 0, path, qemuCompressProgramName(compress), false, The else branch here should raise VIR_ERR_OPERATION_UNSUPPORTED if dumpformat != VIR_DUMP_FORMAT_RAW
So here is the place I should do the "implmentation restriction"(mentioned in your last mail), then VIR_DUMP_FORMAT_RAW is the only choice when VIR_DUMP_MEMORY_ONLY is not specified. -- Regards Qiao Nuohan
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 virDomainCoreDumpWithFormat API. Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- tools/virsh-domain.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2e3f0ed..70613e5 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 = VIR_DUMP_FORMAT_RAW; sigemptyset(&sigmask); sigaddset(&sigmask, SIGINT); @@ -4524,7 +4535,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_DUMP_FORMAT_KDUMP_ZLIB; + else if (STREQ(compression_format, "lzo")) + memory_dump_format = VIR_DUMP_FORMAT_KDUMP_LZO; + else if (STREQ(compression_format, "snappy")) + memory_dump_format = VIR_DUMP_FORMAT_KDUMP_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_DUMP_FORMAT_KDUMP_ZLIB; + } + + if (virDomainCoreDumpWithFormat(dom, to, memory_dump_format, flags) < 0) { vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); goto out; } -- 1.8.5.3
On Mon, Mar 03, 2014 at 10:27:26AM +0800, qiaonuohan wrote:
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 virDomainCoreDumpWithFormat API.
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- tools/virsh-domain.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2e3f0ed..70613e5 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} };
I don't really see much point in having both args here - it is overly verbose IMHO. I suggest '--compress FORMAT' as a single arg 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 03/04/2014 07:45 PM, Daniel P. Berrange wrote:
On Mon, Mar 03, 2014 at 10:27:26AM +0800, qiaonuohan wrote:
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 virDomainCoreDumpWithFormat API.
Signed-off-by: Qiao Nuohan<qiaonuohan@cn.fujitsu.com> --- tools/virsh-domain.c | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 2e3f0ed..70613e5 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} };
I don't really see much point in having both args here - it is overly verbose IMHO.
I suggest '--compress FORMAT' as a single arg
zlib/lzo/snappy are the compression types that will be supported. Obviously, one of them, zlib, is used more *frequently*. Actually, I wanted to add '--compress [format]'(argument can be omitted) to make things simple. '--compress' alone is enough to specify zlib, and '--compress format' is available to specify other formats. But such optional argument is not supported in libvirt. So I use "--compress", instead of '--compression-format zlib', to shorten the amount of typing when using zlib. This is the use case, then what do you think.
Regards, Daniel
-- Regards Qiao Nuohan
participants (3)
-
Daniel P. Berrange -
qiaonuohan -
qiaonuohan＠cn.fujitsu.com