From 31ef0836a73ed8583ff93613d2759e28965103ef Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Tue, 14 Apr 2015 16:15:06 -0600 Subject: [PATCH] virsh: fix regression in 'virsh event' by domain Commit a0670ae caused a regression in 'virsh event' and 'virsh qemu-monitor-event' - if a user tries to filter the command to a specific domain, an error message is printed: $ virsh event dom --loop error: internal error: virsh qemu-monitor-event: no domain VSH_OT_DATA option and then the command continues as though no domain had been supplied (giving events for ALL domains, instead of the requested one). This is because the code was incorrectly assuming that all "domain" options would be supplied via a mandatory VSH_OT_DATA, even though "domain" is optional for these two commands, so we had changed them to VSH_OT_STRING to quit failing for other reasons (ever since it was decided that VSH_OT_DATA and VSH_OT_STRING should no longer be synonyms). In looking at the situation, though, the code for looking up a domain was making a pointless check for whether the option exists prior to finding the option's string value, as vshCommandOptStringReq does just fine at reporting any errors when looking up a string whether or not the option was present. So this is a case of regression fixing by pure code deletion :) * tools/virsh-domain.c (vshCommandOptDomainBy): Drop useless filter. * tools/virsh-interface.c (vshCommandOptInterfaceBy): Likewise. * tools/virsh-network.c (vshCommandOptNetworkBy): Likewise. * tools/virsh-nwfilter.c (vshCommandOptNWFilterBy): Likewise. * tools/virsh-secret.c (vshCommandOptSecret): Likewise. * tools/virsh.h (vshCmdHasOption): Drop unused function. * tools/virsh.c (vshCmdHasOption): Likewise. Signed-off-by: Eric Blake --- tools/virsh-domain.c | 3 --- tools/virsh-interface.c | 2 -- tools/virsh-network.c | 3 --- tools/virsh-nwfilter.c | 5 +---- tools/virsh-secret.c | 5 +---- tools/virsh.c | 23 ----------------------- tools/virsh.h | 2 -- 7 files changed, 2 insertions(+), 41 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index bc6054ad3..2fe3924aa 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -119,9 +119,6 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd, const char *n = NULL; const char *optname = "domain"; - if (!vshCmdHasOption(ctl, cmd, optname)) - return NULL; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) return NULL; diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 3251e01c9..b9c60c580 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -53,8 +53,6 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, if (!optname) optname = "interface"; - if (!vshCmdHasOption(ctl, cmd, optname)) - return NULL; if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) return NULL; diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 09ee11af9..b859b49a7 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -42,9 +42,6 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, const char *optname = "network"; virCheckFlags(VSH_BYUUID | VSH_BYNAME, NULL); - if (!vshCmdHasOption(ctl, cmd, optname)) - return NULL; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) return NULL; diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index e6bef08ed..63c1c7eba 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -1,7 +1,7 @@ /* * virsh-nwfilter.c: Commands to manage network filters * - * Copyright (C) 2005, 2007-2014 Red Hat, Inc. + * Copyright (C) 2005, 2007-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -41,9 +41,6 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, const char *optname = "nwfilter"; virCheckFlags(VSH_BYUUID | VSH_BYNAME, NULL); - if (!vshCmdHasOption(ctl, cmd, optname)) - return NULL; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) return NULL; diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 5065c6ffa..c6ceabd86 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -1,7 +1,7 @@ /* * virsh-secret.c: Commands to manage secret * - * Copyright (C) 2005, 2007-2014 Red Hat, Inc. + * Copyright (C) 2005, 2007-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -41,9 +41,6 @@ vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, const char **name) const char *n = NULL; const char *optname = "secret"; - if (!vshCmdHasOption(ctl, cmd, optname)) - return NULL; - if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) return NULL; diff --git a/tools/virsh.c b/tools/virsh.c index 889a56128..4425774b3 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1860,29 +1860,6 @@ vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt) return NULL; } -/* Determine whether CMD->opts includes an option with name OPTNAME. - If not, give a diagnostic and return false. - If so, return true. */ -bool -vshCmdHasOption(vshControl *ctl, const vshCmd *cmd, const char *optname) -{ - /* Iterate through cmd->opts, to ensure that there is an entry - with name OPTNAME and type VSH_OT_DATA. */ - bool found = false; - const vshCmdOpt *opt; - for (opt = cmd->opts; opt; opt = opt->next) { - if (STREQ(opt->def->name, optname) && opt->def->type == VSH_OT_DATA) { - found = true; - break; - } - } - - if (!found) - vshError(ctl, _("internal error: virsh %s: no %s VSH_OT_DATA option"), - cmd->def->name, optname); - return found; -} - /* Parse an optional --timeout parameter in seconds, but store the * value of the timeout in milliseconds. Return -1 on error, 0 if * no timeout was requested, and 1 if timeout was set. */ diff --git a/tools/virsh.h b/tools/virsh.h index e89d31586..44a5cd85f 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -316,8 +316,6 @@ int vshCommandOptScaledInt(const vshCmd *cmd, const char *name, bool vshCommandOptBool(const vshCmd *cmd, const char *name); const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt); -bool vshCmdHasOption(vshControl *ctl, const vshCmd *cmd, const char *optname); - int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout); /* Filter flags for various vshCommandOpt*By() functions */ -- 2.39.5