virsh: fix regression in 'virsh event' by domain

https://bugzilla.redhat.com/show_bug.cgi?id=1212620

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@redhat.com>
(cherry picked from commit 31ef0836a7)
This commit is contained in:
Eric Blake 2015-04-14 16:15:06 -06:00
parent ecb9a5319a
commit 0f709dd420
7 changed files with 2 additions and 41 deletions

View File

@ -118,9 +118,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;

View File

@ -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;

View File

@ -47,9 +47,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;

View File

@ -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
@ -47,9 +47,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;

View File

@ -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
@ -47,9 +47,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;

View File

@ -1870,29 +1870,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. */

View File

@ -319,8 +319,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 */