7:36 a.m.
Hi, this patch series is a prototype for my GSoC project (Michal Privoznik
is my mentor).
I'm working on virsh auto-completion, trying to make it more "intelligent".
At this stage, prototype is capable of command and option completion. Three
completer functions are currently implemented so you can test it. If it turns
out that this prototype is good enough, I will implement more completer functions.
---
v3:
* vshReconnect() is now called only when we reach the point that completion
is being attempted on commands that needs connection
* moved all .completer intializations into the 4/6, 5/6 and 6/6
v2:
https://www.redhat.com/archives/libvir-list/2013-August/msg00992.html
v1:
https://www.redhat.com/archives/libvir-list/2013-August/msg00371.html
Tomas Meszaros (6):
virsh: C99 style for info_domfstrim and opts_lxc_enter_namespace
virsh: Add vshCmdCompleter and vshOptCompleter
virsh: Improve readline generators and readline completion
virsh: Add vshDomainCompleter
virsh: Add vshSuspendTargetCompleter
virsh: Add vshRebootShutdownModeCompleter
.gnulib | 2 +-
tools/virsh-domain-monitor.c | 32 ++-
tools/virsh-domain.c | 248 +++++++++++++++++-----
tools/virsh-snapshot.c | 45 +++-
tools/virsh.c | 480 +++++++++++++++++++++++++++++++++++++++++--
tools/virsh.h | 11 +
6 files changed, 725 insertions(+), 93 deletions(-)
--
1.8.3.1
7:36 a.m.
New subject: [libvirt] [PATCHv3 1/6] virsh: C99 style for info_domfstrim and opts_lxc_enter_namespace
Change info_domfstrim and opts_lxc_enter_namespace initialization style
to C99.
---
tools/virsh-domain.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b29f934..5d4913d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -7903,10 +7903,21 @@ static const vshCmdInfo info_lxc_enter_namespace[] = {
};
static const vshCmdOptDef opts_lxc_enter_namespace[] = {
- {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or
uuid")},
- {"noseclabel", VSH_OT_BOOL, 0, N_("Do not change process security
label")},
- {"cmd", VSH_OT_ARGV, VSH_OFLAG_REQ, N_("namespace")},
- {NULL, 0, 0, NULL}
+ {.name = "domain",
+ .type = VSH_OT_DATA,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("domain name, id or uuid")
+ },
+ {.name = "noseclabel",
+ .type = VSH_OT_BOOL,
+ .help = N_("Do not change process security label")
+ },
+ {.name = "cmd",
+ .type = VSH_OT_ARGV,
+ .flags = VSH_OFLAG_REQ,
+ .help = N_("namespace")
+ },
+ {.name = NULL}
};
static bool
@@ -10290,7 +10301,7 @@ static const vshCmdOptDef opts_domfstrim[] = {
.type = VSH_OT_DATA,
.help = N_("which mount point to trim")
},
- {NULL, 0, 0, NULL}
+ {.name = NULL}
};
static bool
cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
--
1.8.3.1
11:26 a.m.
New subject: [libvirt] [PATCHv3 1/6] virsh: C99 style for info_domfstrim and opts_lxc_enter_namespace
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
Change info_domfstrim and opts_lxc_enter_namespace initialization
style
to C99.
---
tools/virsh-domain.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
ACK and pushed. Congrats on your first libvirt patch.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:36 a.m.
New subject: [libvirt] [PATCHv3 2/6] virsh: Add vshCmdCompleter and vshOptCompleter
completer and completer_flags added to the _vshCmdDef and _vshCmdOptDef
structures so it will be possible for completion generators to
conveniently call completer functions with desired flags.
---
tools/virsh.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/tools/virsh.h b/tools/virsh.h
index 466ca2d..064acde 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -147,6 +147,9 @@ typedef struct _vshCmdOptDef vshCmdOptDef;
typedef struct _vshControl vshControl;
typedef struct _vshCtrlData vshCtrlData;
+typedef char **(*vshCmdCompleter)(unsigned int flags);
+typedef char **(*vshOptCompleter)(unsigned int flags);
+
/*
* vshCmdInfo -- name/value pair for information about command
*
@@ -168,6 +171,8 @@ struct _vshCmdOptDef {
unsigned int flags; /* flags */
const char *help; /* non-NULL help string; or for VSH_OT_ALIAS
* the name of a later public option */
+ vshOptCompleter completer; /* option completer */
+ unsigned int completer_flags; /* option completer flags */
};
/*
@@ -199,6 +204,8 @@ struct _vshCmdDef {
const vshCmdOptDef *opts; /* definition of command options */
const vshCmdInfo *info; /* details about command */
unsigned int flags; /* bitwise OR of VSH_CMD_FLAG */
+ vshCmdCompleter completer; /* command completer */
+ unsigned int completer_flags; /* command completer flags */
};
/*
--
1.8.3.1
12:47 p.m.
New subject: [libvirt] [PATCHv3 2/6] virsh: Add vshCmdCompleter and vshOptCompleter
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
completer and completer_flags added to the _vshCmdDef and
_vshCmdOptDef
structures so it will be possible for completion generators to
conveniently call completer functions with desired flags.
---
tools/virsh.h | 7 +++++++
1 file changed, 7 insertions(+)
In isolation, this patch looks reasonable, but I want to see how it gets
used before acking it.
diff --git a/tools/virsh.h b/tools/virsh.h
index 466ca2d..064acde 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -147,6 +147,9 @@ typedef struct _vshCmdOptDef vshCmdOptDef;
typedef struct _vshControl vshControl;
typedef struct _vshCtrlData vshCtrlData;
+typedef char **(*vshCmdCompleter)(unsigned int flags);
+typedef char **(*vshOptCompleter)(unsigned int flags);
Do we need two typedefs, or is (*vshCompleter) a sufficient reusable name?
+
/*
* vshCmdInfo -- name/value pair for information about command
*
@@ -168,6 +171,8 @@ struct _vshCmdOptDef {
unsigned int flags; /* flags */
const char *help; /* non-NULL help string; or for VSH_OT_ALIAS
* the name of a later public option */
+ vshOptCompleter completer; /* option completer */
+ unsigned int completer_flags; /* option completer flags */
Per-option completions make sense. For example, 'virsh vol-key --pool
<TAB>' wants to use a pool completer, while 'virsh vol-key --vol
<TAB>'
wants to use a volume completer; furthermore, 'virsh vol-key <TAB>'
should be the combination of the option completer (showing --vol and
--pool) AND the volume completer filtered to names not starting with '-'
(since virsh has the semantics that arguments are positional, where the
option '--vol' is implied if the argument that appears in that position
does not resemble an option).
};
/*
@@ -199,6 +204,8 @@ struct _vshCmdDef {
const vshCmdOptDef *opts; /* definition of command options */
const vshCmdInfo *info; /* details about command */
unsigned int flags; /* bitwise OR of VSH_CMD_FLAG */
+ vshCmdCompleter completer; /* command completer */
+ unsigned int completer_flags; /* command completer flags */
};
I'm not so sure about per-command completers, though. What can a
command complete differently than per-option completers would already
provide? Maybe this argues that you need more rationale in the commit
message about how this will be used. Or maybe I'll figure it out by the
time I get to the end of your series.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:42 a.m.
New subject: [libvirt] [PATCHv3 2/6] virsh: Add vshCmdCompleter and vshOptCompleter
On 26/08/13 at 11:47am, Eric Blake wrote:
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
> completer and completer_flags added to the _vshCmdDef and _vshCmdOptDef
> structures so it will be possible for completion generators to
> conveniently call completer functions with desired flags.
> ---
> tools/virsh.h | 7 +++++++
> 1 file changed, 7 insertions(+)
In isolation, this patch looks reasonable, but I want to see how it gets
used before acking it.
>
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 466ca2d..064acde 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -147,6 +147,9 @@ typedef struct _vshCmdOptDef vshCmdOptDef;
> typedef struct _vshControl vshControl;
> typedef struct _vshCtrlData vshCtrlData;
>
> +typedef char **(*vshCmdCompleter)(unsigned int flags);
> +typedef char **(*vshOptCompleter)(unsigned int flags);
Do we need two typedefs, or is (*vshCompleter) a sufficient reusable name?
You are right, we don't need two typedefs. I will merge them into
(*vshCompleter) as you are suggesting.
> +
> /*
> * vshCmdInfo -- name/value pair for information about command
> *
> @@ -168,6 +171,8 @@ struct _vshCmdOptDef {
> unsigned int flags; /* flags */
> const char *help; /* non-NULL help string; or for VSH_OT_ALIAS
> * the name of a later public option */
> + vshOptCompleter completer; /* option completer */
> + unsigned int completer_flags; /* option completer flags */
Per-option completions make sense. For example, 'virsh vol-key --pool
<TAB>' wants to use a pool completer, while 'virsh vol-key --vol
<TAB>'
wants to use a volume completer; furthermore, 'virsh vol-key <TAB>'
should be the combination of the option completer (showing --vol and
--pool) AND the volume completer filtered to names not starting with '-'
(since virsh has the semantics that arguments are positional, where the
option '--vol' is implied if the argument that appears in that position
does not resemble an option).
So If I get it right, you are suggesting that it should work like this:
virsh # vol-key <TAB>
vol1 vol2 pool1 pool2
as you said, combination of --vol and --pool completers.
I was initially thinking that completion should work like this:
virsh # vol-key <TAB>
vol1 vol2
It is completing <vol> first becasue <vol> is only mandatory argument
for this command.
Only if someone type:
virsh # vol-key --pool <TAB>
pool1 pool2
then call --pool completer.
> };
>
> /*
> @@ -199,6 +204,8 @@ struct _vshCmdDef {
> const vshCmdOptDef *opts; /* definition of command options */
> const vshCmdInfo *info; /* details about command */
> unsigned int flags; /* bitwise OR of VSH_CMD_FLAG */
> + vshCmdCompleter completer; /* command completer */
> + unsigned int completer_flags; /* command completer flags */
> };
I'm not so sure about per-command completers, though. What can a
command complete differently than per-option completers would already
provide? Maybe this argues that you need more rationale in the commit
message about how this will be used. Or maybe I'll figure it out by the
time I get to the end of your series.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
--
Tomas Meszaros
7:36 a.m.
New subject: [libvirt] [PATCHv3 3/6] virsh: Improve readline generators and readline completion
New completion generators responsible for advances command
and command options completions.
vshReadlineCommandCompletionGenerator - generator for advanced
command completions. This function will call some vshCmdCompleter
function (e.g. vshDomainCompleter), which will return relevant
data used for autocompletion (e.g. domain names).
vshReadlineOptionsCompletionGenerator - almost the same as the
vshReadlineCommandCompletionGenerator, but this one completes
cmd options.
vshReadlineCompletion() has become much more complex because we
now have more generator functions and therefore more states to
choose from.
---
v2
* vshMalloc is now used in vshDetermineCommandName
* vshStrdup instead of vshMalloc + snprintf
* joined if's in vshReadlineOptionsCompletionGenerator
v3
* fixed typo
* removed useless if's
.gnulib | 2 +-
tools/virsh.c | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 365 insertions(+), 23 deletions(-)
diff --git a/.gnulib b/.gnulib
index 0ba0877..644c404 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit 0ba087759d2797c8f7d3c34bef6268ba3fd212cb
+Subproject commit 644c40496cf7d5a705a73c9dd32b035fcecc2ab1
diff --git a/tools/virsh.c b/tools/virsh.c
index 2ea44a6..5be8e7e 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2515,6 +2515,25 @@ vshCloseLogFile(vshControl *ctl)
* -----------------
*/
+static const vshCmdDef *
+vshDetermineCommandName(void)
+{
+ const vshCmdDef *cmd = NULL;
+ char *p;
+ char *cmdname;
+
+ if (!(p = strchr(rl_line_buffer, ' ')))
+ return NULL;
+
+ cmdname = vshMalloc(NULL, (p - rl_line_buffer) + 1);
+ memcpy(cmdname, rl_line_buffer, p - rl_line_buffer);
+
+ cmd = vshCmddefSearch(cmdname);
+ VIR_FREE(cmdname);
+
+ return cmd;
+}
+
/*
* Generator function for command completion. STATE lets us
* know whether to start from scratch; without any state
@@ -2562,25 +2581,14 @@ vshReadlineCommandGenerator(const char *text, int state)
static char *
vshReadlineOptionsGenerator(const char *text, int state)
{
- static int list_index, len;
static const vshCmdDef *cmd = NULL;
+ static int list_index, len;
const char *name;
if (!state) {
- /* determine command name */
- char *p;
- char *cmdname;
-
- if (!(p = strchr(rl_line_buffer, ' ')))
- return NULL;
-
- cmdname = vshCalloc(NULL, (p - rl_line_buffer) + 1, 1);
- memcpy(cmdname, rl_line_buffer, p - rl_line_buffer);
-
- cmd = vshCmddefSearch(cmdname);
+ cmd = vshDetermineCommandName();
list_index = 0;
len = strlen(text);
- VIR_FREE(cmdname);
}
if (!cmd)
@@ -2612,22 +2620,356 @@ vshReadlineOptionsGenerator(const char *text, int state)
return NULL;
}
+/*
+ * Generator function for command completion, but unlike
+ * the vshReadlineCommandGenerator which completes command name, this function
+ * provides more advanced completion for commands by calling specific command
+ * completers (e.g. vshDomainCompleter).
+ */
+static char *
+vshReadlineCommandCompletionGenerator(const char *text, int state)
+{
+ static const vshCmdDef *cmd = NULL;
+ static int list_index, len;
+ char **completed_names = NULL;
+ char *name;
+
+ if (!state) {
+ cmd = vshDetermineCommandName();
+ list_index = 0;
+ len = strlen(text);
+ }
+
+ if (!cmd)
+ return NULL;
+
+ if (!cmd->completer)
+ return NULL;
+
+ completed_names = cmd->completer(cmd->completer_flags);
+
+ if (!completed_names)
+ return NULL;
+
+ while ((name = completed_names[list_index])) {
+ char *res;
+ list_index++;
+
+ if (STRNEQLEN(name, text, len))
+ /* Skip irrelevant names */
+ continue;
+
+ res = vshStrdup(NULL, name);
+ VIR_FREE(name);
+ return res;
+ }
+ VIR_FREE(completed_names);
+
+ return NULL;
+}
+
+/*
+ * Generator function for command option completion. Provides advances
+ * completion for command options.
+ */
+static char *
+vshReadlineOptionsCompletionGenerator(const char *text ATTRIBUTE_UNUSED,
+ int state ATTRIBUTE_UNUSED)
+{
+ static const vshCmdDef *cmd = NULL;
+ static const vshCmdOptDef *opt = NULL;
+ static int list_index, len;
+ unsigned long int opt_index = 0;
+ size_t i;
+ char **completed_names = NULL;
+ char *name;
+ char *ptr = NULL;
+
+ if (!state) {
+ cmd = vshDetermineCommandName();
+ list_index = 0;
+ len = strlen(text);
+ }
+
+ if (!cmd)
+ return NULL;
+
+ if (!cmd->opts)
+ return NULL;
+
+ for (i = 0; cmd->opts[i].name; i++) {
+ if ((ptr = strstr(rl_line_buffer, cmd->opts[i].name)) &&
+ (opt_index < (ptr - rl_line_buffer))) {
+ opt_index = ptr - rl_line_buffer;
+ opt = &cmd->opts[i];
+ }
+ }
+
+ if (!opt)
+ return NULL;
+
+ if (!opt->completer)
+ return NULL;
+
+ completed_names = opt->completer(opt->completer_flags);
+
+ if (!completed_names)
+ return NULL;
+
+ while ((name = completed_names[list_index])) {
+ char *res;
+ list_index++;
+
+ if (STRNEQLEN(name, text, len))
+ /* Skip irrelevant names */
+ continue;
+
+ res = vshStrdup(NULL, name);
+ VIR_FREE(name);
+ return res;
+ }
+ VIR_FREE(completed_names);
+
+ return NULL;
+}
+
+/*
+ * Returns current valid command name present in the rl_line_buffer.
+ */
+static char *
+vshCurrentCmd(void)
+{
+ const char *name;
+ const vshCmdGrp *grp;
+ const vshCmdDef *cmds;
+ size_t grp_list_index, cmd_list_index;
+ char *found_cmd = NULL;
+ char *rl_copy = NULL;
+ char *pch;
+
+ grp_list_index = 0;
+ cmd_list_index = 0;
+ grp = cmdGroups;
+
+ while (grp[grp_list_index].name) {
+ cmds = grp[grp_list_index].commands;
+
+ if (cmds[cmd_list_index].name) {
+ while ((name = cmds[cmd_list_index].name)) {
+ cmd_list_index++;
+
+ if (VIR_STRDUP(rl_copy, rl_line_buffer) < 0)
+ return NULL;
+
+ char *saveptr;
+ pch = strtok_r(rl_copy, " ", &saveptr);
+
+ while (pch != NULL) {
+ if (STREQ(pch, name))
+ if (VIR_STRDUP(found_cmd, name) < 0)
+ goto cleanup;
+ pch = strtok_r(NULL, " ", &saveptr);
+ }
+ }
+ } else {
+ cmd_list_index = 0;
+ grp_list_index++;
+ }
+ }
+
+ if (!found_cmd)
+ goto cleanup;
+
+ return found_cmd;
+
+cleanup:
+ VIR_FREE(rl_copy);
+ return NULL;
+}
+
+/*
+ * Returns current valid command option name present in the rl_line_buffer.
+ */
+static char *
+vshCurrentOpt(const char *cmd_name)
+{
+ const vshCmdDef *cmd = NULL;
+ const char *name;
+ unsigned long int opt_index = 0;
+ size_t cmd_opt_list_index;
+ char *found_opt = NULL;
+ char *ptr = NULL;
+
+ if (!cmd_name)
+ return NULL;
+
+ cmd = vshCmddefSearch(cmd_name);
+
+ if (!cmd)
+ return NULL;
+
+ if (!cmd->opts)
+ return NULL;
+
+ cmd_opt_list_index = 0;
+
+ while ((name = cmd->opts[cmd_opt_list_index].name)) {
+ cmd_opt_list_index++;
+
+ if ((ptr = strstr(rl_line_buffer, name))) {
+ if (opt_index < (ptr - rl_line_buffer)) {
+ opt_index = ptr - rl_line_buffer;
+ if (VIR_STRDUP(found_opt, name) < 0)
+ return NULL;
+ }
+ }
+ }
+
+ return found_opt;
+}
+
+/*
+ * Returns name of the command completion currently present in the rl_line_buffer.
+ */
+static char*
+vshCurrentCmdCom(const char *cmd_name)
+{
+ const vshCmdDef *cmd = NULL;
+ size_t i;
+ char **completed_names = NULL;
+ char *found_cmd_com = NULL;
+
+ if (!cmd_name)
+ return NULL;
+
+ cmd = vshCmddefSearch(cmd_name);
+
+ if (!cmd)
+ return NULL;
+
+ if (!cmd->completer)
+ return NULL;
+
+ completed_names = cmd->completer(cmd->completer_flags);
+
+ if (!completed_names)
+ return NULL;
+
+ for (i = 0; completed_names[i]; i++) {
+ if (strstr(rl_line_buffer, completed_names[i])) {
+ if (VIR_STRDUP(found_cmd_com, completed_names[i]) < 0)
+ return NULL;
+ }
+ }
+
+ return found_cmd_com;
+}
+
+/*
+ * Returns name of the option completion currently present in the rl_line_buffer.
+ */
+static char *
+vshCurrentOptCom(const char *cmd_name)
+{
+ const vshCmdDef *cmd = NULL;
+ const vshCmdOptDef *opt = NULL;
+ unsigned long int opt_index = 0;
+ size_t i;
+ char **completed_names = NULL;
+ char *found_opt_com = NULL;
+ char *ptr = NULL;
+
+ if (!cmd_name)
+ return NULL;
+
+ cmd = vshCmddefSearch(cmd_name);
+
+ if (!cmd)
+ return NULL;
+
+ if (!cmd->opts)
+ return NULL;
+
+ for (i = 0; cmd->opts[i].name; i++) {
+ if ((ptr = strstr(rl_line_buffer, cmd->opts[i].name))) {
+ if (opt_index < (ptr - rl_line_buffer)) {
+ opt_index = ptr - rl_line_buffer;
+ opt = &cmd->opts[i];
+ }
+ }
+ }
+
+ if (!opt)
+ return NULL;
+
+ if (!opt->completer)
+ return NULL;
+
+ completed_names = opt->completer(opt->completer_flags);
+
+ if (!completed_names)
+ return NULL;
+
+ for (i = 0; completed_names[i]; i++) {
+ if (strstr(rl_line_buffer, completed_names[i])) {
+ if (VIR_STRDUP(found_opt_com, completed_names[i]) < 0)
+ return NULL;
+ }
+ }
+
+ return found_opt_com;
+}
+
static char **
-vshReadlineCompletion(const char *text, int start,
- int end ATTRIBUTE_UNUSED)
+vshReadlineCompletion(const char *text, int start, int end ATTRIBUTE_UNUSED)
{
- char **matches = (char **) NULL;
+ const char *cmd = vshCurrentCmd();
+ const char *cmd_com = vshCurrentCmdCom(cmd);
+ const char *opt = vshCurrentOpt(cmd);
+ const char *opt_com = vshCurrentOptCom(cmd);
+ char **matches = (char **)NULL;
- if (start == 0)
- /* command name generator */
+ if (start == 0) {
+ /* Command name generator. */
matches = rl_completion_matches(text, vshReadlineCommandGenerator);
- else
- /* commands options */
- matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else {
+ /* Command completer, commands options and commands options completer
+ * generators.
+ */
+ if (strstr(text, "-")) {
+ /* When user wants to see options. */
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (!cmd_com && !opt && !opt_com) {
+ matches = rl_completion_matches(text,
vshReadlineCommandCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (cmd_com && !opt && !opt_com) {
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (cmd_com && opt && !opt_com) {
+ matches = rl_completion_matches(text,
vshReadlineOptionsCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (!cmd_com && opt && !opt_com) {
+ matches = rl_completion_matches(text,
vshReadlineCommandCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text,
vshReadlineOptionsCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (!cmd_com && opt && opt_com) {
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (cmd_com && opt && opt_com) {
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ }
+ }
+
+ VIR_FREE(cmd);
+ VIR_FREE(cmd_com);
+ VIR_FREE(opt);
+ VIR_FREE(opt_com);
+
return matches;
}
-
static int
vshReadlineInit(vshControl *ctl)
{
--
1.8.3.1
12:54 p.m.
New subject: [libvirt] [PATCHv3 3/6] virsh: Improve readline generators and readline completion
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
New completion generators responsible for advances command
and command options completions.
Not sure if you meant s/advances/advanced/ ?
It might help to add a link to
http://cnswww.cns.cwru.edu/php/chet/readline/readline.html#SEC44, which
is documentation on how readline custom completers work, as part of the
commit message and/or in the code where you are installing custom
completers.
.gnulib | 2 +-
tools/virsh.c | 386 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 365 insertions(+), 23 deletions(-)
Oops, the change to .gnulib should NOT be here. You can fix it by
rebasing this series (git rebase -i, then change 'pick' to 'edit' for
this patch), and while on this patch, do 'git checkout HEAD^ .gnulib',
then 'make' (which should automatically rerun ./autogen.sh and thus
cover any fallout from the change to the submodule).
This feels like a pretty big patch. It adds a number of new functions
all at once; does it make sense to split this into multiple smaller
patches, focusing on one thing at a time?
+++ b/tools/virsh.c
@@ -2515,6 +2515,25 @@ vshCloseLogFile(vshControl *ctl)
* -----------------
*/
+static const vshCmdDef *
+vshDetermineCommandName(void)
+{
+ const vshCmdDef *cmd = NULL;
+ char *p;
+ char *cmdname;
+
+ if (!(p = strchr(rl_line_buffer, ' ')))
+ return NULL;
+
+ cmdname = vshMalloc(NULL, (p - rl_line_buffer) + 1);
+ memcpy(cmdname, rl_line_buffer, p - rl_line_buffer);
Why not VIR_STRNDUP? Oh...
+
+ cmd = vshCmddefSearch(cmdname);
+ VIR_FREE(cmdname);
+
+ return cmd;
+}
+
/*
* Generator function for command completion. STATE lets us
* know whether to start from scratch; without any state
@@ -2562,25 +2581,14 @@ vshReadlineCommandGenerator(const char *text, int state)
static char *
vshReadlineOptionsGenerator(const char *text, int state)
{
- static int list_index, len;
static const vshCmdDef *cmd = NULL;
+ static int list_index, len;
const char *name;
if (!state) {
- /* determine command name */
- char *p;
- char *cmdname;
-
- if (!(p = strchr(rl_line_buffer, ' ')))
- return NULL;
-
- cmdname = vshCalloc(NULL, (p - rl_line_buffer) + 1, 1);
- memcpy(cmdname, rl_line_buffer, p - rl_line_buffer);
...because this is a refactoring of pre-existing code.
-
- cmd = vshCmddefSearch(cmdname);
+ cmd = vshDetermineCommandName();
list_index = 0;
len = strlen(text);
- VIR_FREE(cmdname);
}
if (!cmd)
@@ -2612,22 +2620,356 @@ vshReadlineOptionsGenerator(const char *text, int state)
return NULL;
}
My summary from what I gathered after reading the readline programmer's
interface: any time a completion is attempted, the generator function
will be called repeatedly until it returns NULL; the first call has
state==0 and all subsequent calls have non-negative (but otherwise
unspecified) state. Each non-NULL return must be a malloc'd string that
serves as a possible completion of 'text'.
+/*
+ * Generator function for command completion, but unlike
+ * the vshReadlineCommandGenerator which completes command name, this function
+ * provides more advanced completion for commands by calling specific command
+ * completers (e.g. vshDomainCompleter).
+ */
+static char *
+vshReadlineCommandCompletionGenerator(const char *text, int state)
+{
+ static const vshCmdDef *cmd = NULL;
Explicit NULL initialization is not required - all 'static' variables
are implicitly zero-initialized.
+ static int list_index, len;
We maintain static state between functions (yuck - readline is not
thread-friendly - thankfully, we are only ever using completions from
the cli-parsing thread, so I guess we can get away with it).
+ char **completed_names = NULL;
+ char *name;
+
+ if (!state) {
+ cmd = vshDetermineCommandName();
+ list_index = 0;
+ len = strlen(text);
+ }
So on the first call, we determine which command we are completing, and
cache the length of 'text' being completed.
+
+ if (!cmd)
+ return NULL;
If we aren't completing a command, we're done (fall back to readline's
default completer).
+
+ if (!cmd->completer)
+ return NULL;
If the command doesn't have a specific completer, we're done (fall back
to readline's default completer).
+
+ completed_names = cmd->completer(cmd->completer_flags);
Malloc a complete list of names. Ouch - that means we malloc the list
on EVERY iteration of this function, even though we know this function
is called multiple times during one completion attempt. Really, we
should only malloc the list when state == 0, and refer back to that list
on all other calls. Which means the list of completed_names should be
static across calls.
+
+ if (!completed_names)
+ return NULL;
+
+ while ((name = completed_names[list_index])) {
For each name that the helper completer returned (but starting at the
point that we remembered from last time around - all the more reason
that we should ALSO remember the list from last time around, rather than
recomputing the list)...
+ char *res;
+ list_index++;
+
+ if (STRNEQLEN(name, text, len))
+ /* Skip irrelevant names */
+ continue;
...if it doesn't match the text we were given, skip it...
+
+ res = vshStrdup(NULL, name);
+ VIR_FREE(name);
Yuck. Why are we malloc'ing a copy and then freeing the original? Why
not just do transfer semantics (completed_name[list_index] = NULL, then
return name as is, without going through another strdup)?
+ return res;
+ }
+ VIR_FREE(completed_names);
Oops - this frees the list of names, but does NOT free the names
contained within the list. If ALL such names matched text, then you
avoided a leak; but if 'text' is not empty, then you are leaking
everywhere that you did a 'continue' in the loop above.
Oops - this code is only reached if you return NULL, but completed_names
was allocated on every entry (again, my argument is that completed_names
should be static, so that it is collected when state==0, and freed when
returning NULL, so that all other calls in between reuse the list).
+
+ return NULL;
+}
+
+/*
+ * Generator function for command option completion. Provides advances
s/advances/advanced/
> + * completion for command options.
> + */
> +static char *
> +vshReadlineOptionsCompletionGenerator(const char *text ATTRIBUTE_UNUSED,
> + int state ATTRIBUTE_UNUSED)
> +{
> + static const vshCmdDef *cmd = NULL;
> + static const vshCmdOptDef *opt = NULL;
+ static int list_index, len;
> + unsigned long int
opt_index = 0;
> + size_t i;
> + char **completed_names = NULL;
Similar problems about this list not being static between calls.
+
+ for (i = 0; cmd->opts[i].name; i++) {
+ if ((ptr = strstr(rl_line_buffer, cmd->opts[i].name)) &&
Are you sure strstr() is right? Wouldn't STRPREFIX be more appropriate?
That is, you only want to complete when you share a common prefix, not
when the text is a substring anywhere within a longer name.
+
+/*
+ * Returns current valid command name present in the rl_line_buffer.
+ */
+static char *
+vshCurrentCmd(void)
+{
+ const char *name;
+ const vshCmdGrp *grp;
+ const vshCmdDef *cmds;
+ size_t grp_list_index, cmd_list_index;
+ char *found_cmd = NULL;
+ char *rl_copy = NULL;
Please do NOT name your local variable rl_copy. rl_* is reserved for
readline entry points, and mixing namespaces will only cause confusion
for future readers of the code.
+ char *pch;
How often is this function going to be called? Is it something where
you should cache the results in something a bit longer-lasting? (Again,
I'm not too terribly fond of how thread-unsafe readline is, but as long
as it is only used by a single thread doing command-line parsing, we
might as well be efficient in our interactions with it)
+
+ grp_list_index = 0;
+ cmd_list_index = 0;
+ grp = cmdGroups;
+
+ while (grp[grp_list_index].name) {
+ cmds = grp[grp_list_index].commands;
+
+ if (cmds[cmd_list_index].name) {
+ while ((name = cmds[cmd_list_index].name)) {
+ cmd_list_index++;
+
+ if (VIR_STRDUP(rl_copy, rl_line_buffer) < 0)
+ return NULL;
+
+ char *saveptr;
+ pch = strtok_r(rl_copy, " ", &saveptr);
Does this work correctly with virsh's quoting rules, which are there to
allow for quoting arguments that contain a space?
> +
> +/*
> + * Returns current valid command option name present in the rl_line_buffer.
> + */
> +static char *
> +vshCurrentOpt(const char *cmd_name)
> +{
> + const vshCmdDef *cmd = NULL;
> + const char *name;
> + unsigned long int opt_index = 0;
> + size_t cmd_opt_list_index;
> + char *found_opt = NULL;
> + char *ptr = NULL;
> +
> + if (!cmd_name)
> + return NULL;
> +
> + cmd = vshCmddefSearch(cmd_name);
+
+ if (!cmd)
+ return NULL;
> +
> + if (!cmd->opts)
> + return NULL;
> +
> + cmd_opt_list_index = 0;
> +
> + while ((name = cmd->opts[cmd_opt_list_index].name)) {
> + cmd_opt_list_index++;
> +
> + if ((ptr = strstr(rl_line_buffer, name))) {
Again, I'm betting that strstr() is wrong.
> + if (opt_index < (ptr - rl_line_buffer)) {
> + opt_index = ptr - rl_line_buffer;
> + if (VIR_STRDUP(found_opt, name) < 0)
> + return NULL;
> + }
> + }
> + }
> +
> + return found_opt;
> +}
> +
> +/*
> + * Returns name of the command completion currently present in the rl_line_buffer.
> + */
> +static char*
> +vshCurrentCmdCom(const char *cmd_name)
> +{
> + const vshCmdDef *cmd = NULL;
> + size_t i;
> + char **completed_names = NULL;
> + char *found_cmd_com = NULL;
> +
> + if (!cmd_name)
> + return NULL;
> +
> + cmd = vshCmddefSearch(cmd_name);
+
+ if (!cmd)
+ return NULL;
+
+ if (!cmd->completer)
+ return NULL;
+
+ completed_names = cmd->completer(cmd->completer_flags);
> +
> + if (!completed_names)
> + return NULL;
> +
> + for (i = 0; completed_names[i]; i++) {
> + if (strstr(rl_line_buffer, completed_names[i])) {
Another suspicious strstr.
> + if (VIR_STRDUP(found_cmd_com, completed_names[i]) < 0)
> + return NULL;
> + }
> + }
> +
> + return found_cmd_com;
> +}
> +
> +/*
> + * Returns name of the option completion currently present in the rl_line_buffer.
> + */
> +static char *
> +vshCurrentOptCom(const char *cmd_name)
> +{
> + const vshCmdDef *cmd = NULL;
> + const vshCmdOptDef *opt = NULL;
> + unsigned long int opt_index = 0;
> + size_t i;
> + char **completed_names = NULL;
> + char *found_opt_com = NULL;
> + char *ptr = NULL;
> +
> + if (!cmd_name)
> + return NULL;
> +
> + cmd = vshCmddefSearch(cmd_name);
+
+ if (!cmd)
+ return NULL;
> +
> + if (!cmd->opts)
> + return NULL;
> +
> + for (i = 0; cmd->opts[i].name; i++) {
> + if ((ptr = strstr(rl_line_buffer, cmd->opts[i].name))) {
Another suspicious strstr.
+ if (opt_index < (ptr - rl_line_buffer)) {
+ opt_index = ptr - rl_line_buffer;
+ opt = &cmd->opts[i];
+ }
+ }
+ }
+
+ if (!opt)
+ return NULL;
+
+ if (!opt->completer)
+ return NULL;
+
+ completed_names = opt->completer(opt->completer_flags);
+
+ if (!completed_names)
+ return NULL;
+
+ for (i = 0; completed_names[i]; i++) {
+ if (strstr(rl_line_buffer, completed_names[i])) {
and another.
+ if (VIR_STRDUP(found_opt_com, completed_names[i]) <
0)
+ return NULL;
+ }
+ }
+
+ return found_opt_com;
+}
+
static char **
-vshReadlineCompletion(const char *text, int start,
- int end ATTRIBUTE_UNUSED)
+vshReadlineCompletion(const char *text, int start, int end ATTRIBUTE_UNUSED)
Why the spurious reformat?
{
- char **matches = (char **) NULL;
+ const char *cmd = vshCurrentCmd();
+ const char *cmd_com = vshCurrentCmdCom(cmd);
+ const char *opt = vshCurrentOpt(cmd);
+ const char *opt_com = vshCurrentOptCom(cmd);
+ char **matches = (char **)NULL;
- if (start == 0)
- /* command name generator */
+ if (start == 0) {
+ /* Command name generator. */
matches = rl_completion_matches(text, vshReadlineCommandGenerator);
- else
- /* commands options */
- matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else {
+ /* Command completer, commands options and commands options completer
+ * generators.
+ */
+ if (strstr(text, "-")) {
Again, strstr() is probably dead wrong here.
+ /* When user wants to see options. */
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (!cmd_com && !opt && !opt_com) {
+ matches = rl_completion_matches(text,
vshReadlineCommandCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
I'm not sure this is quite right. Remember, in my example of 2/6, if
the user types 'vol-key <TAB>', the completion should list the UNION of
two different completions: all possible options (--vol and --pool, maybe
even --help) and all possible volume names that don't resemble options.
But I don't see the filtering that rejects a volume name beginning with
'-'.
+ } else if (cmd_com && !opt && !opt_com) {
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (cmd_com && opt && !opt_com) {
+ matches = rl_completion_matches(text,
vshReadlineOptionsCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (!cmd_com && opt && !opt_com) {
+ matches = rl_completion_matches(text,
vshReadlineCommandCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text,
vshReadlineOptionsCompletionGenerator);
+ if (!matches)
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (!cmd_com && opt && opt_com) {
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
+ } else if (cmd_com && opt && opt_com) {
+ matches = rl_completion_matches(text, vshReadlineOptionsGenerator);
This looks like a rat's nest of completion functions, especially, when
some of the clauses (like these last two) have identical contents (and
could be combined into one by using simply 'if (opt && opt_com)' instead
of basing off of !cmd_com/cmd_com). Are we sure this complexity is
necessary?
+ }
+ }
+
+ VIR_FREE(cmd);
+ VIR_FREE(cmd_com);
+ VIR_FREE(opt);
+ VIR_FREE(opt_com);
+
return matches;
}
-
static int
vshReadlineInit(vshControl *ctl)
{
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:36 a.m.
New subject: [libvirt] [PATCHv3 4/6] virsh: Add vshDomainCompleter
Function vshDomainCompler returns domains names which can be used
by various virsh commands, for example:
virsh> start <TAB>
fedora domain_foo domain_bar
virsh> start f<TAB>
virsh> start fedora
Michal Privoznik recommended to add global variable virConnectPtr *__my_conn
so we can get the list of domains from the virConnecTListAllDomains().
vshReconnect() is called before the first command is executed
in order to provide autocompletion for the very first command.
---
v2
* global variable __my_conn renamed to vshConn
* @name is now const char *
* label cleanup renamed to error
v3
* removed useless if
* used virStringFreeList() instead of iteration
* removed vshReconnect() call from main(), reconnecting now takes place
in vshReadlineCommandCompletionGenerator() and only for relevant cmds
* added vshControl *vshCtl instead of virConnectPtr *vshConn because
vshCtl is needed in order to call vshReconnect()
* moved all .completer = vshDomainCompleter initializations from
other patches into this
tools/virsh-domain-monitor.c | 32 +++++--
tools/virsh-domain.c | 221 ++++++++++++++++++++++++++++++++++---------
tools/virsh-snapshot.c | 45 +++++++--
tools/virsh.c | 48 ++++++++++
tools/virsh.h | 2 +
5 files changed, 285 insertions(+), 63 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index b29b82a..0f30902 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1876,7 +1876,9 @@ const vshCmdDef domMonitoringCmds[] = {
.handler = cmdDomBlkError,
.opts = opts_domblkerror,
.info = info_domblkerror,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "domblkinfo",
.handler = cmdDomblkinfo,
@@ -1888,7 +1890,10 @@ const vshCmdDef domMonitoringCmds[] = {
.handler = cmdDomblklist,
.opts = opts_domblklist,
.info = info_domblklist,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "domblkstat",
.handler = cmdDomblkstat,
@@ -1900,7 +1905,9 @@ const vshCmdDef domMonitoringCmds[] = {
.handler = cmdDomControl,
.opts = opts_domcontrol,
.info = info_domcontrol,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "domif-getlink",
.handler = cmdDomIfGetLink,
@@ -1912,7 +1919,10 @@ const vshCmdDef domMonitoringCmds[] = {
.handler = cmdDomiflist,
.opts = opts_domiflist,
.info = info_domiflist,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "domifstat",
.handler = cmdDomIfstat,
@@ -1924,19 +1934,27 @@ const vshCmdDef domMonitoringCmds[] = {
.handler = cmdDominfo,
.opts = opts_dominfo,
.info = info_dominfo,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "dommemstat",
.handler = cmdDomMemStat,
.opts = opts_dommemstat,
.info = info_dommemstat,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "domstate",
.handler = cmdDomstate,
.opts = opts_domstate,
.info = info_domstate,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "list",
.handler = cmdList,
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5d4913d..5b200a3 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10358,7 +10358,10 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdAutostart,
.opts = opts_autostart,
.info = info_autostart,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "blkdeviotune",
.handler = cmdBlkdeviotune,
@@ -10370,7 +10373,10 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdBlkiotune,
.opts = opts_blkiotune,
.info = info_blkiotune,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "blockcommit",
.handler = cmdBlockCommit,
@@ -10413,7 +10419,10 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdConsole,
.opts = opts_console,
.info = info_console,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
#endif
{.name = "cpu-baseline",
@@ -10450,13 +10459,18 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdDesc,
.opts = opts_desc,
.info = info_desc,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "destroy",
.handler = cmdDestroy,
.opts = opts_destroy,
.info = info_destroy,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "detach-device",
.handler = cmdDetachDevice,
@@ -10480,31 +10494,45 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdDomDisplay,
.opts = opts_domdisplay,
.info = info_domdisplay,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "domfstrim",
.handler = cmdDomFSTrim,
.opts = opts_domfstrim,
.info = info_domfstrim,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "domhostname",
.handler = cmdDomHostname,
.opts = opts_domhostname,
.info = info_domhostname,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "domid",
.handler = cmdDomid,
.opts = opts_domid,
.info = info_domid,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "domif-setlink",
.handler = cmdDomIfSetLink,
.opts = opts_domif_setlink,
.info = info_domif_setlink,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "domiftune",
.handler = cmdDomIftune,
@@ -10522,7 +10550,9 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdDomjobinfo,
.opts = opts_domjobinfo,
.info = info_domjobinfo,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "domname",
.handler = cmdDomname,
@@ -10534,19 +10564,27 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdDomPMSuspend,
.opts = opts_dom_pm_suspend,
.info = info_dom_pm_suspend,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING
},
{.name = "dompmwakeup",
.handler = cmdDomPMWakeup,
.opts = opts_dom_pm_wakeup,
.info = info_dom_pm_wakeup,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "domuuid",
.handler = cmdDomuuid,
.opts = opts_domuuid,
.info = info_domuuid,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "domxml-from-native",
.handler = cmdDomXMLFromNative,
@@ -10564,31 +10602,46 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdDump,
.opts = opts_dump,
.info = info_dump,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING
},
{.name = "dumpxml",
.handler = cmdDumpXML,
.opts = opts_dumpxml,
.info = info_dumpxml,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "edit",
.handler = cmdEdit,
.opts = opts_edit,
.info = info_edit,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "inject-nmi",
.handler = cmdInjectNMI,
.opts = opts_inject_nmi,
.info = info_inject_nmi,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING
},
{.name = "send-key",
.handler = cmdSendKey,
.opts = opts_send_key,
.info = info_send_key,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING
},
{.name = "send-process-signal",
.handler = cmdSendProcessSignal,
@@ -10600,19 +10653,29 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdLxcEnterNamespace,
.opts = opts_lxc_enter_namespace,
.info = info_lxc_enter_namespace,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "managedsave",
.handler = cmdManagedSave,
.opts = opts_managedsave,
.info = info_managedsave,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING |
+ VIR_CONNECT_LIST_DOMAINS_NO_MANAGEDSAVE
},
{.name = "managedsave-remove",
.handler = cmdManagedSaveRemove,
.opts = opts_managedsaveremove,
.info = info_managedsaveremove,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_INACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_MANAGEDSAVE
},
{.name = "maxvcpus",
.handler = cmdMaxvcpus,
@@ -10624,7 +10687,10 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdMemtune,
.opts = opts_memtune,
.info = info_memtune,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "migrate",
.handler = cmdMigrate,
@@ -10636,31 +10702,46 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdMigrateSetMaxDowntime,
.opts = opts_migrate_setmaxdowntime,
.info = info_migrate_setmaxdowntime,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "migrate-compcache",
.handler = cmdMigrateCompCache,
.opts = opts_migrate_compcache,
.info = info_migrate_compcache,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_RUNNING
},
{.name = "migrate-setspeed",
.handler = cmdMigrateSetMaxSpeed,
.opts = opts_migrate_setspeed,
.info = info_migrate_setspeed,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "migrate-getspeed",
.handler = cmdMigrateGetMaxSpeed,
.opts = opts_migrate_getspeed,
.info = info_migrate_getspeed,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "numatune",
.handler = cmdNumatune,
.opts = opts_numatune,
.info = info_numatune,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "qemu-attach",
.handler = cmdQemuAttach,
@@ -10672,25 +10753,35 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdQemuMonitorCommand,
.opts = opts_qemu_monitor_command,
.info = info_qemu_monitor_command,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "qemu-agent-command",
.handler = cmdQemuAgentCommand,
.opts = opts_qemu_agent_command,
.info = info_qemu_agent_command,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "reboot",
.handler = cmdReboot,
.opts = opts_reboot,
.info = info_reboot,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "reset",
.handler = cmdReset,
.opts = opts_reset,
.info = info_reset,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "restore",
.handler = cmdRestore,
@@ -10702,7 +10793,10 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdResume,
.opts = opts_resume,
.info = info_resume,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_PAUSED
},
{.name = "save",
.handler = cmdSave,
@@ -10732,61 +10826,83 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdSchedinfo,
.opts = opts_schedinfo,
.info = info_schedinfo,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "screenshot",
.handler = cmdScreenshot,
.opts = opts_screenshot,
.info = info_screenshot,
- .flags = 0
+ .flags = 0,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "setmaxmem",
.handler = cmdSetmaxmem,
.opts = opts_setmaxmem,
.info = info_setmaxmem,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "setmem",
.handler = cmdSetmem,
.opts = opts_setmem,
.info = info_setmem,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "setvcpus",
.handler = cmdSetvcpus,
.opts = opts_setvcpus,
.info = info_setvcpus,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "shutdown",
.handler = cmdShutdown,
.opts = opts_shutdown,
.info = info_shutdown,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "start",
.handler = cmdStart,
.opts = opts_start,
.info = info_start,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_INACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_PERSISTENT
},
{.name = "suspend",
.handler = cmdSuspend,
.opts = opts_suspend,
.info = info_suspend,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "ttyconsole",
.handler = cmdTTYConsole,
.opts = opts_ttyconsole,
.info = info_ttyconsole,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = "undefine",
.handler = cmdUndefine,
.opts = opts_undefine,
.info = info_undefine,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "update-device",
.handler = cmdUpdateDevice,
@@ -10804,25 +10920,36 @@ const vshCmdDef domManagementCmds[] = {
.handler = cmdVcpuinfo,
.opts = opts_vcpuinfo,
.info = info_vcpuinfo,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "vcpupin",
.handler = cmdVcpuPin,
.opts = opts_vcpupin,
.info = info_vcpupin,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "emulatorpin",
.handler = cmdEmulatorPin,
.opts = opts_emulatorpin,
.info = info_emulatorpin,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "vncdisplay",
.handler = cmdVNCDisplay,
.opts = opts_vncdisplay,
.info = info_vncdisplay,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
},
{.name = NULL}
};
diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c
index e37a5b3..77f9273 100644
--- a/tools/virsh-snapshot.c
+++ b/tools/virsh-snapshot.c
@@ -2006,25 +2006,37 @@ const vshCmdDef snapshotCmds[] = {
.handler = cmdSnapshotCreate,
.opts = opts_snapshot_create,
.info = info_snapshot_create,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-create-as",
.handler = cmdSnapshotCreateAs,
.opts = opts_snapshot_create_as,
.info = info_snapshot_create_as,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-current",
.handler = cmdSnapshotCurrent,
.opts = opts_snapshot_current,
.info = info_snapshot_current,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-delete",
.handler = cmdSnapshotDelete,
.opts = opts_snapshot_delete,
.info = info_snapshot_delete,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-dumpxml",
.handler = cmdSnapshotDumpXML,
@@ -2036,31 +2048,46 @@ const vshCmdDef snapshotCmds[] = {
.handler = cmdSnapshotEdit,
.opts = opts_snapshot_edit,
.info = info_snapshot_edit,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-info",
.handler = cmdSnapshotInfo,
.opts = opts_snapshot_info,
.info = info_snapshot_info,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-list",
.handler = cmdSnapshotList,
.opts = opts_snapshot_list,
.info = info_snapshot_list,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-parent",
.handler = cmdSnapshotParent,
.opts = opts_snapshot_parent,
.info = info_snapshot_parent,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = "snapshot-revert",
.handler = cmdDomainSnapshotRevert,
.opts = opts_snapshot_revert,
.info = info_snapshot_revert,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
},
{.name = NULL}
};
diff --git a/tools/virsh.c b/tools/virsh.c
index 5be8e7e..ae2dd55 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -88,6 +88,8 @@ static char *progname;
static const vshCmdGrp cmdGroups[];
+vshControl *vshCtl;
+
/* Bypass header poison */
#undef strdup
@@ -317,6 +319,7 @@ vshCatchDisconnect(virConnectPtr conn ATTRIBUTE_UNUSED,
static void
vshReconnect(vshControl *ctl)
{
+ printf("\n>>>\n");
bool connected = false;
if (ctl->conn) {
@@ -2510,6 +2513,46 @@ vshCloseLogFile(vshControl *ctl)
#ifdef USE_READLINE
+/* -------------
+ * Completers
+ * -------------
+ */
+
+char **
+vshDomainCompleter(unsigned int flags)
+{
+ virDomainPtr *domains;
+ size_t i;
+ char **names = NULL;
+ int ndomains;
+
+ if (!vshCtl->conn)
+ return NULL;
+
+ ndomains = virConnectListAllDomains(vshCtl->conn, &domains, flags);
+
+ if (ndomains < 0)
+ return NULL;
+
+ names = vshMalloc(NULL, sizeof(char *) * (ndomains + 1));
+
+ for (i = 0; i < ndomains; i++) {
+ const char *name = virDomainGetName(domains[i]);
+ if (VIR_STRDUP(names[i], name) < 0) {
+ virDomainFree(domains[i]);
+ goto error;
+ }
+ virDomainFree(domains[i]);
+ }
+ names[i] = NULL;
+ VIR_FREE(domains);
+ return names;
+
+error:
+ virStringFreeList(names);
+ return NULL;
+}
+
/* -----------------
* Readline stuff
* -----------------
@@ -2643,6 +2686,10 @@ vshReadlineCommandCompletionGenerator(const char *text, int state)
if (!cmd)
return NULL;
+ if ((vshCtl->conn == NULL || disconnected) &&
+ !(cmd->flags & VSH_CMD_FLAG_NOCONNECT))
+ vshReconnect(vshCtl);
+
if (!cmd->completer)
return NULL;
@@ -3509,6 +3556,7 @@ main(int argc, char **argv)
ctl->debug = VSH_DEBUG_DEFAULT;
ctl->escapeChar = "^]"; /* Same default as telnet */
+ vshCtl = ctl;
if (!setlocale(LC_ALL, "")) {
perror("setlocale");
diff --git a/tools/virsh.h b/tools/virsh.h
index 064acde..68414e4 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -255,6 +255,8 @@ struct _vshCmdGrp {
const vshCmdDef *commands;
};
+char **vshDomainCompleter(unsigned int flags);
+
void vshError(vshControl *ctl, const char *format, ...)
ATTRIBUTE_FMT_PRINTF(2, 3);
void vshOpenLogFile(vshControl *ctl);
--
1.8.3.1
3:37 p.m.
New subject: [libvirt] [PATCHv3 4/6] virsh: Add vshDomainCompleter
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
Function vshDomainCompler returns domains names which can be used
by various virsh commands, for example:
virsh> start <TAB>
fedora domain_foo domain_bar
virsh> start f<TAB>
virsh> start fedora
Michal Privoznik recommended to add global variable virConnectPtr *__my_conn
Stale name in the commit message.
so we can get the list of domains from the
virConnecTListAllDomains().
vshReconnect() is called before the first command is executed
in order to provide autocompletion for the very first command.
Stale comment, now that you fixed it to delay until completion requires
a connection.
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index b29b82a..0f30902 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -1876,7 +1876,9 @@ const vshCmdDef domMonitoringCmds[] = {
.handler = cmdDomBlkError,
.opts = opts_domblkerror,
.info = info_domblkerror,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE
I'm still not sure I get why we need per-command completers; isn't
per-option completion sufficient (by associating the completer with the
--domain of each of these commands)?
@@ -1888,7 +1890,10 @@ const vshCmdDef domMonitoringCmds[] = {
.handler = cmdDomblklist,
.opts = opts_domblklist,
.info = info_domblklist,
- .flags = 0
+ .flags = 0,
+ .completer = vshDomainCompleter,
+ .completer_flags = VIR_CONNECT_LIST_DOMAINS_ACTIVE |
+ VIR_CONNECT_LIST_DOMAINS_INACTIVE
Style: please use trailing comma after the last element of the struct,
so that future additions don't have to modify existing lines when adding
even more struct members down the road (throughout this patch, and
probably throughout the series). Yeah, I know existing C99 initializers
for vshCmdInfo.dad and vshCmdOptDef.help aren't using trailing commas
consistently, so maybe that's worth a separate cleanup patch. If we had
been following that style, you wouldn't be modifying the .flags lines.
+++ b/tools/virsh.c
@@ -88,6 +88,8 @@ static char *progname;
static const vshCmdGrp cmdGroups[];
+vshControl *vshCtl;
This variable should probably be marked static, as I don't see it
referenced in any other file.
+
/* Bypass header poison */
#undef strdup
@@ -317,6 +319,7 @@ vshCatchDisconnect(virConnectPtr conn ATTRIBUTE_UNUSED,
static void
vshReconnect(vshControl *ctl)
{
+ printf("\n>>>\n");
Leftover debugging?
@@ -2510,6 +2513,46 @@ vshCloseLogFile(vshControl *ctl)
#ifdef USE_READLINE
+/* -------------
+ * Completers
+ * -------------
+ */
+
+char **
+vshDomainCompleter(unsigned int flags)
Ouch - you have a link error if building without USE_READLINE, since the
use of this function was unconditional but its implementation is
conditional. How much of this code can you hoist outside of
USE_READLINE? If you can't, then please provide the #else half so that
at least things still link.
+{
+ virDomainPtr *domains;
+ size_t i;
+ char **names = NULL;
+ int ndomains;
+
+ if (!vshCtl->conn)
+ return NULL;
+
+ ndomains = virConnectListAllDomains(vshCtl->conn, &domains, flags);
+
+ if (ndomains < 0)
+ return NULL;
+
+ names = vshMalloc(NULL, sizeof(char *) * (ndomains + 1));
+
VIR_ALLOC_N seems nicer for the task.
+ for (i = 0; i < ndomains; i++) {
+ const char *name = virDomainGetName(domains[i]);
+ if (VIR_STRDUP(names[i], name) < 0) {
+ virDomainFree(domains[i]);
+ goto error;
+ }
+ virDomainFree(domains[i]);
+ }
+ names[i] = NULL;
+ VIR_FREE(domains);
+ return names;
+
+error:
Memory leak - if you get here, you don't finish calling virDomainFree()
on the tail of the list, nor VIR_FREE(domains). That cleanup needs to
be unconditional.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:36 a.m.
New subject: [libvirt] [PATCHv3 5/6] virsh: Add vshSuspendTargetCompleter
vshSuspendTargetCompleter returns targets available for suspend.
This completer can be used for the command option completion
(for dompmsuspend, etc.).
virsh> dompmsuspend --target <TAB>
mem disk hybrid
virsh> dompmsuspend --target h<TAB>
virsh> dompmsuspend --target hybrid
---
v2
* label cleanup renamed to error
* vshSuspendTargetCompleter added to opts_dom_pm_suspend
v3
* removed useless if
* used virStringFreeList() instead of iteration
tools/virsh-domain.c | 3 ++-
tools/virsh.c | 23 +++++++++++++++++++++++
tools/virsh.h | 1 +
3 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 5b200a3..8a118ed 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2756,7 +2756,8 @@ static const vshCmdOptDef opts_dom_pm_suspend[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("mem(Suspend-to-RAM), "
"disk(Suspend-to-Disk), "
- "hybrid(Hybrid-Suspend)")
+ "hybrid(Hybrid-Suspend)"),
+ .completer = vshSuspendTargetCompleter
},
{.name = NULL}
};
diff --git a/tools/virsh.c b/tools/virsh.c
index ae2dd55..5a5e520 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2553,6 +2553,29 @@ error:
return NULL;
}
+char **
+vshSuspendTargetCompleter(unsigned int unused_flags ATTRIBUTE_UNUSED)
+{
+ const char *targets[] = {"mem", "disk", "hybrid"};
+ const unsigned int targets_size = ARRAY_CARDINALITY(targets);
+ char **names = NULL;
+ size_t i;
+
+ names = vshMalloc(NULL, sizeof(char *) * (targets_size + 1));
+
+ for (i = 0; i < targets_size; i++) {
+ if (VIR_STRDUP(names[i], targets[i]) < 0)
+ goto error;
+ }
+
+ names[i] = NULL;
+ return names;
+
+error:
+ virStringFreeList(names);
+ return NULL;
+}
+
/* -----------------
* Readline stuff
* -----------------
diff --git a/tools/virsh.h b/tools/virsh.h
index 68414e4..6767e65 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -256,6 +256,7 @@ struct _vshCmdGrp {
};
char **vshDomainCompleter(unsigned int flags);
+char **vshSuspendTargetCompleter(unsigned int unused_flags);
void vshError(vshControl *ctl, const char *format, ...)
ATTRIBUTE_FMT_PRINTF(2, 3);
--
1.8.3.1
3:43 p.m.
New subject: [libvirt] [PATCHv3 5/6] virsh: Add vshSuspendTargetCompleter
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
vshSuspendTargetCompleter returns targets available for suspend.
This completer can be used for the command option completion
(for dompmsuspend, etc.).
virsh> dompmsuspend --target <TAB>
mem disk hybrid
virsh> dompmsuspend --target h<TAB>
virsh> dompmsuspend --target hybrid
---
v2
* label cleanup renamed to error
* vshSuspendTargetCompleter added to opts_dom_pm_suspend
v3
* removed useless if
* used virStringFreeList() instead of iteration
tools/virsh-domain.c | 3 ++-
tools/virsh.c | 23 +++++++++++++++++++++++
tools/virsh.h | 1 +
3 files changed, 26 insertions(+), 1 deletion(-)
This patch looks pretty decent, especially since it is installed as a
per-option handler.
+char **
+vshSuspendTargetCompleter(unsigned int unused_flags ATTRIBUTE_UNUSED)
+{
+ const char *targets[] = {"mem", "disk", "hybrid"};
+ const unsigned int targets_size = ARRAY_CARDINALITY(targets);
+ char **names = NULL;
+ size_t i;
+
+ names = vshMalloc(NULL, sizeof(char *) * (targets_size + 1));
+
+ for (i = 0; i < targets_size; i++) {
+ if (VIR_STRDUP(names[i], targets[i]) < 0)
+ goto error;
+ }
+
+ names[i] = NULL;
+ return names;
Call me a hacker, but what's wrong with this equivalent implementation
done by reusing existing code:
char **
vshSuspendTargetCompleter(unsigned int ignored ATTRIBUTE_UNUSED)
{
return virStringSplit("mem disk hybrid", " ", 0);
}
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
12:19 p.m.
New subject: [libvirt] [PATCHv3 5/6] virsh: Add vshSuspendTargetCompleter
On 26/08/13 at 02:43pm, Eric Blake wrote:
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
> vshSuspendTargetCompleter returns targets available for suspend.
>
> This completer can be used for the command option completion
> (for dompmsuspend, etc.).
>
> virsh> dompmsuspend --target <TAB>
> mem disk hybrid
> virsh> dompmsuspend --target h<TAB>
> virsh> dompmsuspend --target hybrid
> ---
> v2
> * label cleanup renamed to error
> * vshSuspendTargetCompleter added to opts_dom_pm_suspend
>
> v3
> * removed useless if
> * used virStringFreeList() instead of iteration
>
> tools/virsh-domain.c | 3 ++-
> tools/virsh.c | 23 +++++++++++++++++++++++
> tools/virsh.h | 1 +
> 3 files changed, 26 insertions(+), 1 deletion(-)
This patch looks pretty decent, especially since it is installed as a
per-option handler.
>
> +char **
> +vshSuspendTargetCompleter(unsigned int unused_flags ATTRIBUTE_UNUSED)
> +{
> + const char *targets[] = {"mem", "disk",
"hybrid"};
> + const unsigned int targets_size = ARRAY_CARDINALITY(targets);
> + char **names = NULL;
> + size_t i;
> +
> + names = vshMalloc(NULL, sizeof(char *) * (targets_size + 1));
> +
> + for (i = 0; i < targets_size; i++) {
> + if (VIR_STRDUP(names[i], targets[i]) < 0)
> + goto error;
> + }
> +
> + names[i] = NULL;
> + return names;
Call me a hacker, but what's wrong with this equivalent implementation
done by reusing existing code:
char **
vshSuspendTargetCompleter(unsigned int ignored ATTRIBUTE_UNUSED)
{
return virStringSplit("mem disk hybrid", " ", 0);
}
Yeah, this is much better. I will definitely use this.
--
Tomas Meszaros
7:36 a.m.
New subject: [libvirt] [PATCHv3 6/6] virsh: Add vshRebootShutdownModeCompleter
vshRebootShutdownModeCompleter returns available shutdown mode
names. This can be used for --mode auto completion for commands
such as reboot or shutdown.
for example:
virsh> reboot --mode <TAB>
acpi agent initctl signal
virsh> reboot --mode i<TAB>
virsh> reboot --mode initctl
---
v3
* removed useless if
* used virStringFreeList() instead of iteration
* moved all .completer = vshRebootShutdownModeCompleter initializations
into this patch
tools/virsh-domain.c | 9 +++++----
tools/virsh.c | 23 +++++++++++++++++++++++
tools/virsh.h | 1 +
3 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 8a118ed..c60a01c 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -2756,8 +2756,7 @@ static const vshCmdOptDef opts_dom_pm_suspend[] = {
.flags = VSH_OFLAG_REQ,
.help = N_("mem(Suspend-to-RAM), "
"disk(Suspend-to-Disk), "
- "hybrid(Hybrid-Suspend)"),
- .completer = vshSuspendTargetCompleter
+ "hybrid(Hybrid-Suspend)")
},
{.name = NULL}
};
@@ -4714,7 +4713,8 @@ static const vshCmdOptDef opts_shutdown[] = {
},
{.name = "mode",
.type = VSH_OT_STRING,
- .help = N_("shutdown mode: acpi|agent|initctl|signal")
+ .help = N_("shutdown mode: acpi|agent|initctl|signal"),
+ .completer = vshRebootShutdownModeCompleter
},
{.name = NULL}
};
@@ -4800,7 +4800,8 @@ static const vshCmdOptDef opts_reboot[] = {
},
{.name = "mode",
.type = VSH_OT_STRING,
- .help = N_("shutdown mode: acpi|agent|initctl|signal")
+ .help = N_("shutdown mode: acpi|agent|initctl|signal"),
+ .completer = vshRebootShutdownModeCompleter
},
{.name = NULL}
};
diff --git a/tools/virsh.c b/tools/virsh.c
index 5a5e520..e6b1309 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -2576,6 +2576,29 @@ error:
return NULL;
}
+char **
+vshRebootShutdownModeCompleter(unsigned int unused_flags ATTRIBUTE_UNUSED)
+{
+ const char *modes[] = {"acpi", "agent", "initctl",
"signal"};
+ const unsigned int modes_size = ARRAY_CARDINALITY(modes);
+ char **names = NULL;
+ size_t i;
+
+ names = vshMalloc(NULL, sizeof(char *) * (modes_size + 1));
+
+ for (i = 0; i < modes_size; i++) {
+ if (VIR_STRDUP(names[i], modes[i]) < 0)
+ goto cleanup;
+ }
+
+ names[i] = NULL;
+ return names;
+
+cleanup:
+ virStringFreeList(names);
+ return NULL;
+}
+
/* -----------------
* Readline stuff
* -----------------
diff --git a/tools/virsh.h b/tools/virsh.h
index 6767e65..61510b0 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -257,6 +257,7 @@ struct _vshCmdGrp {
char **vshDomainCompleter(unsigned int flags);
char **vshSuspendTargetCompleter(unsigned int unused_flags);
+char **vshRebootShutdownModeCompleter(unsigned int unused_flags);
void vshError(vshControl *ctl, const char *format, ...)
ATTRIBUTE_FMT_PRINTF(2, 3);
--
1.8.3.1
3:48 p.m.
New subject: [libvirt] [PATCHv3 6/6] virsh: Add vshRebootShutdownModeCompleter
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
vshRebootShutdownModeCompleter returns available shutdown mode
names. This can be used for --mode auto completion for commands
such as reboot or shutdown.
for example:
virsh> reboot --mode <TAB>
acpi agent initctl signal
virsh> reboot --mode i<TAB>
virsh> reboot --mode initctl
---
v3
* removed useless if
* used virStringFreeList() instead of iteration
* moved all .completer = vshRebootShutdownModeCompleter initializations
into this patch
Once again, seems reasonable, but can be shortened:
+char **
+vshRebootShutdownModeCompleter(unsigned int unused_flags ATTRIBUTE_UNUSED)
+{
+ const char *modes[] = {"acpi", "agent", "initctl",
"signal"};
+ const unsigned int modes_size = ARRAY_CARDINALITY(modes);
+ char **names = NULL;
+ size_t i;
+
+ names = vshMalloc(NULL, sizeof(char *) * (modes_size + 1));
+
+ for (i = 0; i < modes_size; i++) {
+ if (VIR_STRDUP(names[i], modes[i]) < 0)
+ goto cleanup;
+ }
+
+ names[i] = NULL;
+ return names;
+
+cleanup:
+ virStringFreeList(names);
+ return NULL;
All this boils down to an open-coded version of
return virStringSplit("acpi agent initctl signal", " ", 0);
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:01 a.m.
On 08/26/2013 06:36 AM, Tomas Meszaros wrote:
Hi, this patch series is a prototype for my GSoC project (Michal
Privoznik
is my mentor).
I'm working on virsh auto-completion, trying to make it more
"intelligent".
At this stage, prototype is capable of command and option completion. Three
completer functions are currently implemented so you can test it. If it turns
out that this prototype is good enough, I will implement more completer functions.
As we just discovered on IRC, the formula for building without readline
is to either uninstall readline-devel, or to build with:
./configure ac_cv_lib_readline_readline=no \
ac_cv_search_tgetent=no ac_cv_lib_readline_rl_initialize=no
(Yes, I need to add a patch that adds './configure --disable-readline';
I started a patch on my local tree when we were trying to resolve the
incompatible vbox (LGPLv2-only) vs. readline (GPLv3+) issue a while
back, but since we solved it via moving vbox, I didn't have to castrate
readline at that time)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4105
days inactive
4109
days old
15 comments
2 participants
participants (2)
-
Eric Blake -
Tomas Meszaros