On 03/22/2014 09:51 PM, Wen Congyang wrote:
From: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
This patch adds "[--format] <string>" to "virsh dump --memory-only", which is changed to use the new virDomainCoreDumpWithFormat API. And "--compress" is added as an alias for "--format kdump-zlib".
Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com> --- tools/virsh-domain.c | 47 ++++++++++++++++++++++++++++++++++++++++++++--- tools/virsh.pod | 6 ++++++ 2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 0664774..d897c72 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -4490,6 +4490,14 @@ static const vshCmdOptDef opts_dump[] = { .type = VSH_OT_BOOL, .help = N_("dump domain's memory only") }, + {.name = "compress", + .type = VSH_OT_ALIAS, + .help = "format=kdump-zlib", + },
Remember that the alias mechanism intentionally results in undocumented support. Observe: $ tools/virsh dump --help | grep compress $ tools/virsh dump --help | grep format dump [--live] [--crash] [--bypass-cache] [--reset] <domain> <file> [--verbose] [--memory-only] [<format>] [--format] <string> specify the format of memory-only dump $ To date, we have only used aliases to fix warts in old interfaces while avoiding back-compat breaks. But you are attempting to use the alias as a brand new interface. I'd much rather NOT push the alias. Undocumented interfaces are evil, and should only exist when back-compat is at stake. But here, we've never had --compress in the past, so back-compat is not an issue. If you absolutely must have --compress (and can't stand to type --format=kdump-zlib in your scripts), it would be better to do that as a followup patch with a lot more justification and while ensuring it is a documented interface. Or even better would be adding support for generic user aliases (where I could create an alias 'd' for 'dump --format=kdump-zlib', then use 'virsh d $dom').
- if (virDomainCoreDump(dom, to, flags) < 0) { - vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); - goto out; + if (vshCommandOptBool(cmd, "format")) { + if (!(flags & VIR_DUMP_MEMORY_ONLY)) { + vshError(ctl, "%s", _("--format only work with --memory-only")); + goto out;
Grammar is off.
+ + if (vshCommandOptString(cmd, "format", &format)) { + if (STREQ(format, "kdump-zlib")) + dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_ZLIB; + else if (STREQ(format, "kdump-lzo")) + dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_LZO; + else if (STREQ(format, "kdump-snappy")) + dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_SNAPPY; + else if (STREQ(format, "elf")) + dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_RAW; + else {
HACKING says that if the 'else' side has {}, the 'if' side must as well.
+ vshError(ctl, _("format '%s' is not supported, expecting " + "'kdump-zlib', 'kdump-lzo', 'kdump-snappy' " + "or 'elf'."), format);
We tend to avoid trailing . in messages I've pushed your series, with this squashed in: diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c index d897c72..f2856a5 100644 --- i/tools/virsh-domain.c +++ w/tools/virsh-domain.c @@ -4490,10 +4490,6 @@ static const vshCmdOptDef opts_dump[] = { .type = VSH_OT_BOOL, .help = N_("dump domain's memory only") }, - {.name = "compress", - .type = VSH_OT_ALIAS, - .help = "format=kdump-zlib", - }, {.name = "format", .type = VSH_OT_DATA, .help = N_("specify the format of memory-only dump") @@ -4540,23 +4536,23 @@ doDump(void *opaque) if (vshCommandOptBool(cmd, "format")) { if (!(flags & VIR_DUMP_MEMORY_ONLY)) { - vshError(ctl, "%s", _("--format only work with --memory-only")); + vshError(ctl, "%s", _("--format only works with --memory-only")); goto out; } if (vshCommandOptString(cmd, "format", &format)) { - if (STREQ(format, "kdump-zlib")) + if (STREQ(format, "kdump-zlib")) { dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_ZLIB; - else if (STREQ(format, "kdump-lzo")) + } else if (STREQ(format, "kdump-lzo")) { dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_LZO; - else if (STREQ(format, "kdump-snappy")) + } else if (STREQ(format, "kdump-snappy")) { dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_SNAPPY; - else if (STREQ(format, "elf")) + } else if (STREQ(format, "elf")) { dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_RAW; - else { + } else { vshError(ctl, _("format '%s' is not supported, expecting " "'kdump-zlib', 'kdump-lzo', 'kdump-snappy' " - "or 'elf'."), format); + "or 'elf'"), format); goto out; } } diff --git i/tools/virsh.pod w/tools/virsh.pod index 7e3ae5a..20352cb 100644 --- i/tools/virsh.pod +++ w/tools/virsh.pod @@ -1027,8 +1027,7 @@ useful if the domain uses host devices directly. I<--format> I<string> is used to specify the format of 'memory-only' dump, and I<string> can be one of them: elf, kdump-zlib(kdump-compressed format with zlib-compressed), kdump-lzo(kdump-compressed format with -lzo-compressed), kdump-snappy(kdump-compressed format with snappy-compressed) -I<--compress> is an alias for I<--format> I<kdump-zlib>. +lzo-compressed), kdump-snappy(kdump-compressed format with snappy-compressed). The progress may be monitored using B<domjobinfo> virsh command and canceled with B<domjobabort> command (sent by another virsh instance). Another option -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org