If a parameter definition is invalid we can include the name of the parameter for simpler debugging. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/vsh.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index e5ecc38f73..d1e795bbc1 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -307,8 +307,8 @@ vshCmddefCheckInternals(vshControl *ctl, case VSH_OT_STRING: case VSH_OT_BOOL: if (opt->flags & VSH_OFLAG_REQ) { - vshError(ctl, _("command '%s' misused VSH_OFLAG_REQ"), - cmd->name); + vshError(ctl, _("parameter '%s' of command '%s' misused VSH_OFLAG_REQ"), + opt->name, cmd->name); return -1; /* neither bool nor string options can be mandatory */ } break; @@ -319,8 +319,8 @@ vshCmddefCheckInternals(vshControl *ctl, char *p; if (opt->flags || !opt->help) { - vshError(ctl, _("command '%s' has incorrect alias option"), - cmd->name); + vshError(ctl, _("parameter '%s' of command '%s' has incorrect alias option"), + opt->name, cmd->name); return -1; /* alias options are tracked by the original name */ } if ((p = strchr(name, '='))) @@ -334,30 +334,30 @@ vshCmddefCheckInternals(vshControl *ctl, VIR_FREE(name); /* If alias comes with value, replacement must not be bool */ if (cmd->opts[j].type == VSH_OT_BOOL) { - vshError(ctl, _("command '%s' has mismatched alias type"), - cmd->name); + vshError(ctl, _("alias '%s' of command '%s' has mismatched alias type"), + opt->name, cmd->name); return -1; } } if (!cmd->opts[j].name) { - vshError(ctl, _("command '%s' has missing alias option"), - cmd->name); + vshError(ctl, _("alias '%s' of command '%s' has missing alias option"), + opt->name, cmd->name); return -1; /* alias option must map to a later option name */ } } break; case VSH_OT_ARGV: if (cmd->opts[i + 1].name) { - vshError(ctl, _("command '%s' does not list argv option last"), - cmd->name); + vshError(ctl, _("parameter '%s' of command '%s' must be listed last"), + opt->name, cmd->name); return -1; /* argv option must be listed last */ } break; case VSH_OT_DATA: if (!(opt->flags & VSH_OFLAG_REQ)) { - vshError(ctl, _("command '%s' has non-required VSH_OT_DATA"), - cmd->name); + vshError(ctl, _("parameter '%s' of command '%s' must use VSH_OFLAG_REQ flag"), + opt->name, cmd->name); return -1; /* OT_DATA should always be required. */ } break; -- 2.28.0

We no longer print help for every command to validate the args. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/vsh.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 28c7533a25..cf4ddc1c2c 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3324,9 +3324,6 @@ const vshCmdInfo info_selftest[] = { {.name = NULL} }; -/* Prints help for every command. - * That runs vshCmddefOptParse which validates - * the per-command options structure. */ bool cmdSelfTest(vshControl *ctl, const vshCmd *cmd G_GNUC_UNUSED) -- 2.28.0

The help formatter called vshCmddefOptParse just for validation purposes. Since vshCmddefOptParse no longer validates the command itself and we don't need the bitmaps returned by it we can drop the call entirely. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/vsh.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index f92759f219..2de940277e 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -606,20 +606,12 @@ vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp) } static bool -vshCmddefHelp(vshControl *ctl, const vshCmdDef *def) +vshCmddefHelp(const vshCmdDef *def) { const char *desc = NULL; char buf[256]; - uint64_t opts_need_arg; - uint64_t opts_required; bool shortopt = false; /* true if 'arg' works instead of '--opt arg' */ - if (vshCmddefOptParse(def, &opts_need_arg, &opts_required)) { - vshError(ctl, _("internal error: bad options in command: '%s'"), - def->name); - return false; - } - fputs(_(" NAME\n"), stdout); fprintf(stdout, " %s - %s\n", def->name, _(vshCmddefGetInfo(def, "help"))); @@ -3113,7 +3105,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) if ((def = vshCmddefSearch(name))) { if (def->flags & VSH_CMD_FLAG_ALIAS) def = vshCmddefSearch(def->alias); - return vshCmddefHelp(ctl, def); + return vshCmddefHelp(def); } else if ((grp = vshCmdGrpSearch(name))) { return vshCmdGrpHelp(ctl, grp); } else { -- 2.28.0

We try to enable keepalive oportunistically. If it's not supported by the connection driver and it was not explicitly requested we keep the error object set and can report it in some cases accidentally: --- stdout --- TEST: /home/pipo/libvirt/tests/virsh-self-test ! 1 FAILED --- stderr --- error: parameter 'target' of command 'attach-disk' must be listed before optional parameters error: this function is not supported by the connection driver: virConnectSetKeepAlive ------- Clear the stored libvirt error. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/virsh.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virsh.c b/tools/virsh.c index 954778b626..a6bfbbbb87 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -176,6 +176,7 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) } vshDebug(ctl, VSH_ERR_INFO, "%s", _("Failed to setup keepalive on connection\n")); + vshResetLibvirtError(); } cleanup: -- 2.28.0