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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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> -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.