[libvirt] [PATCH 00/11] Make auto completion better
After initial RFC [1] I had some time to work on this and here is the result. What's implemented? =================== Auto completion for both interactive and non-interactive virsh/virt-admin. Known limitations ================= Currently, just options completers work. I mean, to bring up list of domains you have to: virsh # start --domain <TAB><TAB> Doing bare: virsh # start <TAB><TAB> brings up list of --options. I am working on this meanwhile. But one can argue that having to use --option is not that much of a problem since we have good completion now. The other known limitation - and actually room for others to join and help - is missing completers. I mean, the last 3 patches give an example how to do that. But that is still very few. We need more completers. How does this work? =================== The basic idea is simple, when defining opts for command two new struct members can be set: {.name = "mac", .type = VSH_OT_STRING, + .completer = virshDomainInterfaceCompleter, + .completer_flags = VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC, .help = N_("MAC address") }, @completer is the callback that is used when user wants to bring up list of possible strings. @completer_flags can then be used to refine return value of @completer. In this specific example, virshDomainInterfaceCompleter() brings up list of interfaces for given domain. It tries to return /interface/target/@dev but if not found (e.g. because domain is not running), /interface/mac/@address is returned otherwise. However, in one specific case we are interested only in MAC addresses (regardless of domain state) and thus VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC flag is specified which alters @completer behaviour. For non-interactive virsh/virt-admin there's tools/bash-completion/vsh script which does no more than calling: $1 complete $USER_INPUT (where $1 is name of the binary user intents to run). How to test this? ================= Interactive completion should work out of the box. Just make sure you're connected. Completers don't connect! You certainly don't want ssh's 'Password:' prompt show on <TAB><TAB>, do you? Non-interactive completers do connect, but in order to avoid any password prompts, /dev/stdin is closed before anything else is done. In order to test it, just: libvirt.git $ source tools/bash-completion/vsh Now, bash completion should work: libvirt.git $ ./tools/virsh -c qemu:///system start --domain <TAB><TAB> Notes ===== This is that part of vsh that nobody wants to touch, but with these patches in we have the framework to just write small functions (as can be seen in examples). User would benefit from this! As usual, you can find all the patches on my github [2]. Michal 1: https://www.redhat.com/archives/libvir-list/2017-October/msg01374.html 2: https://github.com/zippy2/libvirt/tree/bash_autocomplete Michal Privoznik (11): vshCommandStringParse: Allow retrieving partial result vshCommandOpt: Relax check for valid options vsh: Add @silent to vshConnectionHook vsh: Fix vshCompleter signature vsh: Call vshCmdOptDef.completer properly vshCompleter: Pass partially parsed command vsh: Introduce complete command tools: Provide bash autompletion file virsh: Introduce virshDomainNameCompleter virsh: Introduce virshDomainInterfaceCompleter virt-admin: Introduce vshAdmServerCompleter configure.ac | 3 + libvirt.spec.in | 2 + m4/virt-bash-completion.m4 | 74 ++++++++++++++++ tools/Makefile.am | 40 ++++++++- tools/bash-completion/vsh | 73 ++++++++++++++++ tools/virsh-completer.c | 150 +++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 41 +++++++++ tools/virsh-domain-monitor.c | 33 ++++---- tools/virsh-domain.c | 188 +++++++++++++++++++++-------------------- tools/virsh-snapshot.c | 24 +++--- tools/virsh.c | 72 +++++++++------- tools/virsh.h | 12 ++- tools/virt-admin-completer.c | 76 +++++++++++++++++ tools/virt-admin-completer.h | 33 ++++++++ tools/virt-admin.c | 62 ++++++++------ tools/vsh.c | 195 ++++++++++++++++++++++++++++++++----------- tools/vsh.h | 23 ++++- 17 files changed, 873 insertions(+), 228 deletions(-) create mode 100644 m4/virt-bash-completion.m4 create mode 100644 tools/bash-completion/vsh create mode 100644 tools/virsh-completer.c create mode 100644 tools/virsh-completer.h create mode 100644 tools/virt-admin-completer.c create mode 100644 tools/virt-admin-completer.h -- 2.13.6
In the future, this function is going to be called from vshReadlineParse() to provide parsed input for completer callbacks. The idea is to allow the callbacks to provide more specific data. For instance, for the following input: virsh # domifaddr --domain fedora --interface <TAB><TAB> the --interface completer callback is going to be called. Now, it is more user friendly if the completer offers only those interfaces found in 'fedora' domain. But in order to do that it needs to be able to retrieve partially parsed result. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 4 ++-- tools/virt-admin.c | 4 ++-- tools/vsh.c | 67 ++++++++++++++++++++++++++++++++++-------------------- tools/vsh.h | 2 +- 4 files changed, 47 insertions(+), 30 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d1789f03a..d0c135016 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -815,7 +815,7 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) ctl->imode = false; if (argc - optind == 1) { vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); - return vshCommandStringParse(ctl, argv[optind]); + return vshCommandStringParse(ctl, argv[optind], NULL); } else { return vshCommandArgvParse(ctl, argc - optind, argv + optind); } @@ -952,7 +952,7 @@ main(int argc, char **argv) #if WITH_READLINE add_history(ctl->cmdstr); #endif - if (vshCommandStringParse(ctl, ctl->cmdstr)) + if (vshCommandStringParse(ctl, ctl->cmdstr, NULL)) vshCommandRun(ctl, ctl->cmd); } VIR_FREE(ctl->cmdstr); diff --git a/tools/virt-admin.c b/tools/virt-admin.c index e529a2891..b8b33af19 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1335,7 +1335,7 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv) ctl->imode = false; if (argc - optind == 1) { vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]); - return vshCommandStringParse(ctl, argv[optind]); + return vshCommandStringParse(ctl, argv[optind], NULL); } else { return vshCommandArgvParse(ctl, argc - optind, argv + optind); } @@ -1555,7 +1555,7 @@ main(int argc, char **argv) #if WITH_READLINE add_history(ctl->cmdstr); #endif - if (vshCommandStringParse(ctl, ctl->cmdstr)) + if (vshCommandStringParse(ctl, ctl->cmdstr, NULL)) vshCommandRun(ctl, ctl->cmd); } VIR_FREE(ctl->cmdstr); diff --git a/tools/vsh.c b/tools/vsh.c index 10a65c39f..eca312b4b 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1387,26 +1387,27 @@ struct _vshCommandParser { }; static bool -vshCommandParse(vshControl *ctl, vshCommandParser *parser) +vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial) { char *tkdata = NULL; vshCmd *clast = NULL; vshCmdOpt *first = NULL; + const vshCmdDef *cmd = NULL; - if (ctl->cmd) { + if (!partial && ctl->cmd) { vshCommandFree(ctl->cmd); ctl->cmd = NULL; } while (1) { vshCmdOpt *last = NULL; - const vshCmdDef *cmd = NULL; vshCommandToken tk; bool data_only = false; uint64_t opts_need_arg = 0; uint64_t opts_required = 0; uint64_t opts_seen = 0; + cmd = NULL; first = NULL; while (1) { @@ -1425,7 +1426,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) if (cmd == NULL) { /* first token must be command name */ if (!(cmd = vshCmddefSearch(tkdata))) { - vshError(ctl, _("unknown command: '%s'"), tkdata); + if (!partial) + vshError(ctl, _("unknown command: '%s'"), tkdata); goto syntaxError; /* ... or ignore this command only? */ } @@ -1437,9 +1439,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) } if (vshCmddefOptParse(cmd, &opts_need_arg, &opts_required) < 0) { - vshError(ctl, - _("internal error: bad options in command: '%s'"), - tkdata); + if (!partial) + vshError(ctl, + _("internal error: bad options in command: '%s'"), + tkdata); goto syntaxError; } VIR_FREE(tkdata); @@ -1474,11 +1477,12 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) if (tk == VSH_TK_ERROR) goto syntaxError; if (tk != VSH_TK_ARG) { - vshError(ctl, - _("expected syntax: --%s <%s>"), - opt->name, - opt->type == - VSH_OT_INT ? _("number") : _("string")); + if (!partial) + vshError(ctl, + _("expected syntax: --%s <%s>"), + opt->name, + opt->type == + VSH_OT_INT ? _("number") : _("string")); goto syntaxError; } if (opt->type != VSH_OT_ARGV) @@ -1486,8 +1490,9 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) } else { tkdata = NULL; if (optstr) { - vshError(ctl, _("invalid '=' after option --%s"), - opt->name); + if (!partial) + vshError(ctl, _("invalid '=' after option --%s"), + opt->name); VIR_FREE(optstr); goto syntaxError; } @@ -1502,7 +1507,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, &opts_seen)) && STRNEQ(cmd->name, "help")) { - vshError(ctl, _("unexpected data '%s'"), tkdata); + if (!partial) + vshError(ctl, _("unexpected data '%s'"), tkdata); goto syntaxError; } } @@ -1558,12 +1564,15 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) c->def = cmd; c->next = NULL; - if (vshCommandCheckOpts(ctl, c, opts_required, opts_seen) < 0) { + if (!partial && + vshCommandCheckOpts(ctl, c, opts_required, opts_seen) < 0) { VIR_FREE(c); goto syntaxError; } - if (!ctl->cmd) + if (partial) + *partial = c; + else if (!ctl->cmd) ctl->cmd = c; if (clast) clast->next = c; @@ -1577,12 +1586,20 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) return true; syntaxError: - if (ctl->cmd) { - vshCommandFree(ctl->cmd); - ctl->cmd = NULL; + if (partial) { + if (!*partial) + *partial = vshMalloc(ctl, sizeof(vshCmd)); + + (*partial)->opts = first; + (*partial)->def = cmd; + } else { + if (ctl->cmd) { + vshCommandFree(ctl->cmd); + ctl->cmd = NULL; + } + if (first) + vshCommandOptFree(first); } - if (first) - vshCommandOptFree(first); VIR_FREE(tkdata); return false; } @@ -1617,7 +1634,7 @@ vshCommandArgvParse(vshControl *ctl, int nargs, char **argv) parser.arg_pos = argv; parser.arg_end = argv + nargs; parser.getNextArg = vshCommandArgvGetArg; - return vshCommandParse(ctl, &parser); + return vshCommandParse(ctl, &parser, NULL); } /* ---------------------- @@ -1689,7 +1706,7 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res, } bool -vshCommandStringParse(vshControl *ctl, char *cmdstr) +vshCommandStringParse(vshControl *ctl, char *cmdstr, vshCmd **partial) { vshCommandParser parser; @@ -1698,7 +1715,7 @@ vshCommandStringParse(vshControl *ctl, char *cmdstr) parser.pos = cmdstr; parser.getNextArg = vshCommandStringGetArg; - return vshCommandParse(ctl, &parser); + return vshCommandParse(ctl, &parser, partial); } /** diff --git a/tools/vsh.h b/tools/vsh.h index ab755bccf..8f7df9ff8 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -299,7 +299,7 @@ int vshBlockJobOptionBandwidth(vshControl *ctl, unsigned long *bandwidth); bool vshCommandOptBool(const vshCmd *cmd, const char *name); bool vshCommandRun(vshControl *ctl, const vshCmd *cmd); -bool vshCommandStringParse(vshControl *ctl, char *cmdstr); +bool vshCommandStringParse(vshControl *ctl, char *cmdstr, vshCmd **partial); const vshCmdOpt *vshCommandOptArgv(vshControl *ctl, const vshCmd *cmd, const vshCmdOpt *opt); -- 2.13.6
When trying to get an opt for command typed on the command line we first check if command has such option. Because if it doesn't it is a programming error. For instance: vshCommandOptBool(cmd, "config") called from say cmdStart() doesn't make sense since there's no --config for the start command. However, we will want to have generic completers which are going to check if various options are set. And so it can happen that we will check for non-existent option for given command. Therefore, we need to relax the check otherwise we will hit the assert() and don't get anywhere. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index eca312b4b..24ea45aa4 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -815,8 +815,7 @@ vshCommandFree(vshCmd *cmd) * Look up an option passed to CMD by NAME. Returns 1 with *OPT set * to the option if found, 0 with *OPT set to NULL if the name is * valid and the option is not required, -1 with *OPT set to NULL if - * the option is required but not present, and assert if NAME is not - * valid (which indicates a programming error). No error messages are + * the option is required but not present. No error messages are * issued if a value is returned. */ static int @@ -829,8 +828,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, /* See if option is valid and/or required. */ *opt = NULL; - while (valid) { - assert(valid->name); + while (valid && valid->name) { if (STREQ(name, valid->name)) break; valid++; -- 2.13.6
On Tue, Nov 07, 2017 at 01:22:50PM +0100, Michal Privoznik wrote:
When trying to get an opt for command typed on the command line we first check if command has such option. Because if it doesn't it is a programming error. For instance: vshCommandOptBool(cmd, "config") called from say cmdStart() doesn't make sense since there's no --config for the start command. However, we will want to have generic completers which are going to check if various options are set. And so it can happen that we will check for non-existent option for given command. Therefore, we need to relax the check otherwise we will hit the assert() and don't get anywhere.
I would prefer keeping the assert there since it's such an easy check for that kind of programming error. Is there no way to distinguish between calls from the completer? If not, then I would rename it to vshCommandOptInternal(), add a bool argument, make vshCommandOpt() call it with one value and the completer with another one (I don't care if there's yet another function for that or if completers use the Internal one).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c index eca312b4b..24ea45aa4 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -815,8 +815,7 @@ vshCommandFree(vshCmd *cmd) * Look up an option passed to CMD by NAME. Returns 1 with *OPT set * to the option if found, 0 with *OPT set to NULL if the name is * valid and the option is not required, -1 with *OPT set to NULL if - * the option is required but not present, and assert if NAME is not - * valid (which indicates a programming error). No error messages are + * the option is required but not present. No error messages are * issued if a value is returned. */ static int @@ -829,8 +828,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
/* See if option is valid and/or required. */ *opt = NULL; - while (valid) { - assert(valid->name); + while (valid && valid->name) { if (STREQ(name, valid->name)) break; valid++; -- 2.13.6
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 11/08/2017 10:54 AM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:50PM +0100, Michal Privoznik wrote:
When trying to get an opt for command typed on the command line we first check if command has such option. Because if it doesn't it is a programming error. For instance: vshCommandOptBool(cmd, "config") called from say cmdStart() doesn't make sense since there's no --config for the start command. However, we will want to have generic completers which are going to check if various options are set. And so it can happen that we will check for non-existent option for given command. Therefore, we need to relax the check otherwise we will hit the assert() and don't get anywhere.
I would prefer keeping the assert there since it's such an easy check for that kind of programming error. Is there no way to distinguish between calls from the completer? If not, then I would rename it to vshCommandOptInternal(), add a bool argument, make vshCommandOpt() call it with one value and the completer with another one (I don't care if there's yet another function for that or if completers use the Internal one).
So now that I'm trying to implement what you suggested I came to realize that it would be suboptimal. I mean, we have couple of functions for looking up arguments. For instance: vshCommandOptInt(), vshBlockJobOptionBandwidth(), virshCommandOptDomain() to name a few which eventually call vshCommandOpt(). Now, if I leave the assert() in and make it optional (say based on a boolean arg), I'd need to write alternative functions to all aforementioned so that they call vshCommandOpt() with the boolean arg set to false. For instance: virshCommandOptDomainUnsafe(), vshCommandOptIntUnsafe() or something. That looks like too much work for very little gain. Therefore I'd like to stick with my original proposal and just drop the assert. Michal
On Thu, Nov 09, 2017 at 04:57:54PM +0100, Michal Privoznik wrote:
On 11/08/2017 10:54 AM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:50PM +0100, Michal Privoznik wrote:
When trying to get an opt for command typed on the command line we first check if command has such option. Because if it doesn't it is a programming error. For instance: vshCommandOptBool(cmd, "config") called from say cmdStart() doesn't make sense since there's no --config for the start command. However, we will want to have generic completers which are going to check if various options are set. And so it can happen that we will check for non-existent option for given command. Therefore, we need to relax the check otherwise we will hit the assert() and don't get anywhere.
I would prefer keeping the assert there since it's such an easy check for that kind of programming error. Is there no way to distinguish between calls from the completer? If not, then I would rename it to vshCommandOptInternal(), add a bool argument, make vshCommandOpt() call it with one value and the completer with another one (I don't care if there's yet another function for that or if completers use the Internal one).
So now that I'm trying to implement what you suggested I came to realize that it would be suboptimal. I mean, we have couple of functions for looking up arguments. For instance: vshCommandOptInt(), vshBlockJobOptionBandwidth(), virshCommandOptDomain() to name a few which eventually call vshCommandOpt(). Now, if I leave the assert() in and make it optional (say based on a boolean arg), I'd need to write alternative functions to all aforementioned so that they call vshCommandOpt() with the boolean arg set to false. For instance: virshCommandOptDomainUnsafe(), vshCommandOptIntUnsafe() or something. That looks like too much work for very little gain. Therefore I'd like to stick with my original proposal and just drop the assert.
How about setting a boolean in @ctl instead? If you don't like that either, then keep what you have, I don't care that much about it.
On 11/09/2017 05:22 PM, Martin Kletzander wrote:
On Thu, Nov 09, 2017 at 04:57:54PM +0100, Michal Privoznik wrote:
On 11/08/2017 10:54 AM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:50PM +0100, Michal Privoznik wrote:
When trying to get an opt for command typed on the command line we first check if command has such option. Because if it doesn't it is a programming error. For instance: vshCommandOptBool(cmd, "config") called from say cmdStart() doesn't make sense since there's no --config for the start command. However, we will want to have generic completers which are going to check if various options are set. And so it can happen that we will check for non-existent option for given command. Therefore, we need to relax the check otherwise we will hit the assert() and don't get anywhere.
I would prefer keeping the assert there since it's such an easy check for that kind of programming error. Is there no way to distinguish between calls from the completer? If not, then I would rename it to vshCommandOptInternal(), add a bool argument, make vshCommandOpt() call it with one value and the completer with another one (I don't care if there's yet another function for that or if completers use the Internal one).
So now that I'm trying to implement what you suggested I came to realize that it would be suboptimal. I mean, we have couple of functions for looking up arguments. For instance: vshCommandOptInt(), vshBlockJobOptionBandwidth(), virshCommandOptDomain() to name a few which eventually call vshCommandOpt(). Now, if I leave the assert() in and make it optional (say based on a boolean arg), I'd need to write alternative functions to all aforementioned so that they call vshCommandOpt() with the boolean arg set to false. For instance: virshCommandOptDomainUnsafe(), vshCommandOptIntUnsafe() or something. That looks like too much work for very little gain. Therefore I'd like to stick with my original proposal and just drop the assert.
How about setting a boolean in @ctl instead? If you don't like that either, then keep what you have, I don't care that much about it.
I'm afraid that wouldn't fly either. I mean, not every *Opt*() takes ctl. For instance here's a snippet from virshDomainInterfaceCompleter() (patch 10/11): char ** virshDomainInterfaceCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags) { ... if (vshCommandOptBool(cmd, "config")) domainXMLFlags = VIR_DOMAIN_XML_INACTIVE; if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0) goto error; ... } While virshDomainGetXML() takes ctl, vshCommandOptBool() does not. But what we can do is to have a boolean in vshCmd struct (which represents *parsed* command), and depending on that boolean assert() would be called or not. And since the struct is public, we don't need any special functions for it. All that'd be needed is (in the cmdComplete - patch 06/11): if (!completed_list && opt && opt->completer) { vshCmd *partial = NULL; vshCommandStringParse(autoCompleteOpaque, rl_line_buffer, &partial); + partial->doNotCallAssert = true; completed_list = opt->completer(autoCompleteOpaque, partial, opt->completer_flags); vshCommandFree(partial); Okay, let me implement this idea and see how it looks. Michal
In near future we will want to not report error when connecting fails. In order to achieve that we have to pass a boolean that suppresses error messages. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 2 +- tools/virsh.c | 67 ++++++++++++++++++++++++++++++++-------------------- tools/virsh.h | 5 +++- tools/virt-admin.c | 49 +++++++++++++++++++++----------------- tools/vsh.c | 2 +- tools/vsh.h | 3 ++- 6 files changed, 76 insertions(+), 52 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 42d552637..cf612f73e 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -10931,7 +10931,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "desturi", &desturi) < 0) goto cleanup; - dconn = virshConnect(ctl, desturi, false); + dconn = virshConnect(ctl, desturi, false, false); if (!dconn) goto cleanup; diff --git a/tools/virsh.c b/tools/virsh.c index d0c135016..7d6dc2620 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -137,7 +137,7 @@ virshCatchDisconnect(virConnectPtr conn, /* Main Function which should be used for connecting. * This function properly handles keepalive settings. */ virConnectPtr -virshConnect(vshControl *ctl, const char *uri, bool readonly) +virshConnect(vshControl *ctl, const char *uri, bool readonly, bool silent) { virConnectPtr c = NULL; int interval = 5; /* Default */ @@ -191,15 +191,19 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) if (interval > 0 && virConnectSetKeepAlive(c, interval, count) != 0) { if (keepalive_forced) { - vshError(ctl, "%s", - _("Cannot setup keepalive on connection " - "as requested, disconnecting")); + if (!silent) { + vshError(ctl, "%s", + _("Cannot setup keepalive on connection " + "as requested, disconnecting")); + } virConnectClose(c); c = NULL; goto cleanup; } - vshDebug(ctl, VSH_ERR_INFO, "%s", - _("Failed to setup keepalive on connection\n")); + if (!silent) { + vshDebug(ctl, VSH_ERR_INFO, "%s", + _("Failed to setup keepalive on connection\n")); + } } cleanup: @@ -214,7 +218,11 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) * */ static int -virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force) +virshReconnect(vshControl *ctl, + const char *name, + bool readonly, + bool force, + bool silent) { bool connected = false; virshControlPtr priv = ctl->privData; @@ -232,20 +240,25 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force) virConnectUnregisterCloseCallback(priv->conn, virshCatchDisconnect); ret = virConnectClose(priv->conn); - if (ret < 0) - vshError(ctl, "%s", _("Failed to disconnect from the hypervisor")); - else if (ret > 0) - vshError(ctl, "%s", _("One or more references were leaked after " - "disconnect from the hypervisor")); + if (!silent) { + if (ret < 0) + vshError(ctl, "%s", _("Failed to disconnect from the hypervisor")); + else if (ret > 0) + vshError(ctl, "%s", _("One or more references were leaked after " + "disconnect from the hypervisor")); + } } - priv->conn = virshConnect(ctl, name ? name : ctl->connname, readonly); + priv->conn = virshConnect(ctl, name ? name : ctl->connname, + readonly, silent); if (!priv->conn) { - if (disconnected) - vshError(ctl, "%s", _("Failed to reconnect to the hypervisor")); - else - vshError(ctl, "%s", _("failed to connect to the hypervisor")); + if (!silent) { + if (disconnected) + vshError(ctl, "%s", _("Failed to reconnect to the hypervisor")); + else + vshError(ctl, "%s", _("Failed to connect to the hypervisor")); + } return -1; } else { if (name) { @@ -255,8 +268,9 @@ virshReconnect(vshControl *ctl, const char *name, bool readonly, bool force) } if (virConnectRegisterCloseCallback(priv->conn, virshCatchDisconnect, ctl, NULL) < 0) - vshError(ctl, "%s", _("Unable to register disconnect callback")); - if (connected && !force) + if (!silent) + vshError(ctl, "%s", _("Unable to register disconnect callback")); + if (connected && !force && !silent) vshError(ctl, "%s", _("Reconnected to the hypervisor")); } disconnected = 0; @@ -304,7 +318,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0) return false; - if (virshReconnect(ctl, name, ro, true) < 0) + if (virshReconnect(ctl, name, ro, true, false) < 0) return false; return true; @@ -316,11 +330,12 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) */ static bool -virshConnectionUsability(vshControl *ctl, virConnectPtr conn) +virshConnectionUsability(vshControl *ctl, virConnectPtr conn, bool silent) { if (!conn || virConnectIsAlive(conn) == 0) { - vshError(ctl, "%s", _("no valid connection")); + if (!silent) + vshError(ctl, "%s", _("no valid connection")); return false; } @@ -333,15 +348,15 @@ virshConnectionUsability(vshControl *ctl, virConnectPtr conn) } static void * -virshConnectionHandler(vshControl *ctl) +virshConnectionHandler(vshControl *ctl, bool silent) { virshControlPtr priv = ctl->privData; if ((!priv->conn || disconnected) && - virshReconnect(ctl, NULL, false, false) < 0) + virshReconnect(ctl, NULL, false, false, silent) < 0) return NULL; - if (virshConnectionUsability(ctl, priv->conn)) + if (virshConnectionUsability(ctl, priv->conn, silent)) return priv->conn; return NULL; @@ -391,7 +406,7 @@ virshInit(vshControl *ctl) * non-default connection, or might be 'help' which needs no * connection). */ - if (virshReconnect(ctl, NULL, false, false) < 0) { + if (virshReconnect(ctl, NULL, false, false, false) < 0) { vshReportError(ctl); return false; } diff --git a/tools/virsh.h b/tools/virsh.h index b353b645a..e03b597ef 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -144,6 +144,9 @@ typedef enum { VIRSH_BYMAC = (1 << 4), } virshLookupByFlags; -virConnectPtr virshConnect(vshControl *ctl, const char *uri, bool readonly); +virConnectPtr virshConnect(vshControl *ctl, + const char *uri, + bool readonly, + bool silent); #endif /* VIRSH_H */ diff --git a/tools/virt-admin.c b/tools/virt-admin.c index b8b33af19..5d7ef7988 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -154,24 +154,26 @@ vshAdmCatchDisconnect(virAdmConnectPtr conn ATTRIBUTE_UNUSED, } static int -vshAdmConnect(vshControl *ctl, unsigned int flags) +vshAdmConnect(vshControl *ctl, unsigned int flags, bool silent) { vshAdmControlPtr priv = ctl->privData; priv->conn = virAdmConnectOpen(ctl->connname, flags); if (!priv->conn) { - if (priv->wantReconnect) - vshError(ctl, "%s", _("Failed to reconnect to the admin server")); - else - vshError(ctl, "%s", _("Failed to connect to the admin server")); + if (!silent) { + if (priv->wantReconnect) + vshError(ctl, "%s", _("Failed to reconnect to the admin server")); + else + vshError(ctl, "%s", _("Failed to connect to the admin server")); + } return -1; } else { if (virAdmConnectRegisterCloseCallback(priv->conn, vshAdmCatchDisconnect, - NULL, NULL) < 0) + NULL, NULL) < 0 && !silent) vshError(ctl, "%s", _("Unable to register disconnect callback")); - if (priv->wantReconnect) + if (priv->wantReconnect && !silent) vshPrint(ctl, "%s\n", _("Reconnected to the admin server")); } @@ -179,7 +181,7 @@ vshAdmConnect(vshControl *ctl, unsigned int flags) } static int -vshAdmDisconnect(vshControl *ctl) +vshAdmDisconnect(vshControl *ctl, bool silent) { int ret = 0; vshAdmControlPtr priv = ctl->privData; @@ -189,11 +191,13 @@ vshAdmDisconnect(vshControl *ctl) virAdmConnectUnregisterCloseCallback(priv->conn, vshAdmCatchDisconnect); ret = virAdmConnectClose(priv->conn); - if (ret < 0) - vshError(ctl, "%s", _("Failed to disconnect from the admin server")); - else if (ret > 0) - vshError(ctl, "%s", _("One or more references were leaked after " - "disconnect from the hypervisor")); + if (!silent) { + if (ret < 0) + vshError(ctl, "%s", _("Failed to disconnect from the admin server")); + else if (ret > 0) + vshError(ctl, "%s", _("One or more references were leaked after " + "disconnect from the hypervisor")); + } priv->conn = NULL; return ret; } @@ -205,14 +209,14 @@ vshAdmDisconnect(vshControl *ctl) * */ static void -vshAdmReconnect(vshControl *ctl) +vshAdmReconnect(vshControl *ctl, bool silent) { vshAdmControlPtr priv = ctl->privData; if (priv->conn) priv->wantReconnect = true; - vshAdmDisconnect(ctl); - vshAdmConnect(ctl, 0); + vshAdmDisconnect(ctl, silent); + vshAdmConnect(ctl, 0, silent); priv->wantReconnect = false; } @@ -350,7 +354,7 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) ctl->connname = vshStrdup(ctl, name); } - vshAdmReconnect(ctl); + vshAdmReconnect(ctl, false); if (!connected && priv->conn) vshPrint(ctl, "%s\n", _("Connected to the admin server")); @@ -1080,15 +1084,16 @@ cmdDaemonLogOutputs(vshControl *ctl, const vshCmd *cmd) } static void * -vshAdmConnectionHandler(vshControl *ctl) +vshAdmConnectionHandler(vshControl *ctl, bool silent) { vshAdmControlPtr priv = ctl->privData; if (!virAdmConnectIsAlive(priv->conn)) - vshAdmReconnect(ctl); + vshAdmReconnect(ctl, silent); if (!virAdmConnectIsAlive(priv->conn)) { - vshError(ctl, "%s", _("no valid connection")); + if (!silent) + vshError(ctl, "%s", _("no valid connection")); return NULL; } @@ -1122,7 +1127,7 @@ vshAdmInit(vshControl *ctl) ctl->eventLoopStarted = true; if (ctl->connname) { - vshAdmReconnect(ctl); + vshAdmReconnect(ctl, false); /* Connecting to a named connection must succeed, but we delay * connecting to the default connection until we need it * (since the first command might be 'connect' which allows a @@ -1156,7 +1161,7 @@ vshAdmDeinit(vshControl *ctl) VIR_FREE(ctl->connname); if (priv->conn) - vshAdmDisconnect(ctl); + vshAdmDisconnect(ctl, false); virResetLastError(); diff --git a/tools/vsh.c b/tools/vsh.c index 24ea45aa4..83c96e1a8 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -1321,7 +1321,7 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) GETTIMEOFDAY(&before); if ((cmd->def->flags & VSH_CMD_FLAG_NOCONNECT) || - (hooks && hooks->connHandler && hooks->connHandler(ctl))) { + (hooks && hooks->connHandler && hooks->connHandler(ctl, false))) { ret = cmd->def->handler(ctl, cmd); } else { /* connection is not usable, return error */ diff --git a/tools/vsh.h b/tools/vsh.h index 8f7df9ff8..c411c2ca4 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -232,7 +232,8 @@ struct _vshControl { }; typedef void * -(*vshConnectionHook)(vshControl *ctl); +(*vshConnectionHook)(vshControl *ctl, + bool silent); struct _vshClientHooks { vshConnectionHook connHandler; -- 2.13.6
On Tue, Nov 07, 2017 at 01:22:51PM +0100, Michal Privoznik wrote:
In near future we will want to not report error when connecting fails. In order to achieve that we have to pass a boolean that suppresses error messages.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 2 +- tools/virsh.c | 67 ++++++++++++++++++++++++++++++++-------------------- tools/virsh.h | 5 +++- tools/virt-admin.c | 49 +++++++++++++++++++++----------------- tools/vsh.c | 2 +- tools/vsh.h | 3 ++- 6 files changed, 76 insertions(+), 52 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index d0c135016..7d6dc2620 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -191,15 +191,19 @@ virshConnect(vshControl *ctl, const char *uri, bool readonly) if (interval > 0 && virConnectSetKeepAlive(c, interval, count) != 0) { if (keepalive_forced) { - vshError(ctl, "%s", - _("Cannot setup keepalive on connection " - "as requested, disconnecting")); + if (!silent) { + vshError(ctl, "%s", + _("Cannot setup keepalive on connection " + "as requested, disconnecting")); + } virConnectClose(c); c = NULL; goto cleanup; } - vshDebug(ctl, VSH_ERR_INFO, "%s", - _("Failed to setup keepalive on connection\n")); + if (!silent) { + vshDebug(ctl, VSH_ERR_INFO, "%s", + _("Failed to setup keepalive on connection\n")); + }
I guess debug messages probably need to be filtered too. Actually allocation errors should be covered as well. And you missed some. It's fine as it is, since no auto-completion is perfect, I use lot of them extensively and I must say I don't care if it outputs something when some error happens. The nice thing about auto-completion is that if it does not work at all or works differently (outputs something it "shouldn't") nothing happens. There is no data loss, broken migrations or whatever. Anyway looking through all the things that you are modifying here and to those that could still be modified, I think this could be approached a bit better. As I said it's kind of fine if we keep it like this for now, but it could be even better. Consider this: - we do not modify what messages are reported - we leave the function that manages log output to decide - when the completer is called we set it up so that there is no log output Guess what, we have first two things in place and for the third one you can just call vshCloseLogFile(ctl) and you're golden ;) If you want to make it even better, you can already do that before you get to any completer. For example if you specify '-q' multiple times on the command line it could switch to super quiet mode, e.g.: diff --git i/tools/virsh.c w/tools/virsh.c index f830331f6bbe..6b7eafeba0be 100644 --- i/tools/virsh.c +++ w/tools/virsh.c @@ -782,6 +782,8 @@ virshParseArgv(vshControl *ctl, int argc, char **argv) vshOpenLogFile(ctl); break; case 'q': + if (ctl->quiet) + vshCloseLogFile(ctl); ctl->quiet = true; break; case 't': -- And just call virsh from the bash completion with '-qq' And the same for virt-admin of course. And maybe document it :)
The first argument passed to this function is vshControl *. There's no need to use void pointer. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 2 +- tools/vsh.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 83c96e1a8..cbab6f7d0 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -66,7 +66,7 @@ #ifdef WITH_READLINE /* For autocompletion */ -void *autoCompleteOpaque; +vshControl *autoCompleteOpaque; #endif /* NOTE: It would be much nicer to have these two as part of vshControl diff --git a/tools/vsh.h b/tools/vsh.h index c411c2ca4..1ebd5c11a 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -123,7 +123,8 @@ typedef struct _vshCmdOpt vshCmdOpt; typedef struct _vshCmdOptDef vshCmdOptDef; typedef struct _vshControl vshControl; -typedef char **(*vshCompleter)(void *opaque, unsigned int flags); +typedef char **(*vshCompleter)(vshControl *ctl, + unsigned int flags); /* * vshCmdInfo -- name/value pair for information about command -- 2.13.6
The idea is that .completer for vshCmdOptDef would be called if the last token on the input is a cmd opt. For instance: virsh # start --domain<TAB><TAB> However, with current code that's not happening. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index cbab6f7d0..dd2f06ada 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2687,7 +2687,7 @@ vshReadlineParse(const char *text, int state) uint64_t opts_seen; size_t opt_index; static bool cmd_exists, opts_filled, opt_exists; - static bool non_bool_opt_exists, data_complete; + static bool non_bool_opt_exists, complete_data, complete_opts; if (!state) { parser.pos = rl_line_buffer; @@ -2734,7 +2734,7 @@ vshReadlineParse(const char *text, int state) cmd_exists = false; opts_filled = false; non_bool_opt_exists = false; - data_complete = false; + complete_data = false; const_opts_need_arg = 0; const_opts_required = 0; @@ -2800,7 +2800,7 @@ vshReadlineParse(const char *text, int state) } if (STREQ(tkdata, sanitized_text)) { /* auto-complete non-bool option arg */ - data_complete = true; + complete_data = true; break; } non_bool_opt_exists = false; @@ -2847,27 +2847,34 @@ vshReadlineParse(const char *text, int state) virSkipSpaces((const char**)&tkdata); } VIR_FREE(const_tkdata); + complete_opts = opts_filled && !non_bool_opt_exists; } if (!cmd_exists) { res = vshReadlineCommandGenerator(sanitized_text, state); - } else if (opts_filled && !non_bool_opt_exists) { - res = vshReadlineOptionsGenerator(sanitized_text, state, cmd); - } else if (non_bool_opt_exists && data_complete && opt && opt->completer) { - if (!completed_list) - completed_list = opt->completer(autoCompleteOpaque, - opt->completer_flags); - if (completed_list) { - while ((completed_name = completed_list[completed_list_index])) { - completed_list_index++; - if (!STRPREFIX(completed_name, sanitized_text)) - continue; - res = vshStrdup(NULL, completed_name); - return res; + } else { + if (complete_opts) { + res = vshReadlineOptionsGenerator(sanitized_text, state, cmd); + complete_opts = !!res; + } + + if (!complete_opts && complete_data) { + if (!completed_list && opt && opt->completer) + completed_list = opt->completer(autoCompleteOpaque, + opt->completer_flags); + if (completed_list) { + while ((completed_name = completed_list[completed_list_index])) { + completed_list_index++; + if (!STRPREFIX(completed_name, sanitized_text)) + continue; + res = vshStrdup(NULL, completed_name); + return res; + } + res = NULL; + virStringListFree(completed_list); + completed_list = NULL; + completed_list_index = 0; } - res = NULL; - virStringListFree(completed_list); - completed_list_index = 0; } } -- 2.13.6
The callback we're calling might need to make decisions on already passed arguments. For instance, a completer that is supposed to bring up list of domain's interfaces might want to see what domain user wants to work with. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/vsh.c | 7 ++++++- tools/vsh.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/vsh.c b/tools/vsh.c index dd2f06ada..121669574 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2859,9 +2859,14 @@ vshReadlineParse(const char *text, int state) } if (!complete_opts && complete_data) { - if (!completed_list && opt && opt->completer) + if (!completed_list && opt && opt->completer) { + vshCmd *partial = NULL; + vshCommandStringParse(autoCompleteOpaque, rl_line_buffer, &partial); completed_list = opt->completer(autoCompleteOpaque, + partial, opt->completer_flags); + vshCommandFree(partial); + } if (completed_list) { while ((completed_name = completed_list[completed_list_index])) { completed_list_index++; diff --git a/tools/vsh.h b/tools/vsh.h index 1ebd5c11a..c5a62e593 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -124,6 +124,7 @@ typedef struct _vshCmdOptDef vshCmdOptDef; typedef struct _vshControl vshControl; typedef char **(*vshCompleter)(vshControl *ctl, + const vshCmd *cmd, unsigned int flags); /* -- 2.13.6
This command is going to be called from bash completion script in the following form: virsh complete "start --domain" Its only purpose is to return list of possible strings for completion. Note that this is a 'hidden', unlisted command and therefore there's no documentation to it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 1 + tools/virt-admin.c | 1 + tools/vsh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 14 +++++++++++ 4 files changed, 84 insertions(+) diff --git a/tools/virsh.c b/tools/virsh.c index 7d6dc2620..f830331f6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -846,6 +846,7 @@ static const vshCmdDef virshCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "connect", .handler = cmdConnect, .opts = opts_connect, diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 5d7ef7988..c24ed95c0 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1356,6 +1356,7 @@ static const vshCmdDef vshAdmCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "uri", .handler = cmdURI, .opts = NULL, diff --git a/tools/vsh.c b/tools/vsh.c index 121669574..136acb0ab 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3419,3 +3419,71 @@ cmdSelfTest(vshControl *ctl ATTRIBUTE_UNUSED, return true; } + +/* ---------------------- + * Autocompletion command + * ---------------------- */ + +const vshCmdOptDef opts_complete[] = { + {.name = "string", + .type = VSH_OT_ARGV, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("partial string to autocomplete") + }, + {.name = NULL} +}; + +const vshCmdInfo info_complete[] = { + {.name = "help", + .data = N_("internal command for autocompletion") + }, + {.name = "desc", + .data = N_("internal use only") + }, + {.name = NULL} +}; + +bool +cmdComplete(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; +#ifdef WITH_READLINE + const vshClientHooks *hooks = ctl->hooks; + int stdin_fileno = STDIN_FILENO; + const char *arg = NULL; + char **matches = NULL, *tmp = NULL, **iter; + + if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) + goto cleanup; + + /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we + * need to prevent auth hooks reading any input. Therefore we + * have to close stdin and then connect ourselves. */ + VIR_FORCE_CLOSE(stdin_fileno); + + if (!(hooks && hooks->connHandler && hooks->connHandler(ctl, true))) + goto cleanup; + + autoCompleteOpaque = ctl; + + rl_basic_word_break_characters = " \t\n\\`@$><=;|&{("; + if (VIR_STRDUP(rl_line_buffer, arg) < 0) + goto cleanup; + + while ((tmp = strpbrk(arg, rl_basic_word_break_characters))) + arg = tmp + 1; + + if (!(matches = vshReadlineCompletion(arg, 0, 0))) + goto cleanup; + + for (iter = matches; *iter; iter++) + printf("%s\n", *iter); + + ret = true; + cleanup: + for (iter = matches; iter && *iter; iter++) + VIR_FREE(*iter); + VIR_FREE(matches); +#endif /* WITH_READLINE */ + return ret; +} diff --git a/tools/vsh.h b/tools/vsh.h index c5a62e593..f55ba86f0 100644 --- a/tools/vsh.h +++ b/tools/vsh.h @@ -382,6 +382,8 @@ extern const vshCmdInfo info_echo[]; extern const vshCmdInfo info_pwd[]; extern const vshCmdInfo info_quit[]; extern const vshCmdInfo info_selftest[]; +extern const vshCmdOptDef opts_complete[]; +extern const vshCmdInfo info_complete[]; bool cmdHelp(vshControl *ctl, const vshCmd *cmd); bool cmdCd(vshControl *ctl, const vshCmd *cmd); @@ -389,6 +391,7 @@ bool cmdEcho(vshControl *ctl, const vshCmd *cmd); bool cmdPwd(vshControl *ctl, const vshCmd *cmd); bool cmdQuit(vshControl *ctl, const vshCmd *cmd); bool cmdSelfTest(vshControl *ctl, const vshCmd *cmd); +bool cmdComplete(vshControl *ctl, const vshCmd *cmd); # define VSH_CMD_CD \ { \ @@ -454,6 +457,17 @@ bool cmdSelfTest(vshControl *ctl, const vshCmd *cmd); .alias = "self-test" \ } +# define VSH_CMD_COMPLETE \ + { \ + .name = "complete", \ + .handler = cmdComplete, \ + .opts = opts_complete, \ + .info = info_complete, \ + .flags = VSH_CMD_FLAG_NOCONNECT | VSH_CMD_FLAG_ALIAS, \ + .alias = "complete" \ + } + + /* readline */ char * vshReadline(vshControl *ctl, const char *prompt); -- 2.13.6
On Tue, Nov 07, 2017 at 01:22:55PM +0100, Michal Privoznik wrote:
This command is going to be called from bash completion script in the following form:
virsh complete "start --domain"
Its only purpose is to return list of possible strings for completion. Note that this is a 'hidden', unlisted command and therefore there's no documentation to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 1 + tools/virt-admin.c | 1 + tools/vsh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 14 +++++++++++ 4 files changed, 84 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 7d6dc2620..f830331f6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -846,6 +846,7 @@ static const vshCmdDef virshCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "connect", .handler = cmdConnect, .opts = opts_connect, diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 5d7ef7988..c24ed95c0 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1356,6 +1356,7 @@ static const vshCmdDef vshAdmCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "uri", .handler = cmdURI, .opts = NULL, diff --git a/tools/vsh.c b/tools/vsh.c index 121669574..136acb0ab 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3419,3 +3419,71 @@ cmdSelfTest(vshControl *ctl ATTRIBUTE_UNUSED,
return true; } + +/* ---------------------- + * Autocompletion command + * ---------------------- */ + +const vshCmdOptDef opts_complete[] = { + {.name = "string", + .type = VSH_OT_ARGV, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("partial string to autocomplete") + }, + {.name = NULL} +}; + +const vshCmdInfo info_complete[] = { + {.name = "help", + .data = N_("internal command for autocompletion") + }, + {.name = "desc", + .data = N_("internal use only") + }, + {.name = NULL} +}; + +bool +cmdComplete(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; +#ifdef WITH_READLINE + const vshClientHooks *hooks = ctl->hooks; + int stdin_fileno = STDIN_FILENO; + const char *arg = NULL; + char **matches = NULL, *tmp = NULL, **iter; + + if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) + goto cleanup; + + /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we + * need to prevent auth hooks reading any input. Therefore we + * have to close stdin and then connect ourselves. */ + VIR_FORCE_CLOSE(stdin_fileno); + + if (!(hooks && hooks->connHandler && hooks->connHandler(ctl, true))) + goto cleanup; + + autoCompleteOpaque = ctl; + + rl_basic_word_break_characters = " \t\n\\`@$><=;|&{("; + if (VIR_STRDUP(rl_line_buffer, arg) < 0) + goto cleanup; + + while ((tmp = strpbrk(arg, rl_basic_word_break_characters))) + arg = tmp + 1; + + if (!(matches = vshReadlineCompletion(arg, 0, 0))) + goto cleanup; + + for (iter = matches; *iter; iter++) + printf("%s\n", *iter); + + ret = true; + cleanup: + for (iter = matches; iter && *iter; iter++) + VIR_FREE(*iter); + VIR_FREE(matches); +#endif /* WITH_READLINE */
Do we really need readline for it? Did I miss something or are we really just using it here just so we don't have to split the vshReadlineParse() in some way? Don't get me wrong, I'm OK with that, the split would be too complicated and who doesn't use readline, right? It just feels weird.
On 11/08/2017 03:23 PM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:55PM +0100, Michal Privoznik wrote:
This command is going to be called from bash completion script in the following form:
virsh complete "start --domain"
Its only purpose is to return list of possible strings for completion. Note that this is a 'hidden', unlisted command and therefore there's no documentation to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 1 + tools/virt-admin.c | 1 + tools/vsh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 14 +++++++++++ 4 files changed, 84 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 7d6dc2620..f830331f6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -846,6 +846,7 @@ static const vshCmdDef virshCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "connect", .handler = cmdConnect, .opts = opts_connect, diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 5d7ef7988..c24ed95c0 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1356,6 +1356,7 @@ static const vshCmdDef vshAdmCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "uri", .handler = cmdURI, .opts = NULL, diff --git a/tools/vsh.c b/tools/vsh.c index 121669574..136acb0ab 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3419,3 +3419,71 @@ cmdSelfTest(vshControl *ctl ATTRIBUTE_UNUSED,
return true; } + +/* ---------------------- + * Autocompletion command + * ---------------------- */ + +const vshCmdOptDef opts_complete[] = { + {.name = "string", + .type = VSH_OT_ARGV, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("partial string to autocomplete") + }, + {.name = NULL} +}; + +const vshCmdInfo info_complete[] = { + {.name = "help", + .data = N_("internal command for autocompletion") + }, + {.name = "desc", + .data = N_("internal use only") + }, + {.name = NULL} +}; + +bool +cmdComplete(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; +#ifdef WITH_READLINE + const vshClientHooks *hooks = ctl->hooks; + int stdin_fileno = STDIN_FILENO; + const char *arg = NULL; + char **matches = NULL, *tmp = NULL, **iter; + + if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) + goto cleanup; + + /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we + * need to prevent auth hooks reading any input. Therefore we + * have to close stdin and then connect ourselves. */ + VIR_FORCE_CLOSE(stdin_fileno); + + if (!(hooks && hooks->connHandler && hooks->connHandler(ctl, true))) + goto cleanup; + + autoCompleteOpaque = ctl; + + rl_basic_word_break_characters = " \t\n\\`@$><=;|&{("; + if (VIR_STRDUP(rl_line_buffer, arg) < 0) + goto cleanup; + + while ((tmp = strpbrk(arg, rl_basic_word_break_characters))) + arg = tmp + 1; + + if (!(matches = vshReadlineCompletion(arg, 0, 0))) + goto cleanup; + + for (iter = matches; *iter; iter++) + printf("%s\n", *iter); + + ret = true; + cleanup: + for (iter = matches; iter && *iter; iter++) + VIR_FREE(*iter); + VIR_FREE(matches); +#endif /* WITH_READLINE */
Do we really need readline for it? Did I miss something or are we really just using it here just so we don't have to split the vshReadlineParse() in some way? Don't get me wrong, I'm OK with that, the split would be too complicated and who doesn't use readline, right? It just feels weird.
Yes we do need readline unfortunately. vshReadlineCompletion() calls rl_completion_matches() which filters strings returned by vshReadlineParse() so that list of strings presented to user corresponds to their input. We could reimplement the rl_* function ourselves if we really want to be readline independent. On the other hand - readline is everywhere, so why should we bother? Michal
On Wed, Nov 08, 2017 at 04:00:35PM +0100, Michal Privoznik wrote:
On 11/08/2017 03:23 PM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:55PM +0100, Michal Privoznik wrote:
This command is going to be called from bash completion script in the following form:
virsh complete "start --domain"
Its only purpose is to return list of possible strings for completion. Note that this is a 'hidden', unlisted command and therefore there's no documentation to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 1 + tools/virt-admin.c | 1 + tools/vsh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 14 +++++++++++ 4 files changed, 84 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 7d6dc2620..f830331f6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -846,6 +846,7 @@ static const vshCmdDef virshCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "connect", .handler = cmdConnect, .opts = opts_connect, diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 5d7ef7988..c24ed95c0 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1356,6 +1356,7 @@ static const vshCmdDef vshAdmCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "uri", .handler = cmdURI, .opts = NULL, diff --git a/tools/vsh.c b/tools/vsh.c index 121669574..136acb0ab 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3419,3 +3419,71 @@ cmdSelfTest(vshControl *ctl ATTRIBUTE_UNUSED,
return true; } + +/* ---------------------- + * Autocompletion command + * ---------------------- */ + +const vshCmdOptDef opts_complete[] = { + {.name = "string", + .type = VSH_OT_ARGV, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("partial string to autocomplete") + }, + {.name = NULL} +}; + +const vshCmdInfo info_complete[] = { + {.name = "help", + .data = N_("internal command for autocompletion") + }, + {.name = "desc", + .data = N_("internal use only") + }, + {.name = NULL} +}; + +bool +cmdComplete(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; +#ifdef WITH_READLINE + const vshClientHooks *hooks = ctl->hooks; + int stdin_fileno = STDIN_FILENO; + const char *arg = NULL; + char **matches = NULL, *tmp = NULL, **iter; + + if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) + goto cleanup; + + /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we + * need to prevent auth hooks reading any input. Therefore we + * have to close stdin and then connect ourselves. */ + VIR_FORCE_CLOSE(stdin_fileno); + + if (!(hooks && hooks->connHandler && hooks->connHandler(ctl, true))) + goto cleanup; + + autoCompleteOpaque = ctl; + + rl_basic_word_break_characters = " \t\n\\`@$><=;|&{("; + if (VIR_STRDUP(rl_line_buffer, arg) < 0) + goto cleanup; + + while ((tmp = strpbrk(arg, rl_basic_word_break_characters))) + arg = tmp + 1; + + if (!(matches = vshReadlineCompletion(arg, 0, 0))) + goto cleanup; + + for (iter = matches; *iter; iter++) + printf("%s\n", *iter); + + ret = true; + cleanup: + for (iter = matches; iter && *iter; iter++) + VIR_FREE(*iter); + VIR_FREE(matches); +#endif /* WITH_READLINE */
Do we really need readline for it? Did I miss something or are we really just using it here just so we don't have to split the vshReadlineParse() in some way? Don't get me wrong, I'm OK with that, the split would be too complicated and who doesn't use readline, right? It just feels weird.
Yes we do need readline unfortunately. vshReadlineCompletion() calls rl_completion_matches() which filters strings returned by vshReadlineParse() so that list of strings presented to user corresponds to their input. We could reimplement the rl_* function ourselves if we really want to be readline independent. On the other hand - readline is everywhere, so why should we bother?
Sure, I agree, my question was if this is using it just for filtering strings, looks like yes :D
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Nov 07, 2017 at 01:22:55PM +0100, Michal Privoznik wrote:
This command is going to be called from bash completion script in the following form:
virsh complete "start --domain"
Its only purpose is to return list of possible strings for completion. Note that this is a 'hidden', unlisted command and therefore there's no documentation to it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh.c | 1 + tools/virt-admin.c | 1 + tools/vsh.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ tools/vsh.h | 14 +++++++++++ 4 files changed, 84 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 7d6dc2620..f830331f6 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -846,6 +846,7 @@ static const vshCmdDef virshCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "connect", .handler = cmdConnect, .opts = opts_connect, diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 5d7ef7988..c24ed95c0 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -1356,6 +1356,7 @@ static const vshCmdDef vshAdmCmds[] = { VSH_CMD_PWD, VSH_CMD_QUIT, VSH_CMD_SELF_TEST, + VSH_CMD_COMPLETE, {.name = "uri", .handler = cmdURI, .opts = NULL, diff --git a/tools/vsh.c b/tools/vsh.c index 121669574..136acb0ab 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -3419,3 +3419,71 @@ cmdSelfTest(vshControl *ctl ATTRIBUTE_UNUSED,
return true; } + +/* ---------------------- + * Autocompletion command + * ---------------------- */ + +const vshCmdOptDef opts_complete[] = { + {.name = "string", + .type = VSH_OT_ARGV, + .flags = VSH_OFLAG_EMPTY_OK, + .help = N_("partial string to autocomplete") + }, + {.name = NULL} +}; + +const vshCmdInfo info_complete[] = { + {.name = "help", + .data = N_("internal command for autocompletion") + }, + {.name = "desc", + .data = N_("internal use only") + }, + {.name = NULL} +}; + +bool +cmdComplete(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; +#ifdef WITH_READLINE + const vshClientHooks *hooks = ctl->hooks; + int stdin_fileno = STDIN_FILENO; + const char *arg = NULL; + char **matches = NULL, *tmp = NULL, **iter; + + if (vshCommandOptStringQuiet(ctl, cmd, "string", &arg) <= 0) + goto cleanup; + + /* This command is flagged VSH_CMD_FLAG_NOCONNECT because we + * need to prevent auth hooks reading any input. Therefore we + * have to close stdin and then connect ourselves. */ + VIR_FORCE_CLOSE(stdin_fileno); + + if (!(hooks && hooks->connHandler && hooks->connHandler(ctl, true))) + goto cleanup; + + autoCompleteOpaque = ctl; + + rl_basic_word_break_characters = " \t\n\\`@$><=;|&{("; + if (VIR_STRDUP(rl_line_buffer, arg) < 0) + goto cleanup; + + while ((tmp = strpbrk(arg, rl_basic_word_break_characters))) + arg = tmp + 1; + + if (!(matches = vshReadlineCompletion(arg, 0, 0))) + goto cleanup; +
One more thing here, you're skipping to the last arg here, I guess this is the reason why <TAB><TAB> doesn't work in the middle of the input, only at the end. Maybe there should be another optional parameter that would tell you where the completion was requested.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 3 ++ libvirt.spec.in | 2 ++ m4/virt-bash-completion.m4 | 74 ++++++++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 22 ++++++++++++-- tools/bash-completion/vsh | 73 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 m4/virt-bash-completion.m4 create mode 100644 tools/bash-completion/vsh diff --git a/configure.ac b/configure.ac index b2d991c3b..9103612bb 100644 --- a/configure.ac +++ b/configure.ac @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR LIBVIRT_ARG_ATTR LIBVIRT_ARG_AUDIT LIBVIRT_ARG_AVAHI +LIBVIRT_ARG_BASH_COMPLETION LIBVIRT_ARG_BLKID LIBVIRT_ARG_CAPNG LIBVIRT_ARG_CURL @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC LIBVIRT_CHECK_ATTR LIBVIRT_CHECK_AUDIT LIBVIRT_CHECK_AVAHI +LIBVIRT_CHECK_BASH_COMPLETION LIBVIRT_CHECK_BLKID LIBVIRT_CHECK_CAPNG LIBVIRT_CHECK_CURL @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR LIBVIRT_RESULT_ATTR LIBVIRT_RESULT_AUDIT LIBVIRT_RESULT_AVAHI +LIBVIRT_RESULT_BASH_COMPLETION LIBVIRT_RESULT_BLKID LIBVIRT_RESULT_CAPNG LIBVIRT_RESULT_CURL diff --git a/libvirt.spec.in b/libvirt.spec.in index b00689cab..67bbd128c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2043,6 +2043,8 @@ exit 0 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp %{_datadir}/systemtap/tapset/libvirt_functions.stp +%{_datadir}/bash-completion/completions/vsh + %if %{with_systemd} %{_unitdir}/libvirt-guests.service diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4 new file mode 100644 index 000000000..e1ef58740 --- /dev/null +++ b/m4/virt-bash-completion.m4 @@ -0,0 +1,74 @@ +dnl Bash completion support +dnl +dnl Copyright (C) 2017 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl +dnl Inspired by libguestfs code. +dnl + +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[ + LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion], [check], [2.0]) + LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR], + [directory containing bash completions scripts], + [check]) +]) + +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [ + AC_REQUIRE([LIBVIRT_CHECK_READLINE]) + + if test "x$with_readline" != "xyes" ; then + if test "x$with_bash_completion" != "xyes" ; then + with_bash_completion=no + else + AC_MSG_ERROR([readline is required for bash completion support]) + fi + else + if test "x$with_bash_completion" = "xcheck" ; then + with_bash_completion=yes + fi + fi + + LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0]) + + if test "x$with_bash_completion" = "xyes" ; then + if test "x$with_bash_completions_dir" = "xcheck"; then + AC_MSG_CHECKING([for bash-completions directory]) + BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir bash-completion)" + AC_MSG_RESULT([$BASH_COMPLETIONS_DIR]) + + dnl Replace bash completions's exec_prefix with our own. + dnl Note that ${exec_prefix} is kept verbatim at this point in time, + dnl and will only be expanded later, when make is called: this makes + dnl it possible to override such prefix at compilation or installation + dnl time + bash_completions_prefix="$($PKG_CONFIG --variable=prefix bash-completion)" + if test "x$bash_completions_prefix" = "x" ; then + bash_completions_prefix="/usr" + fi + + BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}" + elif test "x$with_bash_completions_dir" = "xno" || test "x$with_bash_completions_dir" = "xyes"; then + AC_MSG_ERROR([bash-completions-dir must be used only with valid path]) + else + BASH_COMPLETIONS_DIR=$with_bash_completions_dir + fi + AC_SUBST([BASH_COMPLETIONS_DIR]) + fi +]) + +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[ + LIBVIRT_RESULT_LIB([BASH_COMPLETION]) +]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 7513a73ac..34a81e69c 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -66,6 +66,7 @@ EXTRA_DIST = \ libvirt-guests.sysconf \ virt-login-shell.conf \ virsh-edit.c \ + bash-completion/vsh \ $(PODFILES) \ $(MANINFILES) \ $(NULL) @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" < $< > $@-t && \ mv $@-t $@ -install-data-local: install-init install-systemd install-nss +install-data-local: install-init install-systemd install-nss \ + install-bash-completion -uninstall-local: uninstall-init uninstall-systemd uninstall-nss +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \ + uninstall-bash-completion install-sysconfig: $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status mv $@-t $@ +if WITH_BASH_COMPLETION +install-bash-completion: + $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)" + $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \ + "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh" + +uninstall-bash-completion: + rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh + rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||: +else ! WITH_BASH_COMPLETION +install-bash-completion: +uninstall-bash-completion: +endif ! WITH_BASH_COMPLETION + + EXTRA_DIST += \ wireshark/util/genxdrstub.pl \ wireshark/util/make-dissector-reg diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh new file mode 100644 index 000000000..0c923c8b5 --- /dev/null +++ b/tools/bash-completion/vsh @@ -0,0 +1,73 @@ +# +# virsh & virt-admin completion command +# + +_vsh_complete() +{ + local words cword c=0 i=0 cur RO URI CMDLINE INPUT A + + # Here, $COMP_WORDS is an array of words on the bash + # command line that user wants to complete. However, when + # parsing command line, the default set of word breaks is + # applied. This doesn't work for us as it mangles libvirt + # arguments, e.g. connection URI (with the default set it's + # split into multiple items within the array). Fortunately, + # there's a fixup function for the array. + _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword + COMP_WORDS=( "${words[@]}" ) + COMP_CWORD=${cword} + cur=${COMP_WORDS[$COMP_CWORD]} + + # See what URI is user trying to connect to and if they are + # connecting RO. Honour that. + while [ $c -le $COMP_CWORD ]; do + word="${COMP_WORDS[c]}" + case "$word" in + -r|--readonly) RO=1 ;; + -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;; + *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;; + esac + c=$((++c)) + done + + CMDLINE= + if [ -n "${RO}" ]; then + CMDLINE="${CMDLINE} -r" + fi + if [ -n "${URI}" ]; then + CMDLINE="${CMDLINE} -c ${URI}" + fi + + INPUT="${COMP_WORDS[@]:$i}" + + # Uncomment these lines for easy debug. +# echo; +# echo "RO=${flag_ro}"; +# echo "URI=${URI}"; +# echo "CMDLINE=${CMDLINE}"; +# echo "INPUT[${#INPUT[@]}]=**${INPUT}**"; +# echo "cur=${cur}"; +# echo; +# return 0; + + # Small shortcut here. According to manpage: + # When the function is executed, the first argument ($1) is + # the name of the command whose arguments are being + # completed. + # Therefore, we might just run $1. + A=($($1 ${CMDLINE} complete "${INPUT}" 2>/dev/null)) + + # If our 'complete' command hasn't offered anything offer + # filedir completion. + if [ ${#A[@]} -gt 0 ]; then + COMPREPLY=($(compgen -W "${A[*]%--}" -- ${cur})) + else + _filedir + fi + __ltrim_colon_completions "$cur" + return 0 +} && +complete -F _vsh_complete virsh && +complete -F _vsh_complete virt-admin + +# vim: ft=sh:et:ts=4:sw=4:tw=80 -- 2.13.6
On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 3 ++ libvirt.spec.in | 2 ++ m4/virt-bash-completion.m4 | 74 ++++++++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 22 ++++++++++++-- tools/bash-completion/vsh | 73 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 m4/virt-bash-completion.m4 create mode 100644 tools/bash-completion/vsh
diff --git a/configure.ac b/configure.ac index b2d991c3b..9103612bb 100644 --- a/configure.ac +++ b/configure.ac @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR LIBVIRT_ARG_ATTR LIBVIRT_ARG_AUDIT LIBVIRT_ARG_AVAHI +LIBVIRT_ARG_BASH_COMPLETION LIBVIRT_ARG_BLKID LIBVIRT_ARG_CAPNG LIBVIRT_ARG_CURL @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC LIBVIRT_CHECK_ATTR LIBVIRT_CHECK_AUDIT LIBVIRT_CHECK_AVAHI +LIBVIRT_CHECK_BASH_COMPLETION LIBVIRT_CHECK_BLKID LIBVIRT_CHECK_CAPNG LIBVIRT_CHECK_CURL @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR LIBVIRT_RESULT_ATTR LIBVIRT_RESULT_AUDIT LIBVIRT_RESULT_AVAHI +LIBVIRT_RESULT_BASH_COMPLETION LIBVIRT_RESULT_BLKID LIBVIRT_RESULT_CAPNG LIBVIRT_RESULT_CURL diff --git a/libvirt.spec.in b/libvirt.spec.in index b00689cab..67bbd128c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2043,6 +2043,8 @@ exit 0 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp %{_datadir}/systemtap/tapset/libvirt_functions.stp
+%{_datadir}/bash-completion/completions/vsh +
%if %{with_systemd} %{_unitdir}/libvirt-guests.service diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4 new file mode 100644 index 000000000..e1ef58740 --- /dev/null +++ b/m4/virt-bash-completion.m4 @@ -0,0 +1,74 @@ +dnl Bash completion support +dnl +dnl Copyright (C) 2017 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl +dnl Inspired by libguestfs code. +dnl + +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[ + LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion], [check], [2.0]) + LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR], + [directory containing bash completions scripts], + [check]) +]) + +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [ + AC_REQUIRE([LIBVIRT_CHECK_READLINE]) + + if test "x$with_readline" != "xyes" ; then + if test "x$with_bash_completion" != "xyes" ; then + with_bash_completion=no + else + AC_MSG_ERROR([readline is required for bash completion support]) + fi + else + if test "x$with_bash_completion" = "xcheck" ; then + with_bash_completion=yes + fi
You should change the "check" to "yes" only after everything below succeeded.
+ fi + + LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0]) + + if test "x$with_bash_completion" = "xyes" ; then + if test "x$with_bash_completions_dir" = "xcheck"; then + AC_MSG_CHECKING([for bash-completions directory]) + BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir bash-completion)" + AC_MSG_RESULT([$BASH_COMPLETIONS_DIR]) + + dnl Replace bash completions's exec_prefix with our own. + dnl Note that ${exec_prefix} is kept verbatim at this point in time, + dnl and will only be expanded later, when make is called: this makes + dnl it possible to override such prefix at compilation or installation + dnl time + bash_completions_prefix="$($PKG_CONFIG --variable=prefix bash-completion)" + if test "x$bash_completions_prefix" = "x" ; then + bash_completions_prefix="/usr" + fi + + BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}" + elif test "x$with_bash_completions_dir" = "xno" || test "x$with_bash_completions_dir" = "xyes"; then + AC_MSG_ERROR([bash-completions-dir must be used only with valid path])
Otherwise you can error out here ^^ even when nobody requested anything.
+ else + BASH_COMPLETIONS_DIR=$with_bash_completions_dir + fi + AC_SUBST([BASH_COMPLETIONS_DIR]) + fi +]) + +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[ + LIBVIRT_RESULT_LIB([BASH_COMPLETION]) +]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 7513a73ac..34a81e69c 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -66,6 +66,7 @@ EXTRA_DIST = \ libvirt-guests.sysconf \ virt-login-shell.conf \ virsh-edit.c \ + bash-completion/vsh \ $(PODFILES) \ $(MANINFILES) \ $(NULL) @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" < $< > $@-t && \ mv $@-t $@
-install-data-local: install-init install-systemd install-nss +install-data-local: install-init install-systemd install-nss \ + install-bash-completion
-uninstall-local: uninstall-init uninstall-systemd uninstall-nss +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \ + uninstall-bash-completion
install-sysconfig: $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status mv $@-t $@
+if WITH_BASH_COMPLETION +install-bash-completion: + $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)" + $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \ + "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh" + +uninstall-bash-completion: + rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh + rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||: +else ! WITH_BASH_COMPLETION +install-bash-completion: +uninstall-bash-completion: +endif ! WITH_BASH_COMPLETION + + EXTRA_DIST += \ wireshark/util/genxdrstub.pl \ wireshark/util/make-dissector-reg diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh new file mode 100644 index 000000000..0c923c8b5 --- /dev/null +++ b/tools/bash-completion/vsh @@ -0,0 +1,73 @@ +# +# virsh & virt-admin completion command +# + +_vsh_complete() +{ + local words cword c=0 i=0 cur RO URI CMDLINE INPUT A + + # Here, $COMP_WORDS is an array of words on the bash + # command line that user wants to complete. However, when + # parsing command line, the default set of word breaks is + # applied. This doesn't work for us as it mangles libvirt + # arguments, e.g. connection URI (with the default set it's + # split into multiple items within the array). Fortunately, + # there's a fixup function for the array. + _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword + COMP_WORDS=( "${words[@]}" ) + COMP_CWORD=${cword} + cur=${COMP_WORDS[$COMP_CWORD]} + + # See what URI is user trying to connect to and if they are + # connecting RO. Honour that. + while [ $c -le $COMP_CWORD ]; do + word="${COMP_WORDS[c]}" + case "$word" in + -r|--readonly) RO=1 ;; + -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;; + *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;; + esac + c=$((++c)) + done + + CMDLINE= + if [ -n "${RO}" ]; then + CMDLINE="${CMDLINE} -r" + fi + if [ -n "${URI}" ]; then + CMDLINE="${CMDLINE} -c ${URI}" + fi +
When I see all the things you have to do here, wouldn't it be easier to have a virsh 'option' rather than a 'command' so that we don't have to parse anything twice and just circumvent the command execution in virsh itself? You would just run the same command with '-C' (for example) appended after the program name. Otherwise looks OK, I would just like your input on it. Should we have any tests for it? ;-) =D
On 11/08/2017 03:46 PM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 3 ++ libvirt.spec.in | 2 ++ m4/virt-bash-completion.m4 | 74 ++++++++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 22 ++++++++++++-- tools/bash-completion/vsh | 73 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 m4/virt-bash-completion.m4 create mode 100644 tools/bash-completion/vsh
diff --git a/configure.ac b/configure.ac index b2d991c3b..9103612bb 100644 --- a/configure.ac +++ b/configure.ac @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR LIBVIRT_ARG_ATTR LIBVIRT_ARG_AUDIT LIBVIRT_ARG_AVAHI +LIBVIRT_ARG_BASH_COMPLETION LIBVIRT_ARG_BLKID LIBVIRT_ARG_CAPNG LIBVIRT_ARG_CURL @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC LIBVIRT_CHECK_ATTR LIBVIRT_CHECK_AUDIT LIBVIRT_CHECK_AVAHI +LIBVIRT_CHECK_BASH_COMPLETION LIBVIRT_CHECK_BLKID LIBVIRT_CHECK_CAPNG LIBVIRT_CHECK_CURL @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR LIBVIRT_RESULT_ATTR LIBVIRT_RESULT_AUDIT LIBVIRT_RESULT_AVAHI +LIBVIRT_RESULT_BASH_COMPLETION LIBVIRT_RESULT_BLKID LIBVIRT_RESULT_CAPNG LIBVIRT_RESULT_CURL diff --git a/libvirt.spec.in b/libvirt.spec.in index b00689cab..67bbd128c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2043,6 +2043,8 @@ exit 0 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp %{_datadir}/systemtap/tapset/libvirt_functions.stp
+%{_datadir}/bash-completion/completions/vsh +
%if %{with_systemd} %{_unitdir}/libvirt-guests.service diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4 new file mode 100644 index 000000000..e1ef58740 --- /dev/null +++ b/m4/virt-bash-completion.m4 @@ -0,0 +1,74 @@ +dnl Bash completion support +dnl +dnl Copyright (C) 2017 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl +dnl Inspired by libguestfs code. +dnl + +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[ + LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion], [check], [2.0]) + LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR], + [directory containing bash completions scripts], + [check]) +]) + +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [ + AC_REQUIRE([LIBVIRT_CHECK_READLINE]) + + if test "x$with_readline" != "xyes" ; then + if test "x$with_bash_completion" != "xyes" ; then + with_bash_completion=no + else + AC_MSG_ERROR([readline is required for bash completion support]) + fi + else + if test "x$with_bash_completion" = "xcheck" ; then + with_bash_completion=yes + fi
You should change the "check" to "yes" only after everything below succeeded.
+ fi + + LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0]) + + if test "x$with_bash_completion" = "xyes" ; then + if test "x$with_bash_completions_dir" = "xcheck"; then + AC_MSG_CHECKING([for bash-completions directory]) + BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir bash-completion)" + AC_MSG_RESULT([$BASH_COMPLETIONS_DIR]) + + dnl Replace bash completions's exec_prefix with our own. + dnl Note that ${exec_prefix} is kept verbatim at this point in time, + dnl and will only be expanded later, when make is called: this makes + dnl it possible to override such prefix at compilation or installation + dnl time + bash_completions_prefix="$($PKG_CONFIG --variable=prefix bash-completion)" + if test "x$bash_completions_prefix" = "x" ; then + bash_completions_prefix="/usr" + fi + + BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
+ elif test "x$with_bash_completions_dir" = "xno" || test "x$with_bash_completions_dir" = "xyes"; then + AC_MSG_ERROR([bash-completions-dir must be used only with valid path])
Otherwise you can error out here ^^ even when nobody requested anything.
+ else + BASH_COMPLETIONS_DIR=$with_bash_completions_dir + fi + AC_SUBST([BASH_COMPLETIONS_DIR]) + fi +]) + +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[ + LIBVIRT_RESULT_LIB([BASH_COMPLETION]) +]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 7513a73ac..34a81e69c 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -66,6 +66,7 @@ EXTRA_DIST = \ libvirt-guests.sysconf \ virt-login-shell.conf \ virsh-edit.c \ + bash-completion/vsh \ $(PODFILES) \ $(MANINFILES) \ $(NULL) @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" < $< > $@-t && \ mv $@-t $@
-install-data-local: install-init install-systemd install-nss +install-data-local: install-init install-systemd install-nss \ + install-bash-completion
-uninstall-local: uninstall-init uninstall-systemd uninstall-nss +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \ + uninstall-bash-completion
install-sysconfig: $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status mv $@-t $@
+if WITH_BASH_COMPLETION +install-bash-completion: + $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)" + $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \ + "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh" + +uninstall-bash-completion: + rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh + rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||: +else ! WITH_BASH_COMPLETION +install-bash-completion: +uninstall-bash-completion: +endif ! WITH_BASH_COMPLETION + + EXTRA_DIST += \ wireshark/util/genxdrstub.pl \ wireshark/util/make-dissector-reg diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh new file mode 100644 index 000000000..0c923c8b5 --- /dev/null +++ b/tools/bash-completion/vsh @@ -0,0 +1,73 @@ +# +# virsh & virt-admin completion command +# + +_vsh_complete() +{ + local words cword c=0 i=0 cur RO URI CMDLINE INPUT A + + # Here, $COMP_WORDS is an array of words on the bash + # command line that user wants to complete. However, when + # parsing command line, the default set of word breaks is + # applied. This doesn't work for us as it mangles libvirt + # arguments, e.g. connection URI (with the default set it's + # split into multiple items within the array). Fortunately, + # there's a fixup function for the array. + _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword + COMP_WORDS=( "${words[@]}" ) + COMP_CWORD=${cword} + cur=${COMP_WORDS[$COMP_CWORD]} + + # See what URI is user trying to connect to and if they are + # connecting RO. Honour that. + while [ $c -le $COMP_CWORD ]; do + word="${COMP_WORDS[c]}" + case "$word" in + -r|--readonly) RO=1 ;; + -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;; + *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;; + esac + c=$((++c)) + done + + CMDLINE= + if [ -n "${RO}" ]; then + CMDLINE="${CMDLINE} -r" + fi + if [ -n "${URI}" ]; then + CMDLINE="${CMDLINE} -c ${URI}" + fi +
When I see all the things you have to do here, wouldn't it be easier to have a virsh 'option' rather than a 'command' so that we don't have to parse anything twice and just circumvent the command execution in virsh itself?
Not really. That would mean parsing the command line in cmdComplete. Which again might be incomplete and thus yet more code would be needed. I don't really see a problem with this approach - now that the bash script is written.
You would just run the same command with '-C' (for example) appended after the program name.
Yeah, there are dozen of other approaches. I've chosen this one. I'm failing to see why one is better than another one. Michal
On Wed, Nov 08, 2017 at 04:09:10PM +0100, Michal Privoznik wrote:
On 11/08/2017 03:46 PM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 3 ++ libvirt.spec.in | 2 ++ m4/virt-bash-completion.m4 | 74 ++++++++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 22 ++++++++++++-- tools/bash-completion/vsh | 73 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 m4/virt-bash-completion.m4 create mode 100644 tools/bash-completion/vsh
diff --git a/configure.ac b/configure.ac index b2d991c3b..9103612bb 100644 --- a/configure.ac +++ b/configure.ac @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR LIBVIRT_ARG_ATTR LIBVIRT_ARG_AUDIT LIBVIRT_ARG_AVAHI +LIBVIRT_ARG_BASH_COMPLETION LIBVIRT_ARG_BLKID LIBVIRT_ARG_CAPNG LIBVIRT_ARG_CURL @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC LIBVIRT_CHECK_ATTR LIBVIRT_CHECK_AUDIT LIBVIRT_CHECK_AVAHI +LIBVIRT_CHECK_BASH_COMPLETION LIBVIRT_CHECK_BLKID LIBVIRT_CHECK_CAPNG LIBVIRT_CHECK_CURL @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR LIBVIRT_RESULT_ATTR LIBVIRT_RESULT_AUDIT LIBVIRT_RESULT_AVAHI +LIBVIRT_RESULT_BASH_COMPLETION LIBVIRT_RESULT_BLKID LIBVIRT_RESULT_CAPNG LIBVIRT_RESULT_CURL diff --git a/libvirt.spec.in b/libvirt.spec.in index b00689cab..67bbd128c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2043,6 +2043,8 @@ exit 0 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp %{_datadir}/systemtap/tapset/libvirt_functions.stp
+%{_datadir}/bash-completion/completions/vsh +
%if %{with_systemd} %{_unitdir}/libvirt-guests.service diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4 new file mode 100644 index 000000000..e1ef58740 --- /dev/null +++ b/m4/virt-bash-completion.m4 @@ -0,0 +1,74 @@ +dnl Bash completion support +dnl +dnl Copyright (C) 2017 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl +dnl Inspired by libguestfs code. +dnl + +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[ + LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion], [check], [2.0]) + LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR], + [directory containing bash completions scripts], + [check]) +]) + +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [ + AC_REQUIRE([LIBVIRT_CHECK_READLINE]) + + if test "x$with_readline" != "xyes" ; then + if test "x$with_bash_completion" != "xyes" ; then + with_bash_completion=no + else + AC_MSG_ERROR([readline is required for bash completion support]) + fi + else + if test "x$with_bash_completion" = "xcheck" ; then + with_bash_completion=yes + fi
You should change the "check" to "yes" only after everything below succeeded.
+ fi + + LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0]) + + if test "x$with_bash_completion" = "xyes" ; then + if test "x$with_bash_completions_dir" = "xcheck"; then + AC_MSG_CHECKING([for bash-completions directory]) + BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir bash-completion)" + AC_MSG_RESULT([$BASH_COMPLETIONS_DIR]) + + dnl Replace bash completions's exec_prefix with our own. + dnl Note that ${exec_prefix} is kept verbatim at this point in time, + dnl and will only be expanded later, when make is called: this makes + dnl it possible to override such prefix at compilation or installation + dnl time + bash_completions_prefix="$($PKG_CONFIG --variable=prefix bash-completion)" + if test "x$bash_completions_prefix" = "x" ; then + bash_completions_prefix="/usr" + fi + + BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
+ elif test "x$with_bash_completions_dir" = "xno" || test "x$with_bash_completions_dir" = "xyes"; then + AC_MSG_ERROR([bash-completions-dir must be used only with valid path])
Otherwise you can error out here ^^ even when nobody requested anything.
+ else + BASH_COMPLETIONS_DIR=$with_bash_completions_dir + fi + AC_SUBST([BASH_COMPLETIONS_DIR]) + fi +]) + +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[ + LIBVIRT_RESULT_LIB([BASH_COMPLETION]) +]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 7513a73ac..34a81e69c 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -66,6 +66,7 @@ EXTRA_DIST = \ libvirt-guests.sysconf \ virt-login-shell.conf \ virsh-edit.c \ + bash-completion/vsh \ $(PODFILES) \ $(MANINFILES) \ $(NULL) @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" < $< > $@-t && \ mv $@-t $@
-install-data-local: install-init install-systemd install-nss +install-data-local: install-init install-systemd install-nss \ + install-bash-completion
-uninstall-local: uninstall-init uninstall-systemd uninstall-nss +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \ + uninstall-bash-completion
install-sysconfig: $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status mv $@-t $@
+if WITH_BASH_COMPLETION +install-bash-completion: + $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)" + $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \ + "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh" + +uninstall-bash-completion: + rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh + rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||: +else ! WITH_BASH_COMPLETION +install-bash-completion: +uninstall-bash-completion: +endif ! WITH_BASH_COMPLETION + + EXTRA_DIST += \ wireshark/util/genxdrstub.pl \ wireshark/util/make-dissector-reg diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh new file mode 100644 index 000000000..0c923c8b5 --- /dev/null +++ b/tools/bash-completion/vsh @@ -0,0 +1,73 @@ +# +# virsh & virt-admin completion command +# + +_vsh_complete() +{ + local words cword c=0 i=0 cur RO URI CMDLINE INPUT A + + # Here, $COMP_WORDS is an array of words on the bash + # command line that user wants to complete. However, when + # parsing command line, the default set of word breaks is + # applied. This doesn't work for us as it mangles libvirt + # arguments, e.g. connection URI (with the default set it's + # split into multiple items within the array). Fortunately, + # there's a fixup function for the array. + _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword + COMP_WORDS=( "${words[@]}" ) + COMP_CWORD=${cword} + cur=${COMP_WORDS[$COMP_CWORD]} + + # See what URI is user trying to connect to and if they are + # connecting RO. Honour that. + while [ $c -le $COMP_CWORD ]; do + word="${COMP_WORDS[c]}" + case "$word" in + -r|--readonly) RO=1 ;; + -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;; + *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;; + esac + c=$((++c)) + done + + CMDLINE= + if [ -n "${RO}" ]; then + CMDLINE="${CMDLINE} -r" + fi + if [ -n "${URI}" ]; then + CMDLINE="${CMDLINE} -c ${URI}" + fi +
When I see all the things you have to do here, wouldn't it be easier to have a virsh 'option' rather than a 'command' so that we don't have to parse anything twice and just circumvent the command execution in virsh itself?
Not really. That would mean parsing the command line in cmdComplete. Which again might be incomplete and thus yet more code would be needed. I don't really see a problem with this approach - now that the bash script is written.
No, there would be no cmdComplete. And we already parse the whole thing anyway.
You would just run the same command with '-C' (for example) appended after the program name.
Yeah, there are dozen of other approaches. I've chosen this one. I'm failing to see why one is better than another one.
Simplicity. I'm not against this approach, I just wanted an open discussion about an idea.
Michal
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 11/08/2017 04:18 PM, Martin Kletzander wrote:
On Wed, Nov 08, 2017 at 04:09:10PM +0100, Michal Privoznik wrote:
On 11/08/2017 03:46 PM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- configure.ac | 3 ++ libvirt.spec.in | 2 ++ m4/virt-bash-completion.m4 | 74 ++++++++++++++++++++++++++++++++++++++++++++++ tools/Makefile.am | 22 ++++++++++++-- tools/bash-completion/vsh | 73 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 m4/virt-bash-completion.m4 create mode 100644 tools/bash-completion/vsh
diff --git a/configure.ac b/configure.ac index b2d991c3b..9103612bb 100644 --- a/configure.ac +++ b/configure.ac @@ -242,6 +242,7 @@ LIBVIRT_ARG_APPARMOR LIBVIRT_ARG_ATTR LIBVIRT_ARG_AUDIT LIBVIRT_ARG_AVAHI +LIBVIRT_ARG_BASH_COMPLETION LIBVIRT_ARG_BLKID LIBVIRT_ARG_CAPNG LIBVIRT_ARG_CURL @@ -278,6 +279,7 @@ LIBVIRT_CHECK_ATOMIC LIBVIRT_CHECK_ATTR LIBVIRT_CHECK_AUDIT LIBVIRT_CHECK_AVAHI +LIBVIRT_CHECK_BASH_COMPLETION LIBVIRT_CHECK_BLKID LIBVIRT_CHECK_CAPNG LIBVIRT_CHECK_CURL @@ -976,6 +978,7 @@ LIBVIRT_RESULT_APPARMOR LIBVIRT_RESULT_ATTR LIBVIRT_RESULT_AUDIT LIBVIRT_RESULT_AVAHI +LIBVIRT_RESULT_BASH_COMPLETION LIBVIRT_RESULT_BLKID LIBVIRT_RESULT_CAPNG LIBVIRT_RESULT_CURL diff --git a/libvirt.spec.in b/libvirt.spec.in index b00689cab..67bbd128c 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -2043,6 +2043,8 @@ exit 0 %{_datadir}/systemtap/tapset/libvirt_qemu_probes*.stp %{_datadir}/systemtap/tapset/libvirt_functions.stp
+%{_datadir}/bash-completion/completions/vsh +
%if %{with_systemd} %{_unitdir}/libvirt-guests.service diff --git a/m4/virt-bash-completion.m4 b/m4/virt-bash-completion.m4 new file mode 100644 index 000000000..e1ef58740 --- /dev/null +++ b/m4/virt-bash-completion.m4 @@ -0,0 +1,74 @@ +dnl Bash completion support +dnl +dnl Copyright (C) 2017 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl +dnl Inspired by libguestfs code. +dnl + +AC_DEFUN([LIBVIRT_ARG_BASH_COMPLETION],[ + LIBVIRT_ARG_WITH_FEATURE([BASH_COMPLETION], [bash-completion], [check], [2.0]) + LIBVIRT_ARG_WITH([BASH_COMPLETIONS_DIR], + [directory containing bash completions scripts], + [check]) +]) + +AC_DEFUN([LIBVIRT_CHECK_BASH_COMPLETION], [ + AC_REQUIRE([LIBVIRT_CHECK_READLINE]) + + if test "x$with_readline" != "xyes" ; then + if test "x$with_bash_completion" != "xyes" ; then + with_bash_completion=no + else + AC_MSG_ERROR([readline is required for bash completion support]) + fi + else + if test "x$with_bash_completion" = "xcheck" ; then + with_bash_completion=yes + fi
You should change the "check" to "yes" only after everything below succeeded.
+ fi + + LIBVIRT_CHECK_PKG([BASH_COMPLETION], [bash-completion], [2.0]) + + if test "x$with_bash_completion" = "xyes" ; then + if test "x$with_bash_completions_dir" = "xcheck"; then + AC_MSG_CHECKING([for bash-completions directory]) + BASH_COMPLETIONS_DIR="$($PKG_CONFIG --variable=completionsdir bash-completion)" + AC_MSG_RESULT([$BASH_COMPLETIONS_DIR]) + + dnl Replace bash completions's exec_prefix with our own. + dnl Note that ${exec_prefix} is kept verbatim at this point in time, + dnl and will only be expanded later, when make is called: this makes + dnl it possible to override such prefix at compilation or installation + dnl time + bash_completions_prefix="$($PKG_CONFIG --variable=prefix bash-completion)" + if test "x$bash_completions_prefix" = "x" ; then + bash_completions_prefix="/usr" + fi + + BASH_COMPLETIONS_DIR='${exec_prefix}'"${BASH_COMPLETIONS_DIR#$bash_completions_prefix}"
+ elif test "x$with_bash_completions_dir" = "xno" || test "x$with_bash_completions_dir" = "xyes"; then + AC_MSG_ERROR([bash-completions-dir must be used only with valid path])
Otherwise you can error out here ^^ even when nobody requested anything.
+ else + BASH_COMPLETIONS_DIR=$with_bash_completions_dir + fi + AC_SUBST([BASH_COMPLETIONS_DIR]) + fi +]) + +AC_DEFUN([LIBVIRT_RESULT_BASH_COMPLETION],[ + LIBVIRT_RESULT_LIB([BASH_COMPLETION]) +]) diff --git a/tools/Makefile.am b/tools/Makefile.am index 7513a73ac..34a81e69c 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -66,6 +66,7 @@ EXTRA_DIST = \ libvirt-guests.sysconf \ virt-login-shell.conf \ virsh-edit.c \ + bash-completion/vsh \ $(PODFILES) \ $(MANINFILES) \ $(NULL) @@ -332,9 +333,11 @@ POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" < $< > $@-t && \ mv $@-t $@
-install-data-local: install-init install-systemd install-nss +install-data-local: install-init install-systemd install-nss \ + install-bash-completion
-uninstall-local: uninstall-init uninstall-systemd uninstall-nss +uninstall-local: uninstall-init uninstall-systemd uninstall-nss \ + uninstall-bash-completion
install-sysconfig: $(MKDIR_P) $(DESTDIR)$(sysconfdir)/sysconfig @@ -420,6 +423,21 @@ libvirt-guests.service: libvirt-guests.service.in $(top_builddir)/config.status mv $@-t $@
+if WITH_BASH_COMPLETION +install-bash-completion: + $(MKDIR_P) "$(DESTDIR)$(BASH_COMPLETIONS_DIR)" + $(INSTALL_SCRIPT) $(srcdir)/bash-completion/vsh \ + "$(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh" + +uninstall-bash-completion: + rm -f $(DESTDIR)$(BASH_COMPLETIONS_DIR)/vsh + rmdir $(DESTDIR)$(BASH_COMPLETIONS_DIR) ||: +else ! WITH_BASH_COMPLETION +install-bash-completion: +uninstall-bash-completion: +endif ! WITH_BASH_COMPLETION + + EXTRA_DIST += \ wireshark/util/genxdrstub.pl \ wireshark/util/make-dissector-reg diff --git a/tools/bash-completion/vsh b/tools/bash-completion/vsh new file mode 100644 index 000000000..0c923c8b5 --- /dev/null +++ b/tools/bash-completion/vsh @@ -0,0 +1,73 @@ +# +# virsh & virt-admin completion command +# + +_vsh_complete() +{ + local words cword c=0 i=0 cur RO URI CMDLINE INPUT A + + # Here, $COMP_WORDS is an array of words on the bash + # command line that user wants to complete. However, when + # parsing command line, the default set of word breaks is + # applied. This doesn't work for us as it mangles libvirt + # arguments, e.g. connection URI (with the default set it's + # split into multiple items within the array). Fortunately, + # there's a fixup function for the array. + _get_comp_words_by_ref -n "\"'><=;|&(:" -w words -i cword + COMP_WORDS=( "${words[@]}" ) + COMP_CWORD=${cword} + cur=${COMP_WORDS[$COMP_CWORD]} + + # See what URI is user trying to connect to and if they are + # connecting RO. Honour that. + while [ $c -le $COMP_CWORD ]; do + word="${COMP_WORDS[c]}" + case "$word" in + -r|--readonly) RO=1 ;; + -c|--connect) c=$((++c)); URI=${COMP_WORDS[c]} ;; + *) if [ $c -ne 0 ] && [ $i -eq 0 ]; then i=$c; break; fi ;; + esac + c=$((++c)) + done + + CMDLINE= + if [ -n "${RO}" ]; then + CMDLINE="${CMDLINE} -r" + fi + if [ -n "${URI}" ]; then + CMDLINE="${CMDLINE} -c ${URI}" + fi +
When I see all the things you have to do here, wouldn't it be easier to have a virsh 'option' rather than a 'command' so that we don't have to parse anything twice and just circumvent the command execution in virsh itself?
Not really. That would mean parsing the command line in cmdComplete. Which again might be incomplete and thus yet more code would be needed. I don't really see a problem with this approach - now that the bash script is written.
No, there would be no cmdComplete. And we already parse the whole thing anyway.
How would it generate the list then? -C would need set some boolean flag so that after parsing the vshReadlineCompletion() is called. So instead of having the code in a separate cmd*() function it would be worked into argv parsing. I don't really see the benefit.
You would just run the same command with '-C' (for example) appended after the program name.
Yeah, there are dozen of other approaches. I've chosen this one. I'm failing to see why one is better than another one.
Simplicity.
As I write above, while the bash script would be simpler, vsh's argv parsing would be more complicated. Or am I missing something?
I'm not against this approach, I just wanted an open discussion about an idea.
Sure. I'm not saying that my approach is the best there is. It's just that I understand this solution the most. Just take a look at our argv parsing - I don't wan to touch that code with 10 feet pole unless I really need to. Michal
On 11/08/2017 09:09 AM, Michal Privoznik wrote:
On 11/08/2017 03:46 PM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When I see all the things you have to do here, wouldn't it be easier to have a virsh 'option' rather than a 'command' so that we don't have to parse anything twice and just circumvent the command execution in virsh itself?
Not really. That would mean parsing the command line in cmdComplete. Which again might be incomplete and thus yet more code would be needed. I don't really see a problem with this approach - now that the bash script is written.
You would just run the same command with '-C' (for example) appended after the program name.
Yeah, there are dozen of other approaches. I've chosen this one. I'm failing to see why one is better than another one.
[I haven't read this thread closely yet, just adding a drive-by comment] Several years ago, when autocompletion was attempted (and failed) as a GSoC project, I had several ideas on how completion should work, and how it should be tested. Ideally, we need a new virsh command 'complete', which takes varargs, and then performs completion based on those args. So the bash script for completion for this scenario: $ virsh some-command --arg partial<TAB><TAB> is as simple as having the completion function in bash call: $ virsh complete some-command --arg partial<TAB><TAB> The complete command in virsh should then know that it is performing completion on some-command, and do enough arg parsing to determine how to complete 'partial' in the current context of whatever argument some-command would be parsing at that point. If a user in bash backs up the cursor and types <TAB> somewhere other than the end of the line, hopefully bash gives us enough hooks to call 'virsh complete args-up-to-TAB' by merely truncating whatever appears beyond the point where the user attempted tab completion. ALL the completion logic lives in virsh, nothing in bash - all bash has to do is insert 'complete' and call into virsh. That is, once you've written the bash script, it should NEVER need future modification, because any further improvements to completion will live in virsh (whether you use virsh in interactive mode or in batch mode from the shell). I don't know how much my original idea has carried over into your proposal in this thread, but you may want to read the emails I posted from the archives on the topic, for more ideas. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 11/08/2017 04:53 PM, Eric Blake wrote:
On 11/08/2017 09:09 AM, Michal Privoznik wrote:
On 11/08/2017 03:46 PM, Martin Kletzander wrote:
On Tue, Nov 07, 2017 at 01:22:56PM +0100, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
When I see all the things you have to do here, wouldn't it be easier to have a virsh 'option' rather than a 'command' so that we don't have to parse anything twice and just circumvent the command execution in virsh itself?
Not really. That would mean parsing the command line in cmdComplete. Which again might be incomplete and thus yet more code would be needed. I don't really see a problem with this approach - now that the bash script is written.
You would just run the same command with '-C' (for example) appended after the program name.
Yeah, there are dozen of other approaches. I've chosen this one. I'm failing to see why one is better than another one.
[I haven't read this thread closely yet, just adding a drive-by comment]
Several years ago, when autocompletion was attempted (and failed) as a GSoC project, I had several ideas on how completion should work, and how it should be tested.
Ideally, we need a new virsh command 'complete', which takes varargs, and then performs completion based on those args. So the bash script for completion for this scenario:
$ virsh some-command --arg partial<TAB><TAB>
is as simple as having the completion function in bash call:
$ virsh complete some-command --arg partial<TAB><TAB>
This is excatly what my patches are doing. With one tiny difference. In fact bash script calls: virsh complete "some-command --arg partial" so that I can have cmdComplete which takes exactly one string argument. Parsing of the string is then done within cmdComplete. I don't think that we have variable args in virsh for commands, do we?
The complete command in virsh should then know that it is performing completion on some-command, and do enough arg parsing to determine how to complete 'partial' in the current context of whatever argument some-command would be parsing at that point.
If a user in bash backs up the cursor and types <TAB> somewhere other than the end of the line, hopefully bash gives us enough hooks to call 'virsh complete args-up-to-TAB' by merely truncating whatever appears beyond the point where the user attempted tab completion.
ALL the completion logic lives in virsh, nothing in bash - all bash has to do is insert 'complete' and call into virsh. That is, once you've written the bash script, it should NEVER need future modification, because any further improvements to completion will live in virsh (whether you use virsh in interactive mode or in batch mode from the shell).
Correct. And again, my patches do this. For instance: virsh -r -c qemu+ssh://host/system domifaddr --domain<TAB><TAB> becomes: virsh -r -c qemu+ssh://host/system complete "domifaddr --domain" And it's the 'complete' command that is responsible for bringing up a list of possible strings. Michal
On 11/08/2017 10:06 AM, Michal Privoznik wrote:
$ virsh complete some-command --arg partial<TAB><TAB>
This is excatly what my patches are doing. With one tiny difference. In fact bash script calls:
virsh complete "some-command --arg partial"
so that I can have cmdComplete which takes exactly one string argument.
Ouch. That difference matters, and I don't think you want to do that. We don't want to reimplement shell parsing; if the user does: virsh snapshot-create-as 'domain with space' 'name with space' 'description with space', then my approach feeds those three arguments as is (the virsh parser doesn't have to undo any shell quoting), while your approach HAS to over-quote the original command line, then reimplement shell quoting to remove those quotes in virsh, in order to reconstruct the original line in spite of being temporarily squashed through one argument.
Parsing of the string is then done within cmdComplete. I don't think that we have variable args in virsh for commands, do we?
Yes, we do. See 'virsh echo' for an example of its use.
Correct. And again, my patches do this. For instance:
virsh -r -c qemu+ssh://host/system domifaddr --domain<TAB><TAB>
becomes:
virsh -r -c qemu+ssh://host/system complete "domifaddr --domain"
And it's the 'complete' command that is responsible for bringing up a list of possible strings.
Ah, you are trying to put 'complete' after virsh-specific options (-r, -c), but before the command (domifaddr). And if properly implemented, you should be able to have: virsh domif<TAB><TAB> show 'domifaddr' (and any other commands starting with 'domif') (by calling "virsh complete -- domif"), virsh domifaddr <TAB><TAB> show both a list of domain names AND a list of --options accepted by domifaddr (by calling "virsh complete -- domifaddr ''"), virsh domifaddr --<TAB><TAB> show a list of long options for domifaddr (by calling "virsh complete -- domifaddr --") (yes, I'm specifically making sure the second '--' doesn't get eaten by getopts, by inserting 'complete --' rather than just 'complete'), and virsh domifaddr a<TAB><TAB> show a filtered list of just domain names starting with 'a' (by calling "virsh complete domifaddr a"). That is, the completions should be context-sensitive based on how much of the command line is present prior to the <TAB><TAB>, and should NOT need the user to type an explicit --domain just to get domain completions. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 11/08/2017 05:26 PM, Eric Blake wrote:
On 11/08/2017 10:06 AM, Michal Privoznik wrote:
$ virsh complete some-command --arg partial<TAB><TAB>
This is excatly what my patches are doing. With one tiny difference. In fact bash script calls:
virsh complete "some-command --arg partial"
so that I can have cmdComplete which takes exactly one string argument.
Ouch. That difference matters, and I don't think you want to do that.
We don't want to reimplement shell parsing; if the user does:
virsh snapshot-create-as 'domain with space' 'name with space' 'description with space', then my approach feeds those three arguments as is (the virsh parser doesn't have to undo any shell quoting), while your approach HAS to over-quote the original command line, then reimplement shell quoting to remove those quotes in virsh, in order to reconstruct the original line in spite of being temporarily squashed through one argument.
Right. On the other hand, who uses space in file names? ;-)
Parsing of the string is then done within cmdComplete. I don't think that we have variable args in virsh for commands, do we?
Yes, we do. See 'virsh echo' for an example of its use.
Okay, I will look into this.
Correct. And again, my patches do this. For instance:
virsh -r -c qemu+ssh://host/system domifaddr --domain<TAB><TAB>
becomes:
virsh -r -c qemu+ssh://host/system complete "domifaddr --domain"
And it's the 'complete' command that is responsible for bringing up a list of possible strings.
Ah, you are trying to put 'complete' after virsh-specific options (-r, -c), but before the command (domifaddr).
Yes.
And if properly implemented, you should be able to have:
virsh domif<TAB><TAB>
show 'domifaddr' (and any other commands starting with 'domif') (by calling "virsh complete -- domif"),
Yup. This works.
virsh domifaddr <TAB><TAB>
show both a list of domain names AND a list of --options accepted by domifaddr (by calling "virsh complete -- domifaddr ''"),
This is still WIP. As I mention in the cover letter, the parser is hard to grasp for me and therefore this is yet to be implemented. Currently all you get is list of --options. However, completers for --option work. For instance: virsh start --domain <TAB><TAB> brings up list of domains to start.
virsh domifaddr --<TAB><TAB>
show a list of long options for domifaddr (by calling "virsh complete -- domifaddr --") (yes, I'm specifically making sure the second '--' doesn't get eaten by getopts, by inserting 'complete --' rather than just 'complete'), and
virsh domifaddr a<TAB><TAB>
show a filtered list of just domain names starting with 'a' (by calling "virsh complete domifaddr a").
That is, the completions should be context-sensitive based on how much of the command line is present prior to the <TAB><TAB>, and should NOT need the user to type an explicit --domain just to get domain completions.
Yeah, that's the idea. And I'm looking into the code, but it's not that easy for me to understand it. So, if anybody wants to help, be my guest. Michal
On 11/08/2017 05:26 PM, Eric Blake wrote:
On 11/08/2017 10:06 AM, Michal Privoznik wrote:
$ virsh complete some-command --arg partial<TAB><TAB>
This is excatly what my patches are doing. With one tiny difference. In fact bash script calls:
virsh complete "some-command --arg partial"
so that I can have cmdComplete which takes exactly one string argument.
Ouch. That difference matters, and I don't think you want to do that.
We don't want to reimplement shell parsing; if the user does:
virsh snapshot-create-as 'domain with space' 'name with space' 'description with space', then my approach feeds those three arguments as is (the virsh parser doesn't have to undo any shell quoting), while your approach HAS to over-quote the original command line, then reimplement shell quoting to remove those quotes in virsh, in order to reconstruct the original line in spite of being temporarily squashed through one argument.
Parsing of the string is then done within cmdComplete. I don't think that we have variable args in virsh for commands, do we?
Yes, we do. See 'virsh echo' for an example of its use.
So while implementing this I realized it will not fly. At ARGV level it is impossible to tell apart "--domain" and "--domain ". But the difference is important, because in the first case we want to offer list of all --options, while in the second case we want to call the --domain completer. Okay, this might not be the best example because there's no other option that shares the prefix, but for instance 'migrate' command has: --timeout --timeout-suspend --timeout-postcopy So if user types in: "virsh migrate --timeout" we have to bring up list: "--timeout" "--timeout-suspend" "--timeout-postcopy" and only after they type: "virsh migrate --timeout " we can call --timeout completer. Since it's impossible to differentiate these cases with VSH_OT_ARGV, I'm sticking with what I have now. Sure, it might has its own problems but those can be solved later. Michal
On 11/10/2017 05:01 AM, Michal Privoznik wrote:
So while implementing this I realized it will not fly. At ARGV level it is impossible to tell apart "--domain" and "--domain ".
But it is not impossible to tell apart '"--domain"' (one arg) and '"--domain" ""' (two args). If you have trailing whitespace, then tack on an empty argument.
But the difference is important, because in the first case we want to offer list of all --options, while in the second case we want to call the --domain completer.
Yes, the difference between one argument and two is significant - the completion should always be done on the last argument, but the last argument needs to be the empty string if it is not in the middle of a word. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/Makefile.am | 9 +++ tools/virsh-completer.c | 90 +++++++++++++++++++++ tools/virsh-completer.h | 33 ++++++++ tools/virsh-domain-monitor.c | 30 +++---- tools/virsh-domain.c | 182 ++++++++++++++++++++++--------------------- tools/virsh-snapshot.c | 24 +++--- tools/virsh.h | 7 +- 7 files changed, 256 insertions(+), 119 deletions(-) create mode 100644 tools/virsh-completer.c create mode 100644 tools/virsh-completer.h diff --git a/tools/Makefile.am b/tools/Makefile.am index 34a81e69c..8eddc7bbe 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -231,6 +231,15 @@ virsh_SOURCES = \ virsh-volume.c virsh-volume.h \ $(NULL) +VIRSH_COMPLETER = \ + virsh-completer.c virsh-completer.h + +if WITH_READLINE +virsh_SOURCES += $(VIRSH_COMPLETER) +else ! WITH_READLINE +EXTRA_DIST += $(VIRSH_COMPLETER) +endif ! WITH_READLINE + virsh_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c new file mode 100644 index 000000000..4e32b882b --- /dev/null +++ b/tools/virsh-completer.c @@ -0,0 +1,90 @@ +/* + * virsh-completer.c: virsh completer callbacks + * + * Copyright (C) 2017 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Michal Privoznik <mprivozn@redhat.com> + * + */ + +#include <config.h> + +#include "virsh-completer.h" +#include "virsh.h" +#include "virsh-util.h" +#include "internal.h" +#include "viralloc.h" +#include "virstring.h" + + +char ** +virshDomainNameCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + virDomainPtr *domains = NULL; + int ndomains = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(VIR_CONNECT_LIST_DOMAINS_ACTIVE | + VIR_CONNECT_LIST_DOMAINS_INACTIVE | + VIR_CONNECT_LIST_DOMAINS_PERSISTENT | + VIR_CONNECT_LIST_DOMAINS_TRANSIENT | + VIR_CONNECT_LIST_DOMAINS_RUNNING | + VIR_CONNECT_LIST_DOMAINS_PAUSED | + VIR_CONNECT_LIST_DOMAINS_SHUTOFF | + VIR_CONNECT_LIST_DOMAINS_OTHER | + VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE | + VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE | + VIR_CONNECT_LIST_DOMAINS_AUTOSTART | + VIR_CONNECT_LIST_DOMAINS_NO_AUTOSTART | + VIR_CONNECT_LIST_DOMAINS_HAS_SNAPSHOT | + VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT, + NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if ((ndomains = virConnectListAllDomains(priv->conn, &domains, flags)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, ndomains + 1) < 0) + goto error; + + for (i = 0; i < ndomains; i++) { + const char *name = virDomainGetName(domains[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virshDomainFree(domains[i]); + } + VIR_FREE(domains); + + return ret; + error: + + for (; i < ndomains; i++) + virshDomainFree(domains[i]); + VIR_FREE(domains); + for (i = 0; i < ndomains; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h new file mode 100644 index 000000000..288e17909 --- /dev/null +++ b/tools/virsh-completer.h @@ -0,0 +1,33 @@ +/* + * virsh-completer.h: virsh completer callbacks + * + * Copyright (C) 2017 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Michal Privoznik <mprivozn@redhat.com> + * + */ + +#ifndef VIRSH_COMPLETER +# define VIRSH_COMPLETER + +# include "vsh.h" + +char ** virshDomainNameCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + +#endif diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index b3bfc33c8..d0edb177d 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -40,8 +40,8 @@ #include "virxml.h" #include "virstring.h" -#define VIRSH_COMMON_OPT_DOMAIN_FULL \ - VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid")) +#define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \ + VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags) VIR_ENUM_DECL(virshDomainIOError) VIR_ENUM_IMPL(virshDomainIOError, @@ -278,7 +278,7 @@ static const vshCmdInfo info_dommemstat[] = { }; static const vshCmdOptDef opts_dommemstat[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "period", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ_OPT, @@ -390,7 +390,7 @@ static const vshCmdInfo info_domblkinfo[] = { }; static const vshCmdOptDef opts_domblkinfo[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "device", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -460,7 +460,7 @@ static const vshCmdInfo info_domblklist[] = { }; static const vshCmdOptDef opts_domblklist[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "inactive", .type = VSH_OT_BOOL, .help = N_("get inactive rather than running configuration") @@ -566,7 +566,7 @@ static const vshCmdInfo info_domiflist[] = { }; static const vshCmdOptDef opts_domiflist[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "inactive", .type = VSH_OT_BOOL, .help = N_("get inactive rather than running configuration") @@ -655,7 +655,7 @@ static const vshCmdInfo info_domif_getlink[] = { }; static const vshCmdOptDef opts_domif_getlink[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -752,7 +752,7 @@ static const vshCmdInfo info_domcontrol[] = { }; static const vshCmdOptDef opts_domcontrol[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -805,7 +805,7 @@ static const vshCmdInfo info_domblkstat[] = { }; static const vshCmdOptDef opts_domblkstat[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "device", .type = VSH_OT_STRING, .flags = VSH_OFLAG_EMPTY_OK, @@ -991,7 +991,7 @@ static const vshCmdInfo info_domifstat[] = { }; static const vshCmdOptDef opts_domifstat[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -1064,7 +1064,7 @@ static const vshCmdInfo info_domblkerror[] = { }; static const vshCmdOptDef opts_domblkerror[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -1125,7 +1125,7 @@ static const vshCmdInfo info_dominfo[] = { }; static const vshCmdOptDef opts_dominfo[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = NULL} }; @@ -1264,7 +1264,7 @@ static const vshCmdInfo info_domstate[] = { }; static const vshCmdOptDef opts_domstate[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "reason", .type = VSH_OT_BOOL, .help = N_("also print reason for the state") @@ -1316,7 +1316,7 @@ static const vshCmdInfo info_domtime[] = { }; static const vshCmdOptDef opts_domtime[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "now", .type = VSH_OT_BOOL, .help = N_("set to the time of the host running virsh") @@ -2145,7 +2145,7 @@ static const vshCmdInfo info_domifaddr[] = { }; static const vshCmdOptDef opts_domifaddr[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "interface", .type = VSH_OT_STRING, .flags = VSH_OFLAG_NONE, diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index cf612f73e..101a5f647 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -65,8 +65,8 @@ # define SA_SIGINFO 0 #endif -#define VIRSH_COMMON_OPT_DOMAIN_FULL \ - VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid")) +#define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \ + VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags) #define VIRSH_COMMON_OPT_DOMAIN_PERSISTENT \ {.name = "persistent", \ @@ -154,7 +154,7 @@ static const vshCmdInfo info_attach_device[] = { }; static const vshCmdOptDef opts_attach_device[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), VIRSH_COMMON_OPT_FILE(N_("XML file")), VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, VIRSH_COMMON_OPT_DOMAIN_CONFIG, @@ -236,7 +236,7 @@ static const vshCmdInfo info_attach_disk[] = { }; static const vshCmdOptDef opts_attach_disk[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "source", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ | VSH_OFLAG_EMPTY_OK, @@ -727,7 +727,7 @@ static const vshCmdInfo info_attach_interface[] = { }; static const vshCmdOptDef opts_attach_interface[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "type", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -1037,7 +1037,7 @@ static const vshCmdInfo info_autostart[] = { }; static const vshCmdOptDef opts_autostart[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "disable", .type = VSH_OT_BOOL, .help = N_("disable autostarting") @@ -1089,7 +1089,7 @@ static const vshCmdInfo info_blkdeviotune[] = { }; static const vshCmdOptDef opts_blkdeviotune[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "device", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -1416,7 +1416,7 @@ static const vshCmdInfo info_blkiotune[] = { }; static const vshCmdOptDef opts_blkiotune[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "weight", .type = VSH_OT_INT, .help = N_("IO Weight") @@ -1885,7 +1885,7 @@ static const vshCmdInfo info_block_commit[] = { }; static const vshCmdOptDef opts_block_commit[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "path", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -2110,7 +2110,7 @@ static const vshCmdInfo info_block_copy[] = { }; static const vshCmdOptDef opts_block_copy[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "path", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -2426,7 +2426,7 @@ static const vshCmdInfo info_block_job[] = { }; static const vshCmdOptDef opts_block_job[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "path", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -2669,7 +2669,7 @@ static const vshCmdInfo info_block_pull[] = { }; static const vshCmdOptDef opts_block_pull[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "path", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -2815,7 +2815,7 @@ static const vshCmdInfo info_block_resize[] = { }; static const vshCmdOptDef opts_block_resize[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "path", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -2879,7 +2879,7 @@ static const vshCmdInfo info_console[] = { }; static const vshCmdOptDef opts_console[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "devname", /* sc_prohibit_devname */ .type = VSH_OT_STRING, .help = N_("character device name") @@ -2973,7 +2973,7 @@ static const vshCmdInfo info_domif_setlink[] = { }; static const vshCmdOptDef opts_domif_setlink[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -3143,7 +3143,7 @@ static const vshCmdInfo info_domiftune[] = { }; static const vshCmdOptDef opts_domiftune[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -3340,7 +3340,7 @@ static const vshCmdInfo info_suspend[] = { }; static const vshCmdOptDef opts_suspend[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_RUNNING), {.name = NULL} }; @@ -3382,7 +3382,7 @@ static const vshCmdInfo info_dom_pm_suspend[] = { }; static const vshCmdOptDef opts_dom_pm_suspend[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_RUNNING), {.name = "target", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -3460,7 +3460,7 @@ static const vshCmdInfo info_dom_pm_wakeup[] = { }; static const vshCmdOptDef opts_dom_pm_wakeup[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_OTHER), {.name = NULL} }; @@ -3505,7 +3505,7 @@ static const vshCmdInfo info_undefine[] = { }; static const vshCmdOptDef opts_undefine[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_PERSISTENT), {.name = "managed-save", .type = VSH_OT_BOOL, .help = N_("remove domain managed state file") @@ -3923,7 +3923,8 @@ static const vshCmdInfo info_start[] = { }; static const vshCmdOptDef opts_start[] = { - VIRSH_COMMON_OPT_DOMAIN(N_("name of the inactive domain")), + VIRSH_COMMON_OPT_DOMAIN(N_("name of the inactive domain"), + VIR_CONNECT_LIST_DOMAINS_SHUTOFF), #ifndef WIN32 {.name = "console", .type = VSH_OT_BOOL, @@ -4098,7 +4099,7 @@ static const vshCmdInfo info_save[] = { }; static const vshCmdOptDef opts_save[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), VIRSH_COMMON_OPT_FILE(N_("where to save the data")), {.name = "bypass-cache", .type = VSH_OT_BOOL, @@ -4544,7 +4545,7 @@ static const vshCmdInfo info_managedsave[] = { }; static const vshCmdOptDef opts_managedsave[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "bypass-cache", .type = VSH_OT_BOOL, .help = N_("avoid file system cache when saving") @@ -4663,7 +4664,7 @@ static const vshCmdInfo info_managedsaveremove[] = { }; static const vshCmdOptDef opts_managedsaveremove[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = NULL} }; @@ -4718,7 +4719,7 @@ static const vshCmdInfo info_managed_save_edit[] = { }; static const vshCmdOptDef opts_managed_save_edit[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "running", .type = VSH_OT_BOOL, .help = N_("set domain to be running on start") @@ -4784,7 +4785,7 @@ static const vshCmdInfo info_managed_save_dumpxml[] = { }; static const vshCmdOptDef opts_managed_save_dumpxml[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "security-info", .type = VSH_OT_BOOL, .help = N_("include security sensitive information in XML dump") @@ -4832,7 +4833,7 @@ static const vshCmdInfo info_managed_save_define[] = { }; static const vshCmdOptDef opts_managed_save_define[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "xml", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -4904,7 +4905,7 @@ static const vshCmdInfo info_schedinfo[] = { }; static const vshCmdOptDef opts_schedinfo[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "weight", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ_OPT, @@ -5215,7 +5216,7 @@ static const vshCmdInfo info_dump[] = { }; static const vshCmdOptDef opts_dump[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), VIRSH_COMMON_OPT_FILE(N_("where to dump the core")), VIRSH_COMMON_OPT_LIVE(N_("perform a live core dump if supported")), {.name = "crash", @@ -5387,7 +5388,7 @@ static const vshCmdInfo info_screenshot[] = { }; static const vshCmdOptDef opts_screenshot[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "file", .type = VSH_OT_STRING, .help = N_("where to store the screenshot") @@ -5530,7 +5531,7 @@ static const vshCmdInfo info_setLifecycleAction[] = { }; static const vshCmdOptDef opts_setLifecycleAction[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "type", .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ, @@ -5626,7 +5627,7 @@ static const vshCmdInfo info_set_user_password[] = { }; static const vshCmdOptDef opts_set_user_password[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "user", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -5690,7 +5691,7 @@ static const vshCmdInfo info_resume[] = { }; static const vshCmdOptDef opts_resume[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_PAUSED), {.name = NULL} }; @@ -5729,7 +5730,7 @@ static const vshCmdInfo info_shutdown[] = { }; static const vshCmdOptDef opts_shutdown[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mode", .type = VSH_OT_STRING, .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt") @@ -5813,7 +5814,7 @@ static const vshCmdInfo info_reboot[] = { }; static const vshCmdOptDef opts_reboot[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mode", .type = VSH_OT_STRING, .help = N_("shutdown mode: acpi|agent|initctl|signal|paravirt") @@ -5892,7 +5893,7 @@ static const vshCmdInfo info_reset[] = { }; static const vshCmdOptDef opts_reset[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = NULL} }; @@ -5931,7 +5932,7 @@ static const vshCmdInfo info_domjobinfo[] = { }; static const vshCmdOptDef opts_domjobinfo[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "completed", .type = VSH_OT_BOOL, .help = N_("return statistics of a recently completed job") @@ -6275,7 +6276,7 @@ static const vshCmdInfo info_domjobabort[] = { }; static const vshCmdOptDef opts_domjobabort[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -6309,7 +6310,7 @@ static const vshCmdInfo info_vcpucount[] = { }; static const vshCmdOptDef opts_vcpucount[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "maximum", .type = VSH_OT_BOOL, .help = N_("get maximum count of vcpus") @@ -6506,7 +6507,7 @@ static const vshCmdInfo info_vcpuinfo[] = { }; static const vshCmdOptDef opts_vcpuinfo[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "pretty", .type = VSH_OT_BOOL, .help = N_("return human readable output") @@ -6755,7 +6756,7 @@ static const vshCmdInfo info_vcpupin[] = { }; static const vshCmdOptDef opts_vcpupin[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "vcpu", .type = VSH_OT_INT, .help = N_("vcpu number") @@ -6972,7 +6973,7 @@ static const vshCmdInfo info_emulatorpin[] = { }; static const vshCmdOptDef opts_emulatorpin[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "cpulist", .type = VSH_OT_STRING, .flags = VSH_OFLAG_EMPTY_OK, @@ -7076,7 +7077,7 @@ static const vshCmdInfo info_setvcpus[] = { }; static const vshCmdOptDef opts_setvcpus[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "count", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ, @@ -7174,7 +7175,7 @@ static const vshCmdInfo info_guestvcpus[] = { }; static const vshCmdOptDef opts_guestvcpus[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "cpulist", .type = VSH_OT_STRING, .help = N_("list of cpus to enable or disable") @@ -7259,7 +7260,7 @@ static const vshCmdInfo info_setvcpu[] = { }; static const vshCmdOptDef opts_setvcpu[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "vcpulist", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -7342,7 +7343,7 @@ static const vshCmdInfo info_domblkthreshold[] = { }; static const vshCmdOptDef opts_domblkthreshold[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "dev", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -7398,7 +7399,7 @@ static const vshCmdInfo info_iothreadinfo[] = { {.name = NULL} }; static const vshCmdOptDef opts_iothreadinfo[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), VIRSH_COMMON_OPT_DOMAIN_CONFIG, VIRSH_COMMON_OPT_DOMAIN_LIVE, VIRSH_COMMON_OPT_DOMAIN_CURRENT, @@ -7474,7 +7475,7 @@ static const vshCmdInfo info_iothreadpin[] = { }; static const vshCmdOptDef opts_iothreadpin[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "iothread", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ, @@ -7556,7 +7557,7 @@ static const vshCmdInfo info_iothreadadd[] = { }; static const vshCmdOptDef opts_iothreadadd[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "id", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ, @@ -7621,7 +7622,7 @@ static const vshCmdInfo info_iothreaddel[] = { }; static const vshCmdOptDef opts_iothreaddel[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "id", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ, @@ -7897,7 +7898,7 @@ static const vshCmdInfo info_cpu_stats[] = { }; static const vshCmdOptDef opts_cpu_stats[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "total", .type = VSH_OT_BOOL, .help = N_("Show total statistics only") @@ -8236,7 +8237,7 @@ static const vshCmdInfo info_destroy[] = { }; static const vshCmdOptDef opts_destroy[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "graceful", .type = VSH_OT_BOOL, .help = N_("terminate gracefully") @@ -8289,7 +8290,7 @@ static const vshCmdInfo info_desc[] = { }; static const vshCmdOptDef opts_desc[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), VIRSH_COMMON_OPT_LIVE(N_("modify/get running state")), VIRSH_COMMON_OPT_CONFIG(N_("modify/get persistent configuration")), VIRSH_COMMON_OPT_CURRENT(N_("modify/get current state configuration")), @@ -8454,7 +8455,7 @@ static const vshCmdInfo info_metadata[] = { }; static const vshCmdOptDef opts_metadata[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "uri", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -8600,7 +8601,7 @@ static const vshCmdInfo info_inject_nmi[] = { }; static const vshCmdOptDef opts_inject_nmi[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -8634,7 +8635,7 @@ static const vshCmdInfo info_send_key[] = { }; static const vshCmdOptDef opts_send_key[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "codeset", .type = VSH_OT_STRING, .flags = VSH_OFLAG_REQ_OPT, @@ -8730,7 +8731,7 @@ static const vshCmdInfo info_send_process_signal[] = { }; static const vshCmdOptDef opts_send_process_signal[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "pid", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -8835,7 +8836,7 @@ static const vshCmdInfo info_setmem[] = { }; static const vshCmdOptDef opts_setmem[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "kilobytes", .type = VSH_OT_ALIAS, .help = "size" @@ -8916,7 +8917,7 @@ static const vshCmdInfo info_setmaxmem[] = { }; static const vshCmdOptDef opts_setmaxmem[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "kilobytes", .type = VSH_OT_ALIAS, .help = "size" @@ -9004,7 +9005,7 @@ static const vshCmdInfo info_memtune[] = { }; static const vshCmdOptDef opts_memtune[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "hard-limit", .type = VSH_OT_INT, .help = N_("Max memory, as scaled integer (default KiB)") @@ -9181,7 +9182,7 @@ static const vshCmdInfo info_perf[] = { }; static const vshCmdOptDef opts_perf[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "enable", .type = VSH_OT_STRING, .help = N_("perf events which will be enabled") @@ -9315,7 +9316,7 @@ static const vshCmdInfo info_numatune[] = { }; static const vshCmdOptDef opts_numatune[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "mode", .type = VSH_OT_STRING, .help = N_("NUMA mode, one of strict, preferred and interleave \n" @@ -9450,7 +9451,7 @@ static const vshCmdInfo info_qemu_monitor_command[] = { }; static const vshCmdOptDef opts_qemu_monitor_command[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "hmp", .type = VSH_OT_BOOL, .help = N_("command is in human monitor protocol") @@ -9751,7 +9752,7 @@ static const vshCmdInfo info_qemu_agent_command[] = { }; static const vshCmdOptDef opts_qemu_agent_command[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "timeout", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ_OPT, @@ -9873,7 +9874,7 @@ static const vshCmdInfo info_lxc_enter_namespace[] = { }; static const vshCmdOptDef opts_lxc_enter_namespace[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "noseclabel", .type = VSH_OT_BOOL, .help = N_("Do not change process security label") @@ -10014,7 +10015,7 @@ static const vshCmdInfo info_dumpxml[] = { }; static const vshCmdOptDef opts_dumpxml[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "inactive", .type = VSH_OT_BOOL, .help = N_("show inactive defined XML") @@ -10223,7 +10224,7 @@ static const vshCmdInfo info_domname[] = { }; static const vshCmdOptDef opts_domname[] = { - VIRSH_COMMON_OPT_DOMAIN(N_("domain id or uuid")), + VIRSH_COMMON_OPT_DOMAIN(N_("domain id or uuid"), 0), {.name = NULL} }; @@ -10255,7 +10256,7 @@ static const vshCmdInfo info_domrename[] = { }; static const vshCmdOptDef opts_domrename[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_INACTIVE), {.name = "new-name", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -10302,7 +10303,8 @@ static const vshCmdInfo info_domid[] = { }; static const vshCmdOptDef opts_domid[] = { - VIRSH_COMMON_OPT_DOMAIN(N_("domain name or uuid")), + VIRSH_COMMON_OPT_DOMAIN(N_("domain name or uuid"), + VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -10339,7 +10341,7 @@ static const vshCmdInfo info_domuuid[] = { }; static const vshCmdOptDef opts_domuuid[] = { - VIRSH_COMMON_OPT_DOMAIN(N_("domain id or name")), + VIRSH_COMMON_OPT_DOMAIN(N_("domain id or name"), 0), {.name = NULL} }; @@ -10376,7 +10378,7 @@ static const vshCmdInfo info_migrate[] = { }; static const vshCmdOptDef opts_migrate[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "desturi", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -10974,7 +10976,7 @@ static const vshCmdInfo info_migrate_setmaxdowntime[] = { }; static const vshCmdOptDef opts_migrate_setmaxdowntime[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "downtime", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ, @@ -11025,7 +11027,7 @@ static const vshCmdInfo info_migrate_getmaxdowntime[] = { }; static const vshCmdOptDef opts_migrate_getmaxdowntime[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -11066,7 +11068,7 @@ static const vshCmdInfo info_migrate_compcache[] = { }; static const vshCmdOptDef opts_migrate_compcache[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "size", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ_OPT, @@ -11123,7 +11125,7 @@ static const vshCmdInfo info_migrate_setspeed[] = { }; static const vshCmdOptDef opts_migrate_setspeed[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "bandwidth", .type = VSH_OT_INT, .flags = VSH_OFLAG_REQ, @@ -11169,7 +11171,7 @@ static const vshCmdInfo info_migrate_getspeed[] = { }; static const vshCmdOptDef opts_migrate_getspeed[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -11252,7 +11254,7 @@ static const vshCmdInfo info_domdisplay[] = { }; static const vshCmdOptDef opts_domdisplay[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "include-password", .type = VSH_OT_BOOL, .help = N_("includes the password into the connection URI if available") @@ -11533,7 +11535,7 @@ static const vshCmdInfo info_vncdisplay[] = { }; static const vshCmdOptDef opts_vncdisplay[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -11609,7 +11611,7 @@ static const vshCmdInfo info_ttyconsole[] = { }; static const vshCmdOptDef opts_ttyconsole[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; @@ -11651,7 +11653,7 @@ static const vshCmdInfo info_domhostname[] = { }; static const vshCmdOptDef opts_domhostname[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = NULL} }; @@ -11810,7 +11812,7 @@ static const vshCmdInfo info_detach_device[] = { }; static const vshCmdOptDef opts_detach_device[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), VIRSH_COMMON_OPT_FILE(N_("XML file")), VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, VIRSH_COMMON_OPT_DOMAIN_CONFIG, @@ -11891,7 +11893,7 @@ static const vshCmdInfo info_update_device[] = { }; static const vshCmdOptDef opts_update_device[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), VIRSH_COMMON_OPT_FILE(N_("XML file")), VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, VIRSH_COMMON_OPT_DOMAIN_CONFIG, @@ -11973,7 +11975,7 @@ static const vshCmdInfo info_detach_interface[] = { }; static const vshCmdOptDef opts_detach_interface[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "type", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -12416,7 +12418,7 @@ static const vshCmdInfo info_detach_disk[] = { }; static const vshCmdOptDef opts_detach_disk[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "target", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -12516,7 +12518,7 @@ static const vshCmdInfo info_edit[] = { }; static const vshCmdOptDef opts_edit[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "skip-validate", .type = VSH_OT_BOOL, .help = N_("skip validation of the XML against the schema") @@ -13475,7 +13477,7 @@ static const vshCmdInfo info_change_media[] = { }; static const vshCmdOptDef opts_change_media[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "path", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -13636,7 +13638,7 @@ static const vshCmdInfo info_domfstrim[] = { }; static const vshCmdOptDef opts_domfstrim[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "minimum", .type = VSH_OT_INT, .help = N_("Just a hint to ignore contiguous " @@ -13689,7 +13691,7 @@ static const vshCmdInfo info_domfsfreeze[] = { }; static const vshCmdOptDef opts_domfsfreeze[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mountpoint", .type = VSH_OT_ARGV, .help = N_("mountpoint path to be frozen") @@ -13742,7 +13744,7 @@ static const vshCmdInfo info_domfsthaw[] = { }; static const vshCmdOptDef opts_domfsthaw[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = "mountpoint", .type = VSH_OT_ARGV, .help = N_("mountpoint path to be thawed") @@ -13795,7 +13797,7 @@ static const vshCmdInfo info_domfsinfo[] = { }; static const vshCmdOptDef opts_domfsinfo[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE), {.name = NULL} }; diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index cd89a414a..c44a36f98 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -42,8 +42,8 @@ #include "virxml.h" #include "conf/snapshot_conf.h" -#define VIRSH_COMMON_OPT_DOMAIN_FULL \ - VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid")) +#define VIRSH_COMMON_OPT_DOMAIN_FULL(cflags) \ + VIRSH_COMMON_OPT_DOMAIN(N_("domain name, id or uuid"), cflags) /* Helper for snapshot-create and snapshot-create-as */ static bool @@ -125,7 +125,7 @@ static const vshCmdInfo info_snapshot_create[] = { }; static const vshCmdOptDef opts_snapshot_create[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "xmlfile", .type = VSH_OT_STRING, .help = N_("domain snapshot XML") @@ -319,7 +319,7 @@ static const vshCmdInfo info_snapshot_create_as[] = { }; static const vshCmdOptDef opts_snapshot_create_as[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "name", .type = VSH_OT_STRING, .help = N_("name of snapshot") @@ -508,7 +508,7 @@ static const vshCmdInfo info_snapshot_edit[] = { }; static const vshCmdOptDef opts_snapshot_edit[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, .help = N_("snapshot name") @@ -620,7 +620,7 @@ static const vshCmdInfo info_snapshot_current[] = { }; static const vshCmdOptDef opts_snapshot_current[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "name", .type = VSH_OT_BOOL, .help = N_("list the name, rather than the full xml") @@ -851,7 +851,7 @@ static const vshCmdInfo info_snapshot_info[] = { }; static const vshCmdOptDef opts_snapshot_info[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, .help = N_("snapshot name") @@ -1401,7 +1401,7 @@ static const vshCmdInfo info_snapshot_list[] = { }; static const vshCmdOptDef opts_snapshot_list[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "parent", .type = VSH_OT_BOOL, .help = N_("add a column showing parent snapshot") @@ -1657,7 +1657,7 @@ static const vshCmdInfo info_snapshot_dumpxml[] = { }; static const vshCmdOptDef opts_snapshot_dumpxml[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, @@ -1720,7 +1720,7 @@ static const vshCmdInfo info_snapshot_parent[] = { }; static const vshCmdOptDef opts_snapshot_parent[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, .help = N_("find parent of snapshot name") @@ -1779,7 +1779,7 @@ static const vshCmdInfo info_snapshot_revert[] = { }; static const vshCmdOptDef opts_snapshot_revert[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, .help = N_("snapshot name") @@ -1863,7 +1863,7 @@ static const vshCmdInfo info_snapshot_delete[] = { }; static const vshCmdOptDef opts_snapshot_delete[] = { - VIRSH_COMMON_OPT_DOMAIN_FULL, + VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "snapshotname", .type = VSH_OT_STRING, .help = N_("snapshot name") diff --git a/tools/virsh.h b/tools/virsh.h index e03b597ef..856513a48 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -38,6 +38,7 @@ # include "virthread.h" # include "virpolkit.h" # include "vsh.h" +# include "virsh-completer.h" # define VIRSH_PROMPT_RW "virsh # " # define VIRSH_PROMPT_RO "virsh > " @@ -70,11 +71,13 @@ .help = _helpstr \ } -# define VIRSH_COMMON_OPT_DOMAIN(_helpstr) \ +# define VIRSH_COMMON_OPT_DOMAIN(_helpstr, cflags) \ {.name = "domain", \ .type = VSH_OT_DATA, \ .flags = VSH_OFLAG_REQ, \ - .help = _helpstr \ + .help = _helpstr, \ + .completer = virshDomainNameCompleter, \ + .completer_flags = cflags, \ } # define VIRSH_COMMON_OPT_CONFIG(_helpstr) \ -- 2.13.6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-completer.c | 60 ++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-completer.h | 8 ++++++ tools/virsh-domain-monitor.c | 3 +++ tools/virsh-domain.c | 4 +++ 4 files changed, 75 insertions(+) diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c index 4e32b882b..25c3aaa0d 100644 --- a/tools/virsh-completer.c +++ b/tools/virsh-completer.c @@ -29,6 +29,7 @@ #include "internal.h" #include "viralloc.h" #include "virstring.h" +#include "virxml.h" char ** @@ -88,3 +89,62 @@ virshDomainNameCompleter(vshControl *ctl, VIR_FREE(ret); return NULL; } + + +char ** +virshDomainInterfaceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + virshControlPtr priv = ctl->privData; + xmlDocPtr xmldoc = NULL; + xmlXPathContextPtr ctxt = NULL; + int ninterfaces; + xmlNodePtr *interfaces = NULL; + size_t i; + unsigned int domainXMLFlags = 0; + char **ret = NULL; + + virCheckFlags(VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC, NULL); + + if (!priv->conn || virConnectIsAlive(priv->conn) <= 0) + return NULL; + + if (vshCommandOptBool(cmd, "config")) + domainXMLFlags = VIR_DOMAIN_XML_INACTIVE; + + if (virshDomainGetXML(ctl, cmd, domainXMLFlags, &xmldoc, &ctxt) < 0) + goto error; + + ninterfaces = virXPathNodeSet("./devices/interface", ctxt, &interfaces); + if (ninterfaces < 0) + goto error; + + if (VIR_ALLOC_N(ret, ninterfaces + 1) < 0) + goto error; + + for (i = 0; i < ninterfaces; i++) { + ctxt->node = interfaces[i]; + + if (!(flags & VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC) && + (ret[i] = virXPathString("string(./target/@dev)", ctxt))) + continue; + + /* In case we are dealing with inactive domain XML there's no + * <target dev=''/>. Offer MAC addresses then. */ + if (!(ret[i] = virXPathString("string(./mac/@address)", ctxt))) + goto error; + } + + VIR_FREE(interfaces); + xmlFreeDoc(xmldoc); + xmlXPathFreeContext(ctxt); + return ret; + + error: + VIR_FREE(interfaces); + xmlFreeDoc(xmldoc); + xmlXPathFreeContext(ctxt); + virStringListFree(ret); + return NULL; +} diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h index 288e17909..680cd12ff 100644 --- a/tools/virsh-completer.h +++ b/tools/virsh-completer.h @@ -30,4 +30,12 @@ char ** virshDomainNameCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); +enum { + VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC = 1 << 1, /* Return just MACs */ +}; + +char ** virshDomainInterfaceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); + #endif diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index d0edb177d..dd89f0758 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -659,6 +659,7 @@ static const vshCmdOptDef opts_domif_getlink[] = { {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshDomainInterfaceCompleter, .help = N_("interface device (MAC Address)") }, {.name = "persistent", @@ -995,6 +996,7 @@ static const vshCmdOptDef opts_domifstat[] = { {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshDomainInterfaceCompleter, .help = N_("interface device") }, {.name = NULL} @@ -2149,6 +2151,7 @@ static const vshCmdOptDef opts_domifaddr[] = { {.name = "interface", .type = VSH_OT_STRING, .flags = VSH_OFLAG_NONE, + .completer = virshDomainInterfaceCompleter, .help = N_("network interface name")}, {.name = "full", .type = VSH_OT_BOOL, diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 101a5f647..f977832aa 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -2977,6 +2977,7 @@ static const vshCmdOptDef opts_domif_setlink[] = { {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshDomainInterfaceCompleter, .help = N_("interface device (MAC Address)") }, {.name = "state", @@ -3147,6 +3148,7 @@ static const vshCmdOptDef opts_domiftune[] = { {.name = "interface", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = virshDomainInterfaceCompleter, .help = N_("interface device (MAC Address)") }, {.name = "inbound", @@ -11983,6 +11985,8 @@ static const vshCmdOptDef opts_detach_interface[] = { }, {.name = "mac", .type = VSH_OT_STRING, + .completer = virshDomainInterfaceCompleter, + .completer_flags = VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC, .help = N_("MAC address") }, VIRSH_COMMON_OPT_DOMAIN_PERSISTENT, -- 2.13.6
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/Makefile.am | 9 ++++++ tools/virt-admin-completer.c | 76 ++++++++++++++++++++++++++++++++++++++++++++ tools/virt-admin-completer.h | 33 +++++++++++++++++++ tools/virt-admin.c | 8 +++++ 4 files changed, 126 insertions(+) create mode 100644 tools/virt-admin-completer.c create mode 100644 tools/virt-admin-completer.h diff --git a/tools/Makefile.am b/tools/Makefile.am index 8eddc7bbe..a1652d459 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -261,6 +261,15 @@ virt_admin_SOURCES = \ virt-admin.c virt-admin.h \ $(NULL) +VIRT_ADMIN_COMPLETER = \ + virt-admin-completer.c virt-admin-completer.h + +if WITH_READLINE +virt_admin_SOURCES += $(VIRT_ADMIN_COMPLETER) +else ! WITH_READLINE +EXTRA_DIST += $(VIRT_ADMIN_COMPLETER) +endif ! WITH_READLINE + virt_admin_LDFLAGS = \ $(AM_LDFLAGS) \ $(COVERAGE_LDFLAGS) \ diff --git a/tools/virt-admin-completer.c b/tools/virt-admin-completer.c new file mode 100644 index 000000000..2cd471f32 --- /dev/null +++ b/tools/virt-admin-completer.c @@ -0,0 +1,76 @@ +/* + * virt-admin-completer.c: virt-admin completer callbacks + * + * Copyright (C) 2017 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Michal Privoznik <mprivozn@redhat.com> + * + */ + +#include <config.h> + +#include "virt-admin-completer.h" +#include "internal.h" +#include "virt-admin.h" +#include "viralloc.h" +#include "virstring.h" + + +char ** +vshAdmServerCompleter(vshControl *ctl, + const vshCmd *cmd ATTRIBUTE_UNUSED, + unsigned int flags) +{ + vshAdmControlPtr priv = ctl->privData; + virAdmServerPtr *srvs = NULL; + int nsrvs = 0; + size_t i = 0; + char **ret = NULL; + + virCheckFlags(0, NULL); + + if (!priv->conn || virAdmConnectIsAlive(priv->conn) <= 0) + return NULL; + + /* Obtain a list of available servers on the daemon */ + if ((nsrvs = virAdmConnectListServers(priv->conn, &srvs, 0)) < 0) + return NULL; + + if (VIR_ALLOC_N(ret, nsrvs + 1) < 0) + goto error; + + for (i = 0; i < nsrvs; i++) { + const char *name = virAdmServerGetName(srvs[i]); + + if (VIR_STRDUP(ret[i], name) < 0) + goto error; + + virAdmServerFree(srvs[i]); + } + VIR_FREE(srvs); + + return ret; + + error: + for (; i < nsrvs; i++) + virAdmServerFree(srvs[i]); + VIR_FREE(srvs); + for (i = 0; i < nsrvs; i++) + VIR_FREE(ret[i]); + VIR_FREE(ret); + return ret; +} diff --git a/tools/virt-admin-completer.h b/tools/virt-admin-completer.h new file mode 100644 index 000000000..7507b95c1 --- /dev/null +++ b/tools/virt-admin-completer.h @@ -0,0 +1,33 @@ +/* + * virt-admin-completer.h: virt-admin completer callbacks + * + * Copyright (C) 2017 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 + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Michal Privoznik <mprivozn@redhat.com> + * + */ + +#ifndef VIRT_ADMIN_COMPLETER +# define VIRT_ADMIN_COMPLETER + +# include "vsh.h" + +char ** +vshAdmServerCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); +#endif diff --git a/tools/virt-admin.c b/tools/virt-admin.c index c24ed95c0..41b945e49 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -39,6 +39,7 @@ #include "virthread.h" #include "virgettext.h" #include "virtime.h" +#include "virt-admin-completer.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -432,6 +433,7 @@ static const vshCmdOptDef opts_srv_threadpool_info[] = { {.name = "server", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = vshAdmServerCompleter, .help = N_("Server to retrieve threadpool attributes from."), }, {.name = NULL} @@ -493,6 +495,7 @@ static const vshCmdOptDef opts_srv_threadpool_set[] = { {.name = "server", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = vshAdmServerCompleter, .help = N_("Server to alter threadpool attributes on."), }, {.name = "min-workers", @@ -599,6 +602,7 @@ static const vshCmdOptDef opts_srv_clients_list[] = { {.name = "server", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = vshAdmServerCompleter, .help = N_("server which to list connected clients from"), }, {.name = NULL} @@ -680,6 +684,7 @@ static const vshCmdOptDef opts_client_info[] = { {.name = "server", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = vshAdmServerCompleter, .help = N_("server to which <client> is connected to"), }, {.name = "client", @@ -767,6 +772,7 @@ static const vshCmdOptDef opts_client_disconnect[] = { {.name = "server", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = vshAdmServerCompleter, .help = N_("server which the client is currently connected to"), }, {.name = "client", @@ -832,6 +838,7 @@ static const vshCmdOptDef opts_srv_clients_info[] = { {.name = "server", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = vshAdmServerCompleter, .help = N_("Server to retrieve the client limits from."), }, {.name = NULL} @@ -891,6 +898,7 @@ static const vshCmdOptDef opts_srv_clients_set[] = { {.name = "server", .type = VSH_OT_DATA, .flags = VSH_OFLAG_REQ, + .completer = vshAdmServerCompleter, .help = N_("Server to alter the client-related configuration limits on."), }, {.name = "max-clients", -- 2.13.6
On Tue, Nov 07, 2017 at 01:22:48PM +0100, Michal Privoznik wrote:
After initial RFC [1] I had some time to work on this and here is the result.
What's implemented? =================== Auto completion for both interactive and non-interactive virsh/virt-admin.
Known limitations ================= Currently, just options completers work. I mean, to bring up list of domains you have to:
virsh # start --domain <TAB><TAB>
Doing bare:
virsh # start <TAB><TAB>
brings up list of --options. I am working on this meanwhile. But one can argue that having to use --option is not that much of a problem since we have good completion now.
The other known limitation - and actually room for others to join and help - is missing completers. I mean, the last 3 patches give an example how to do that. But that is still very few. We need more completers.
How does this work? =================== The basic idea is simple, when defining opts for command two new struct members can be set:
{.name = "mac", .type = VSH_OT_STRING, + .completer = virshDomainInterfaceCompleter, + .completer_flags = VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC, .help = N_("MAC address") },
@completer is the callback that is used when user wants to bring up list of possible strings. @completer_flags can then be used to refine return value of @completer. In this specific example, virshDomainInterfaceCompleter() brings up list of interfaces for given domain. It tries to return /interface/target/@dev but if not found (e.g. because domain is not running), /interface/mac/@address is returned otherwise. However, in one specific case we are interested only in MAC addresses (regardless of domain state) and thus VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC flag is specified which alters @completer behaviour.
For non-interactive virsh/virt-admin there's tools/bash-completion/vsh script which does no more than calling:
$1 complete $USER_INPUT
(where $1 is name of the binary user intents to run).
How to test this? =================
Interactive completion should work out of the box. Just make sure you're connected. Completers don't connect! You certainly don't want ssh's 'Password:' prompt show on <TAB><TAB>, do you? Non-interactive completers do connect, but in order to avoid any password prompts, /dev/stdin is closed before anything else is done. In order to test it, just:
libvirt.git $ source tools/bash-completion/vsh
Now, bash completion should work:
libvirt.git $ ./tools/virsh -c qemu:///system start --domain <TAB><TAB>
So if I do this, it works, but if I copy it where it is supposed to be installed it doesn't for some reason. I haven't dug into that, I don't even know where to start, probably. Also it is very possible that it's my fault. I also found one more thing, but that's only easy to fix in few cases. If you type 'virsh start --domain fedora <TAB><TAB>' then one of the strings it offers is '--domain'. It'd be nice to have that filtered out. Not needed for ACK, though. Just an idea for you. Apart from that it looks sane, so ACK to the general concept, I would just change 2/11 as written in the reply, drop (or completely replace) 3/11 and instead do the '-qq' thing. And of course adjust other code depending on that. I'd ACK the rest, I would just like to have the middle-of-the-command completion working too so it doesn't confuse people.
participants (3)
-
Eric Blake -
Martin Kletzander -
Michal Privoznik