Re: [libvirt] [PATCH v8 resend 4/4] allow "virsh dump --memory-only" specify dump format

On 03/22/2014 09:51 PM, Wen Congyang wrote: 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'). Grammar is off. HACKING says that if the 'else' side has {}, the 'if' side must as well. 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