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 <eblake(a)redhat.com&gt; --- 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 bc6054a..2fe3924 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 3251e01..b9c60c5 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 09ee11a..b859b49 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 e6bef08..63c1c7e 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 5065c6f..c6ceabd 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 889a561..4425774 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 e89d315..44a5cd8 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.1.0