[libvirt] [PATCH] virsh: Add a completer for `domifaddr` --source parameter.
The command `domifaddr` can use three different sources to grab IP address of a Virtual Machine: lease, agent and arp. This parameter does not have a completer function to return source options. Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-completer-domain.c | 17 +++++++++++++++++ tools/virsh-completer-domain.h | 5 +++++ tools/virsh-domain-monitor.c | 1 + 3 files changed, 23 insertions(+) diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 0311ee50d0..c8709baa38 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -296,3 +296,20 @@ virshDomainShutdownModeCompleter(vshControl *ctl, return virshCommaStringListComplete(mode, modes); } + + +char ** +virshDomainIfAddrSourceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + const char *sources[] = {"lease", "agent", "arp", NULL}; + const char *source = NULL; + + virCheckFlags(0, NULL); + + if (vshCommandOptStringQuiet(ctl, cmd, "source", &source) < 0) + return NULL; + + return virshCommaStringListComplete(source, sources); +} diff --git a/tools/virsh-completer-domain.h b/tools/virsh-completer-domain.h index 083ab327cc..f5e5625051 100644 --- a/tools/virsh-completer-domain.h +++ b/tools/virsh-completer-domain.h @@ -53,3 +53,8 @@ char ** virshDomainDeviceAliasCompleter(vshControl *ctl, char ** virshDomainShutdownModeCompleter(vshControl *ctl, const vshCmd *cmd, unsigned int flags); + +char ** +virshDomainIfAddrSourceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 30b186ffd1..1d1f87eb9e 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -2346,6 +2346,7 @@ static const vshCmdOptDef opts_domifaddr[] = { {.name = "source", .type = VSH_OT_STRING, .flags = VSH_OFLAG_NONE, + .completer = virshDomainIfAddrSourceCompleter, .help = N_("address source: 'lease', 'agent', or 'arp'")}, {.name = NULL} }; -- 2.20.1
On Thu, Jan 02, 2020 at 12:07:06PM -0300, Julio Faracco wrote:
The command `domifaddr` can use three different sources to grab IP address of a Virtual Machine: lease, agent and arp. This parameter does not have a completer function to return source options.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- ...
+ +char ** +virshDomainIfAddrSourceCompleter(vshControl *ctl,
I'd stay consistent with the existing naming scheme we have in place and expand 'If' to 'Interface' --> virshDomainInterfaceAddrSourceCompleter. Changed and pushed. Reviewed-by: Erik Skultety <eskultet@redhat.com>
On 1/2/20 4:07 PM, Julio Faracco wrote:
The command `domifaddr` can use three different sources to grab IP address of a Virtual Machine: lease, agent and arp. This parameter does not have a completer function to return source options.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-completer-domain.c | 17 +++++++++++++++++ tools/virsh-completer-domain.h | 5 +++++ tools/virsh-domain-monitor.c | 1 + 3 files changed, 23 insertions(+)
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 0311ee50d0..c8709baa38 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -296,3 +296,20 @@ virshDomainShutdownModeCompleter(vshControl *ctl,
return virshCommaStringListComplete(mode, modes); } + + +char ** +virshDomainIfAddrSourceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + const char *sources[] = {"lease", "agent", "arp", NULL}; + const char *source = NULL; + + virCheckFlags(0, NULL); + + if (vshCommandOptStringQuiet(ctl, cmd, "source", &source) < 0) + return NULL; + + return virshCommaStringListComplete(source, sources);
Actually, I don't think this is coorect. This helper completes a string list separated by commas, for instance a shutdown mode where more than one string (method) can be used: virsh shutdown --mode acpi,agent,signal But this is not the case for domifaddr --source, is it? It accepts exactly one string. I know Erik pushed this, so I will post a fix up later. Stay tuned. Michal
Michal, This case of comma separated options, can we include a comma after completion by default? I.e.: virsh # domifaddr 1 --source [TAB] agent arp lease virsh # domifaddr 1 --source ar[TAB] virsh # domifaddr 1 --source arp, This is easy to test and avoid mistakes. -- Julio Cesar Faracco Em sex., 3 de jan. de 2020 às 12:45, Michal Prívozník <mprivozn@redhat.com> escreveu:
On 1/2/20 4:07 PM, Julio Faracco wrote:
The command `domifaddr` can use three different sources to grab IP address of a Virtual Machine: lease, agent and arp. This parameter does not have a completer function to return source options.
Signed-off-by: Julio Faracco <jcfaracco@gmail.com> --- tools/virsh-completer-domain.c | 17 +++++++++++++++++ tools/virsh-completer-domain.h | 5 +++++ tools/virsh-domain-monitor.c | 1 + 3 files changed, 23 insertions(+)
diff --git a/tools/virsh-completer-domain.c b/tools/virsh-completer-domain.c index 0311ee50d0..c8709baa38 100644 --- a/tools/virsh-completer-domain.c +++ b/tools/virsh-completer-domain.c @@ -296,3 +296,20 @@ virshDomainShutdownModeCompleter(vshControl *ctl,
return virshCommaStringListComplete(mode, modes); } + + +char ** +virshDomainIfAddrSourceCompleter(vshControl *ctl, + const vshCmd *cmd, + unsigned int flags) +{ + const char *sources[] = {"lease", "agent", "arp", NULL}; + const char *source = NULL; + + virCheckFlags(0, NULL); + + if (vshCommandOptStringQuiet(ctl, cmd, "source", &source) < 0) + return NULL; + + return virshCommaStringListComplete(source, sources);
Actually, I don't think this is coorect. This helper completes a string list separated by commas, for instance a shutdown mode where more than one string (method) can be used:
virsh shutdown --mode acpi,agent,signal
But this is not the case for domifaddr --source, is it? It accepts exactly one string. I know Erik pushed this, so I will post a fix up later. Stay tuned.
Michal
On 1/3/20 4:56 PM, Julio Faracco wrote:
Michal,
This case of comma separated options, can we include a comma after completion by default? I.e.:
virsh # domifaddr 1 --source [TAB] agent arp lease virsh # domifaddr 1 --source ar[TAB] virsh # domifaddr 1 --source arp,
This is easy to test and avoid mistakes.
BTW: --source doesn't accept multiple values. But anyway, for that we would need to change the way we parse the argument value because for instance: virsh shutdown --mode agent, fedora is not viewed as valid command by current implementation: error: Unknown mode value, expecting 'acpi', 'agent', 'initctl', 'signal', or 'paravirt' The fix for this is pretty simple: index 9315755990..e3daa7d015 100644 --- i/tools/virsh-domain.c +++ w/tools/virsh-domain.c @@ -5847 +5847 @@ cmdShutdown(vshControl *ctl, const vshCmd *cmd) - while (tmp && *tmp) { + while (tmp && *tmp && **tmp) { But the question is, should we? I mean, this way of accepting multiple values is pretty unique. I haven't seen any other CLI tool like it (except for QEMU obviously) so I don't see any behaviour we could mimic. And I find comma at the end a bit ugly looking. Michal
participants (3)
-
Erik Skultety -
Julio Faracco -
Michal Prívozník