[libvirt] [PATCH 7/8] Fix up inconsistent virsh option error reporting.

The virsh option error reporting was not being used consistently; some commands would spit out errors on missing required options while others would just silently fail. However, vshCommandOptString knows which ones are required and which ones aren't, so make it spit out an error where appropriate. The rest of the patch is just cleaning up the uses of vshCommandOptString to deal with the new error reporting. Signed-off-by: Chris Lalancette <clalance(a)redhat.com&gt; --- tools/virsh.c | 66 ++++++++++++++++++++------------------------------------ 1 files changed, 24 insertions(+), 42 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 02ab7d3..6a22071 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -571,6 +571,7 @@ static int cmdConnect(vshControl *ctl, const vshCmd *cmd) { int ro = vshCommandOptBool(cmd, "readonly"); + char *name; if (ctl->conn) { if (virConnectClose(ctl->conn) != 0) { @@ -581,7 +582,10 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) } VIR_FREE(ctl->name); - ctl->name = vshStrdup(ctl, vshCommandOptString(cmd, "name", NULL)); + name = vshCommandOptString(cmd, "name", NULL); + if (!name) + return FALSE; + ctl->name = vshStrdup(ctl, name); if (!ro) { ctl->readonly = 0; @@ -2359,7 +2363,6 @@ cmdVcpupin(vshControl *ctl, const vshCmd *cmd) } if (!(cpulist = vshCommandOptString(cmd, "cpulist", NULL))) { - vshError(ctl, "%s", _("vcpupin: Missing cpulist")); virDomainFree(dom); return FALSE; } @@ -2976,10 +2979,8 @@ cmdMigrate (vshControl *ctl, const vshCmd *cmd) return FALSE; desturi = vshCommandOptString (cmd, "desturi", &found); - if (!found) { - vshError(ctl, "%s", _("migrate: Missing desturi")); + if (!found) goto done; - } migrateuri = vshCommandOptString (cmd, "migrateuri", NULL); @@ -7542,7 +7543,6 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd) from = vshCommandOptString(cmd, "file", &found); if (!found) { - vshError(ctl, "%s", _("attach-device: Missing <file> option")); virDomainFree(dom); return FALSE; } @@ -7609,7 +7609,6 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd) from = vshCommandOptString(cmd, "file", &found); if (!found) { - vshError(ctl, "%s", _("detach-device: Missing <file> option")); virDomainFree(dom); return FALSE; } @@ -7676,7 +7675,6 @@ cmdUpdateDevice(vshControl *ctl, const vshCmd *cmd) from = vshCommandOptString(cmd, "file", &found); if (!found) { - vshError(ctl, "%s", _("update-device: Missing <file> option")); virDomainFree(dom); return FALSE; } @@ -9087,10 +9085,8 @@ cmdSnapshotDumpXML(vshControl *ctl, const vshCmd *cmd) goto cleanup; name = vshCommandOptString(cmd, "snapshotname", NULL); - if (name == NULL) { - vshError(ctl, "%s", _("missing snapshotname")); + if (name == NULL) goto cleanup; - } snapshot = virDomainSnapshotLookupByName(dom, name, 0); if (snapshot == NULL) { @@ -9149,10 +9145,8 @@ cmdDomainSnapshotRevert(vshControl *ctl, const vshCmd *cmd) goto cleanup; name = vshCommandOptString(cmd, "snapshotname", NULL); - if (name == NULL) { - vshError(ctl, "%s", _("missing snapshotname")); + if (name == NULL) goto cleanup; - } snapshot = virDomainSnapshotLookupByName(dom, name, 0); if (snapshot == NULL) { @@ -9209,10 +9203,8 @@ cmdSnapshotDelete(vshControl *ctl, const vshCmd *cmd) goto cleanup; name = vshCommandOptString(cmd, "snapshotname", NULL); - if (name == NULL) { - vshError(ctl, "%s", _("missing snapshotname")); + if (name == NULL) goto cleanup; - } if (vshCommandOptBool(cmd, "children")) flags |= VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN; @@ -9647,7 +9639,13 @@ vshCommandOptString(const vshCmd *cmd, const char *name, int *found) if (found) *found = arg ? TRUE : FALSE; - return arg && arg->data && *arg->data ? arg->data : NULL; + if (arg && arg->data && *arg->data) + return arg->data; + + if ((arg->def->flag) & VSH_OFLAG_REQ) + vshError(NULL, _("Missing required option '%s'"), name); + + return NULL; } /* @@ -9711,10 +9709,8 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd, if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) { - vshError(ctl, "%s", _("undefined domain name or id")); + if (!(n = vshCommandOptString(cmd, optname, NULL))) return NULL; - } vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); @@ -9759,10 +9755,8 @@ vshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) { - vshError(ctl, "%s", _("undefined network name")); + if (!(n = vshCommandOptString(cmd, optname, NULL))) return NULL; - } vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); @@ -9800,10 +9794,8 @@ vshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) { - vshError(ctl, "%s", _("undefined nwfilter name")); + if (!(n = vshCommandOptString(cmd, optname, NULL))) return NULL; - } vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); @@ -9840,10 +9832,8 @@ vshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, if (!cmd_has_option (ctl, cmd, optname)) return NULL; - if (!(n = vshCommandOptString(cmd, optname, NULL))) { - vshError(ctl, "%s", _("undefined interface identifier")); + if (!(n = vshCommandOptString(cmd, optname, NULL))) return NULL; - } vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); @@ -9877,10 +9867,8 @@ vshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, virStoragePoolPtr pool = NULL; char *n; - if (!(n = vshCommandOptString(cmd, optname, NULL))) { - vshError(ctl, "%s", _("undefined pool name")); + if (!(n = vshCommandOptString(cmd, optname, NULL))) return NULL; - } vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); @@ -9918,15 +9906,11 @@ vshCommandOptVolBy(vshControl *ctl, const vshCmd *cmd, char *n, *p; int found; - if (!(n = vshCommandOptString(cmd, optname, NULL))) { - vshError(ctl, "%s", _("undefined vol name")); + if (!(n = vshCommandOptString(cmd, optname, NULL))) return NULL; - } - if (!(p = vshCommandOptString(cmd, pooloptname, &found)) && found) { - vshError(ctl, "%s", _("undefined pool name")); + if (!(p = vshCommandOptString(cmd, pooloptname, &found)) && found) return NULL; - } if (p) pool = vshCommandOptPoolBy(ctl, cmd, pooloptname, name, flag); @@ -9976,10 +9960,8 @@ vshCommandOptSecret(vshControl *ctl, const vshCmd *cmd, char **name) return NULL; n = vshCommandOptString(cmd, optname, NULL); - if (n == NULL) { - vshError(ctl, "%s", _("undefined secret UUID")); + if (n == NULL) return NULL; - } vshDebug(ctl, 5, "%s: found option <%s>: %s\n", cmd->def->name, optname, n); -- 1.7.1.1