[libvirt] [PATCH] Make the options "--crash" and "--live" of "virsh dump" mutually exclusive
This patch makes the options "--crash" and "--live" of "virsh dump" mutually exclusive diff --git a/tools/virsh.c b/tools/virsh.c index 2e35021..ef77d7f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1869,14 +1869,19 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DUMP_LIVE; if (vshCommandOptBool (cmd, "crash")) flags |= VIR_DUMP_CRASH; - + if ((flags& VIR_DUMP_CRASH)&& (flags& VIR_DUMP_CRASH)) { + vshError(ctl, "%s", _("--crash and --live are mutually exclusive. " + "Please choose only one.")); + ret = FALSE; + goto cleanup; + } if (virDomainCoreDump(dom, to, flags) == 0) { vshPrint(ctl, _("Domain %s dumped to %s\n"), name, to); } else { vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); ret = FALSE; } - +cleanup: virDomainFree(dom); return ret; }
This patch makes the options "--crash" and "--live" of "virsh dump" mutually exclusive
diff --git a/tools/virsh.c b/tools/virsh.c index 2e35021..ef77d7f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1869,14 +1869,19 @@ cmdDump(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DUMP_LIVE; if (vshCommandOptBool (cmd, "crash")) flags |= VIR_DUMP_CRASH; - + if ((flags& VIR_DUMP_CRASH)&& (flags& VIR_DUMP_CRASH)) { + vshError(ctl, "%s", _("--crash and --live are mutually exclusive. " + "Please choose only one.")); + ret = FALSE; + goto cleanup; + } if (virDomainCoreDump(dom, to, flags) == 0) { vshPrint(ctl, _("Domain %s dumped to %s\n"), name, to); } else { vshError(ctl, _("Failed to core dump domain %s to %s"), name, to); ret = FALSE; } - +cleanup: virDomainFree(dom); return ret; }
Looks like thunderbird showed his best again and exchanged '&' and ' '. Also there is a typo in the condition (checking for CRASH twice) and make syntax-check forbids use of TABs for indentation. Moreover, this would only protect virsh. It's better to have this check in virDomainCoreDump() so that all users can get the error. The following patch should fix these issues. Jirka
From 42385de4db211a372c2db6d6a2b14cfcaa0cd511 Mon Sep 17 00:00:00 2001 Message-Id: <42385de4db211a372c2db6d6a2b14cfcaa0cd511.1303466112.git.jdenemar@redhat.com> From: Mark Wu <dwu@redhat.com> Date: Fri, 22 Apr 2011 11:45:33 +0200 Subject: [PATCH] Make crash and live flags mutually exclusive in virDomainCoreDump Mail-Followup-To: libvir-list@redhat.com
They don't make any sense when used together. --- src/libvirt.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index 10c3cdf..2d56b77 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2379,6 +2379,12 @@ virDomainCoreDump(virDomainPtr domain, const char *to, int flags) goto error; } + if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_LIVE)) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("crash and live flags are mutually exclusive")); + goto error; + } + if (conn->driver->domainCoreDump) { int ret; char *absolute_to; -- 1.7.5.rc1
On 04/22/2011 03:57 AM, Jiri Denemark wrote:
This patch makes the options "--crash" and "--live" of "virsh dump" mutually exclusive Moreover, this would only protect virsh. It's better to have this check in virDomainCoreDump() so that all users can get the error.
Agreed.
The following patch should fix these issues.
Subject: [PATCH] Make crash and live flags mutually exclusive in virDomainCoreDump Mail-Followup-To: libvir-list@redhat.com
They don't make any sense when used together. --- src/libvirt.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c index 10c3cdf..2d56b77 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -2379,6 +2379,12 @@ virDomainCoreDump(virDomainPtr domain, const char *to, int flags) goto error; }
+ if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_LIVE)) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("crash and live flags are mutually exclusive"));
ACK, but incomplete. A few lines earlier, the documentation states: * @flags: extra flags, currently unused * * This method will dump the core of a domain on a given file for analysis. It would be a wise time to update the docs at the same time :) -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Fri, Apr 22, 2011 at 09:57:46 -0600, Eric Blake wrote:
@@ -2379,6 +2379,12 @@ virDomainCoreDump(virDomainPtr domain, const char *to, int flags) goto error; }
+ if ((flags & VIR_DUMP_CRASH) && (flags & VIR_DUMP_LIVE)) { + virLibDomainError(VIR_ERR_INVALID_ARG, + _("crash and live flags are mutually exclusive"));
ACK, but incomplete. A few lines earlier, the documentation states:
* @flags: extra flags, currently unused * * This method will dump the core of a domain on a given file for analysis.
It would be a wise time to update the docs at the same time :)
Sure. I fixed that and pushed, thanks. Jirka
participants (3)
-
Eric Blake -
Jiri Denemark -
Mark Wu