[PATCH 07/13] virsh: Introduce new 'VSH_OT_ARGV' accessors

In preparation for internal parser refactor introduce new accessors for the VSH_OT_ARGV type which will return a NULL-terminated string list or even a concatenated string for the given argument. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- tools/virsh-checkpoint.c | 10 +-- tools/virsh-domain-monitor.c | 8 +-- tools/virsh-domain.c | 103 +++++++++---------------------- tools/virsh-network.c | 9 +-- tools/virsh-snapshot.c | 9 +-- tools/vsh.c | 114 ++++++++++++++++++++++++----------- tools/vsh.h | 10 ++- 7 files changed, 133 insertions(+), 130 deletions(-) diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index fea6b4fb4b..972b2f979c 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -226,7 +226,7 @@ cmdCheckpointCreateAs(vshControl *ctl, const char *desc = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = 0; - const vshCmdOpt *opt = NULL; + const char **diskspec = NULL; if (vshCommandOptBool(cmd, "quiesce")) flags |= VIR_DOMAIN_CHECKPOINT_CREATE_QUIESCE; @@ -243,13 +243,15 @@ cmdCheckpointCreateAs(vshControl *ctl, virBufferEscapeString(&buf, "<name>%s</name>\n", name); virBufferEscapeString(&buf, "<description>%s</description>\n", desc); - if (vshCommandOptBool(cmd, "diskspec")) { + if ((diskspec = vshCommandOptArgv(cmd, "diskspec"))) { virBufferAddLit(&buf, "<disks>\n"); virBufferAdjustIndent(&buf, 2); - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (virshParseCheckpointDiskspec(ctl, &buf, opt->data) < 0) + + for (; *diskspec; diskspec++) { + if (virshParseCheckpointDiskspec(ctl, &buf, *diskspec) < 0) return false; } + virBufferAdjustIndent(&buf, -2); virBufferAddLit(&buf, "</disks>\n"); } diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 700f3ae094..ef07ace577 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2096,7 +2096,7 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) virDomainStatsRecordPtr *next; bool raw = vshCommandOptBool(cmd, "raw"); int flags = 0; - const vshCmdOpt *opt = NULL; + const char **doms; bool ret = false; virshControl *priv = ctl->privData; @@ -2166,12 +2166,12 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "nowait")) flags |= VIR_CONNECT_GET_ALL_DOMAINS_STATS_NOWAIT; - if (vshCommandOptBool(cmd, "domain")) { + if ((doms = vshCommandOptArgv(cmd, "domain"))) { domlist = g_new0(virDomainPtr, 1); ndoms = 1; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (!(dom = virshLookupDomainBy(ctl, opt->data, + for (; *doms; doms++) { + if (!(dom = virshLookupDomainBy(ctl, *doms, VIRSH_BYID | VIRSH_BYUUID | VIRSH_BYNAME))) goto cleanup; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 28d90377a1..50e80689a2 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -5065,15 +5065,15 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, { char *set_val = NULL; const char *val = NULL; - const vshCmdOpt *opt = NULL; + const char **opt; virTypedParameterPtr params = NULL; int nparams = 0; int maxparams = 0; int ret = -1; int rv; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - g_autofree char *set_field = g_strdup(opt->data); + for (opt = vshCommandOptArgv(cmd, "set"); opt && *opt; opt++) { + g_autofree char *set_field = g_strdup(*opt); if (!(set_val = strchr(set_field, '='))) { vshError(ctl, "%s", _("Invalid syntax for --set, expecting name=value")); @@ -8248,8 +8248,6 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) int state; int type; g_autofree char *descArg = NULL; - const vshCmdOpt *opt = NULL; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; unsigned int queryflags = 0; @@ -8274,12 +8272,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) else type = VIR_DOMAIN_METADATA_DESCRIPTION; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - descArg = virBufferContentAndReset(&buf); + descArg = g_strdup(vshCommandOptArgvString(cmd, "new-desc")); if (edit || descArg) { g_autofree char *descDom = NULL; @@ -8559,7 +8552,7 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) int codeset; unsigned int holdtime = 0; int count = 0; - const vshCmdOpt *opt = NULL; + const char **opt = NULL; int keycode; unsigned int keycodes[VIR_DOMAIN_SEND_KEY_MAX_KEYS]; @@ -8583,15 +8576,15 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) return false; } - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { + for (opt = vshCommandOptArgv(cmd, "keycode"); opt && *opt; opt++) { if (count == VIR_DOMAIN_SEND_KEY_MAX_KEYS) { vshError(ctl, _("too many keycodes")); return false; } - if ((keycode = virshKeyCodeGetInt(opt->data)) < 0) { - if ((keycode = virKeycodeValueFromString(codeset, opt->data)) < 0) { - vshError(ctl, _("invalid keycode: '%1$s'"), opt->data); + if ((keycode = virshKeyCodeGetInt(*opt)) < 0) { + if ((keycode = virKeycodeValueFromString(codeset, *opt)) < 0) { + vshError(ctl, _("invalid keycode: '%1$s'"), *opt); return false; } } @@ -9659,31 +9652,15 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = { }; -static char * -cmdQemuMonitorCommandConcatCmd(vshControl *ctl, - const vshCmd *cmd, - const vshCmdOpt *opt) -{ - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - return virBufferContentAndReset(&buf); -} - - static char * cmdQemuMonitorCommandQMPWrap(vshControl *ctl, const vshCmd *cmd) { - g_autofree char *fullcmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL); + const char *fullcmd = vshCommandOptArgvString(cmd, "cmd"); g_autoptr(virJSONValue) fullcmdjson = NULL; g_autofree char *fullargs = NULL; g_autoptr(virJSONValue) fullargsjson = NULL; - const vshCmdOpt *opt = NULL; + const char **opt = NULL; const char *commandname = NULL; g_autoptr(virJSONValue) command = NULL; g_autoptr(virJSONValue) arguments = NULL; @@ -9695,16 +9672,17 @@ cmdQemuMonitorCommandQMPWrap(vshControl *ctl, /* if we've got a JSON object, pass it through */ if (virJSONValueIsObject(fullcmdjson)) - return g_steal_pointer(&fullcmd); + return g_strdup(fullcmd); /* we try to wrap the command and possible arguments into a JSON object, if * we as fall back we pass through what we've got from the user */ - if ((opt = vshCommandOptArgv(ctl, cmd, opt))) - commandname = opt->data; + opt = vshCommandOptArgv(cmd, "cmd"); + commandname = *opt; + opt++; /* now we process arguments similarly to how we've dealt with the full command */ - if ((fullargs = cmdQemuMonitorCommandConcatCmd(ctl, cmd, opt)) && + if ((fullargs = g_strjoinv(" ", (GStrv) opt)) && !(fullargsjson = virJSONValueFromString(fullargs))) { /* Reset the error before adding wrapping. */ vshResetLibvirtError(); @@ -9721,8 +9699,8 @@ cmdQemuMonitorCommandQMPWrap(vshControl *ctl, virBufferAddLit(&buf, "{"); /* opt points to the _ARGV option bit containing the command so we'll * iterate through the arguments now */ - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s,", opt->data); + for (; *opt; opt++) + virBufferAsprintf(&buf, "%s,", *opt); virBufferTrim(&buf, ","); virBufferAddLit(&buf, "}"); @@ -9767,7 +9745,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "hmp")) { flags |= VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP; - monitor_cmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL); + monitor_cmd = g_strdup(vshCommandOptArgvString(cmd, "cmd")); } else { monitor_cmd = cmdQemuMonitorCommandQMPWrap(ctl, cmd); } @@ -10062,26 +10040,16 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; bool ret = false; - g_autofree char *guest_agent_cmd = NULL; char *result = NULL; int timeout = VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT; int judge = 0; unsigned int flags = 0; - const vshCmdOpt *opt = NULL; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; virJSONValue *pretty = NULL; dom = virshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - guest_agent_cmd = virBufferContentAndReset(&buf); - judge = vshCommandOptInt(ctl, cmd, "timeout", &timeout); if (judge < 0) goto cleanup; @@ -10106,7 +10074,7 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - result = virDomainQemuAgentCommand(dom, guest_agent_cmd, timeout, flags); + result = virDomainQemuAgentCommand(dom, vshCommandOptArgvString(cmd, "cmd"), timeout, flags); if (!result) goto cleanup; @@ -10158,9 +10126,7 @@ static bool cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; - const vshCmdOpt *opt = NULL; - g_autofree char **cmdargv = NULL; - size_t ncmdargv = 0; + const char **cmdargv = NULL; pid_t pid; int nfdlist; int *fdlist; @@ -10178,12 +10144,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "noseclabel")) setlabel = false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - VIR_EXPAND_N(cmdargv, ncmdargv, 1); - cmdargv[ncmdargv-1] = opt->data; - } - VIR_EXPAND_N(cmdargv, ncmdargv, 1); - cmdargv[ncmdargv - 1] = NULL; + cmdargv = vshCommandOptArgv(cmd, "cmd"); if ((nfdlist = virDomainLxcOpenNamespace(dom, &fdlist, 0)) < 0) return false; @@ -10233,7 +10194,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) _exit(EXIT_CANCELED); if (pid == 0) { - execv(cmdargv[0], cmdargv); + execv(cmdargv[0], (char **) cmdargv); _exit(errno == ENOENT ? EXIT_ENOENT : EXIT_CANNOT_INVOKE); } @@ -12872,18 +12833,15 @@ static bool cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; - const vshCmdOpt *opt = NULL; - g_autofree const char **mountpoints = NULL; + const char **mountpoints = vshCommandOptArgv(cmd, "mountpoint"); size_t nmountpoints = 0; int count = 0; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - VIR_EXPAND_N(mountpoints, nmountpoints, 1); - mountpoints[nmountpoints-1] = opt->data; - } + if (mountpoints) + nmountpoints = g_strv_length((GStrv) mountpoints); if ((count = virDomainFSFreeze(dom, mountpoints, nmountpoints, 0)) < 0) { vshError(ctl, _("Unable to freeze filesystems")); @@ -12913,18 +12871,15 @@ static bool cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) { g_autoptr(virshDomain) dom = NULL; - const vshCmdOpt *opt = NULL; - g_autofree const char **mountpoints = NULL; + const char **mountpoints = vshCommandOptArgv(cmd, "mountpoint"); size_t nmountpoints = 0; int count = 0; if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - VIR_EXPAND_N(mountpoints, nmountpoints, 1); - mountpoints[nmountpoints-1] = opt->data; - } + if (mountpoints) + nmountpoints = g_strv_length((GStrv) mountpoints); if ((count = virDomainFSThaw(dom, mountpoints, nmountpoints, 0)) < 0) { vshError(ctl, _("Unable to thaw filesystems")); diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 24049a66f3..6fcc7fd8ee 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -388,8 +388,6 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd) int type; g_autofree char *descArg = NULL; - const vshCmdOpt *opt = NULL; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = VIR_NETWORK_UPDATE_AFFECT_CURRENT; unsigned int queryflags = 0; @@ -411,12 +409,7 @@ cmdNetworkDesc(vshControl *ctl, const vshCmd *cmd) else type = VIR_NETWORK_METADATA_DESCRIPTION; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) - virBufferAsprintf(&buf, "%s ", opt->data); - - virBufferTrim(&buf, " "); - - descArg = virBufferContentAndReset(&buf); + descArg = g_strdup(vshCommandOptArgvString(cmd, "new-desc")); if (edit || descArg) { g_autofree char *descNet = NULL; diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 415a390786..8b6a950a01 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -379,7 +379,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) const char *memspec = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = 0; - const vshCmdOpt *opt = NULL; + const char **diskspec; if (vshCommandOptBool(cmd, "no-metadata")) flags |= VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA; @@ -416,11 +416,12 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (memspec && virshParseSnapshotMemspec(ctl, &buf, memspec) < 0) return false; - if (vshCommandOptBool(cmd, "diskspec")) { + if ((diskspec = vshCommandOptArgv(cmd, "diskspec"))) { virBufferAddLit(&buf, "<disks>\n"); virBufferAdjustIndent(&buf, 2); - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (virshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) + + for (; *diskspec; diskspec++) { + if (virshParseSnapshotDiskspec(ctl, &buf, *diskspec) < 0) return false; } virBufferAdjustIndent(&buf, -2); diff --git a/tools/vsh.c b/tools/vsh.c index 8af6aeaa48..e9ed7c0794 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -779,6 +779,9 @@ vshCommandOptFree(vshCmdOpt * arg) a = a->next; g_free(tmp->data); + /* 'argv' doesn't own the strings themselves */ + g_free(tmp->argv); + g_free(tmp->argvstr); g_free(tmp); } } @@ -1226,30 +1229,80 @@ vshCommandOptBool(const vshCmd *cmd, const char *name) return vshCommandOpt(cmd, name, &dummy, false) == 1; } + +static vshCmdOpt * +vshCommandOptArgvInternal(const vshCmd *cmd, + const char *name) +{ + vshCmdOpt *first = NULL; + vshCmdOpt *opt; + size_t nargv = 0; + size_t nargv_alloc = 0; + + for (opt = cmd->opts; opt; opt = opt->next) { + if (STRNEQ(opt->def->name, name)) + continue; + + if (!first) { + /* Return existing data if we'we already processed it */ + if (opt->argv) + return opt; + + first = opt; + } + + /* extra NULL terminator */ + VIR_RESIZE_N(first->argv, nargv_alloc, nargv, 2); + first->argv[nargv++] = opt->data; + } + + if (first) + first->argvstr = g_strjoinv(" ", (GStrv) first->argv); + + return first; +} + + /** * vshCommandOptArgv: - * @ctl virtshell control structure - * @cmd command reference - * @opt starting point for the search + * @cmd: command reference + * @name: name of argument * - * Returns the next argv argument after OPT (or the first one if OPT - * is NULL), or NULL if no more are present. + * Returns a NULL terminated list of strings of values passed as argument of + * ARGV argument named @name. The returned string list is owned by @cmd and + * caller must not free or modify it. + */ +const char ** +vshCommandOptArgv(const vshCmd *cmd, + const char *name) +{ + vshCmdOpt *opt = vshCommandOptArgvInternal(cmd, name); + + if (!opt) + return NULL; + + return opt->argv; +} + + +/** + * vshCommandOptArgvString: + * @cmd: command reference + * @name: name of argument * - * Requires that a VSH_OT_ARGV option be last in the - * list of supported options in CMD->def->opts. + * Returns a string containing all values passed as ARGV argument @name + * delimited/concatenated by adding spaces. */ -const vshCmdOpt * -vshCommandOptArgv(vshControl *ctl G_GNUC_UNUSED, const vshCmd *cmd, - const vshCmdOpt *opt) +const char * +vshCommandOptArgvString(const vshCmd *cmd, + const char *name) { - opt = opt ? opt->next : cmd->opts; + vshCmdOpt *opt = vshCommandOptArgvInternal(cmd, name); - while (opt) { - if (opt->def->type == VSH_OT_ARGV) - return opt; - opt = opt->next; - } - return NULL; + if (!opt) + return NULL; + + return opt->argvstr; } @@ -3275,9 +3328,9 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) bool err = vshCommandOptBool(cmd, "err"); bool split = vshCommandOptBool(cmd, "split"); const char *prefix; - const vshCmdOpt *opt = NULL; g_autofree char *arg = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; + const char **o; VSH_EXCLUSIVE_OPTIONS_VAR(shell, xml); VSH_EXCLUSIVE_OPTIONS_VAR(shell, split); @@ -3288,8 +3341,8 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) if (prefix) virBufferAsprintf(&buf, "%s ", prefix); - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - const char *curr = opt->data; + for (o = vshCommandOptArgv(cmd, "string"); o && *o; o++) { + const char *curr = *o; if (xml) { virBufferEscapeString(&buf, "%s", curr); @@ -3431,14 +3484,14 @@ bool cmdComplete(vshControl *ctl, const vshCmd *cmd) { const vshClientHooks *hooks = ctl->hooks; - const char *arg = ""; - const vshCmdOpt *opt = NULL; + const char *lastArg = NULL; + const char **args = NULL; g_auto(GStrv) matches = NULL; char **iter; - g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; - if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) - return false; + /* The completer needs also the last component */ + for (args = vshCommandOptArgv(cmd, "string"); args && *args; args++) + lastArg = *args; /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we * need to prevent auth hooks reading any input. Therefore, we @@ -3451,23 +3504,16 @@ cmdComplete(vshControl *ctl, const vshCmd *cmd) if (!(hooks && hooks->connHandler && hooks->connHandler(ctl))) return false; - while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { - if (virBufferUse(&buf) != 0) - virBufferAddChar(&buf, ' '); - virBufferAddStr(&buf, opt->data); - arg = opt->data; - } - vshReadlineInit(ctl); - if (!(rl_line_buffer = virBufferContentAndReset(&buf))) + if (!(rl_line_buffer = g_strdup(vshCommandOptArgvString(cmd, "string")))) rl_line_buffer = g_strdup(""); /* rl_point is current cursor position in rl_line_buffer. * In our case it's at the end of the whole line. */ rl_point = strlen(rl_line_buffer); - if (!(matches = vshReadlineCompletion(arg, 0, 0))) + if (!(matches = vshReadlineCompletion(lastArg, 0, 0))) return false; for (iter = matches; *iter; iter++) { diff --git a/tools/vsh.h b/tools/vsh.h index d2426d1e30..4576c286fe 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -147,6 +147,8 @@ struct _vshCmdOptDef { struct _vshCmdOpt { const vshCmdOptDef *def; /* non-NULL pointer to option definition */ char *data; /* allocated data, or NULL for bool option */ + const char **argv; /* for VSH_OT_ARGV, the list of options */ + char *argvstr; /* space-joined @argv */ bool completeThis; /* true if this is the option user's wishing to autocomplete */ vshCmdOpt *next; @@ -292,8 +294,12 @@ bool vshCommandRun(vshControl *ctl, const vshCmd *cmd); bool vshCommandStringParse(vshControl *ctl, char *cmdstr, vshCmd **partial, size_t point); -const vshCmdOpt *vshCommandOptArgv(vshControl *ctl, const vshCmd *cmd, - const vshCmdOpt *opt); +const char ** +vshCommandOptArgv(const vshCmd *cmd, + const char *name); +const char * +vshCommandOptArgvString(const vshCmd *cmd, + const char *name); bool vshCommandArgvParse(vshControl *ctl, int nargs, char **argv); int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout); -- 2.44.0