[libvirt] [PATCH] virsh: fix build without readline

John Ferlan jferlan at redhat.com
Sat Jan 13 12:42:37 UTC 2018



On 01/13/2018 02:00 AM, Roman Bogorodskiy wrote:
> Completion in virsh is enabled when readline is available. However,
> when it's not available, we should:
> 
>  * avoid defining completers with completion functions;
>  * in cmdComplete(), mark unused arguments when there's no readline with
>    ATTRIBUTE_UNUSED.
> ---
>  tools/virsh-domain-monitor.c |  6 ++++++
>  tools/virsh-domain.c         |  6 ++++++
>  tools/virsh.h                | 11 ++++++++++-
>  tools/virt-admin.c           | 14 ++++++++++++++
>  tools/vsh.c                  |  2 +-
>  5 files changed, 37 insertions(+), 2 deletions(-)
> 

Any chance to reduce the number of places to change that we can modify
virsh-completer.c to have the "#if WITH_READLINE" where the #else would
just return NULL and uses virCheckFlags(-1, NULL); to avoid the
syntax-check issue for the two API's defined so far?

Reading vshReadlineParse it seems as if the .completer returns NULL,
then it's no different than if the .completer was NULL.

I'm only one cup of coffee in though - so  I could be wrong ;-).  It
would have been "better" if the two new API's came with some
documentation over usage expectations (grumble, grumble).

John

> diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
> index 32a42707e..0df20eea0 100644
> --- a/tools/virsh-domain-monitor.c
> +++ b/tools/virsh-domain-monitor.c
> @@ -659,7 +659,9 @@ static const vshCmdOptDef opts_domif_getlink[] = {
>      {.name = "interface",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#if WITH_READLINE
>       .completer = virshDomainInterfaceCompleter,
> +#endif
>       .help = N_("interface device (MAC Address)")
>      },
>      {.name = "persistent",
> @@ -996,7 +998,9 @@ static const vshCmdOptDef opts_domifstat[] = {
>      {.name = "interface",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = virshDomainInterfaceCompleter,
> +#endif
>       .help = N_("interface device specified by name or MAC Address")
>      },
>      {.name = NULL}
> @@ -2151,7 +2155,9 @@ static const vshCmdOptDef opts_domifaddr[] = {
>      {.name = "interface",
>       .type = VSH_OT_STRING,
>       .flags = VSH_OFLAG_NONE,
> +#ifdef WITH_READLINE
>       .completer = virshDomainInterfaceCompleter,
> +#endif
>       .help = N_("network interface name")},
>      {.name = "full",
>       .type = VSH_OT_BOOL,
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 0f329d6d7..c5511adbf 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -2977,7 +2977,9 @@ static const vshCmdOptDef opts_domif_setlink[] = {
>      {.name = "interface",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = virshDomainInterfaceCompleter,
> +#endif
>       .help = N_("interface device (MAC Address)")
>      },
>      {.name = "state",
> @@ -3148,7 +3150,9 @@ static const vshCmdOptDef opts_domiftune[] = {
>      {.name = "interface",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = virshDomainInterfaceCompleter,
> +#endif
>       .help = N_("interface device (MAC Address)")
>      },
>      {.name = "inbound",
> @@ -11985,8 +11989,10 @@ static const vshCmdOptDef opts_detach_interface[] = {
>      },
>      {.name = "mac",
>       .type = VSH_OT_STRING,
> +#ifdef WITH_READLINE
>       .completer = virshDomainInterfaceCompleter,
>       .completer_flags = VIRSH_DOMAIN_INTERFACE_COMPLETER_MAC,
> +#endif
>       .help = N_("MAC address")
>      },
>      VIRSH_COMMON_OPT_DOMAIN_PERSISTENT,
> diff --git a/tools/virsh.h b/tools/virsh.h
> index 528e04558..b3cb15ac4 100644
> --- a/tools/virsh.h
> +++ b/tools/virsh.h
> @@ -71,7 +71,8 @@
>       .help = _helpstr \
>      }
>  
> -# define VIRSH_COMMON_OPT_DOMAIN(_helpstr, cflags) \
> +# ifdef WITH_READLINE
> +#  define VIRSH_COMMON_OPT_DOMAIN(_helpstr, cflags) \
>      {.name = "domain", \
>       .type = VSH_OT_DATA, \
>       .flags = VSH_OFLAG_REQ, \
> @@ -79,6 +80,14 @@
>       .completer = virshDomainNameCompleter, \
>       .completer_flags = cflags, \
>      }
> +# else
> +#  define VIRSH_COMMON_OPT_DOMAIN(_helpstr, cflags) \
> +    {.name = "domain", \
> +     .type = VSH_OT_DATA, \
> +     .flags = VSH_OFLAG_REQ, \
> +     .help = _helpstr, \
> +    }
> +# endif
>  
>  # define VIRSH_COMMON_OPT_CONFIG(_helpstr) \
>      {.name = "config", \
> diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> index c86b5763a..ac4d00dd7 100644
> --- a/tools/virt-admin.c
> +++ b/tools/virt-admin.c
> @@ -429,7 +429,9 @@ static const vshCmdOptDef opts_srv_threadpool_info[] = {
>      {.name = "server",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = vshAdmServerCompleter,
> +#endif
>       .help = N_("Server to retrieve threadpool attributes from."),
>      },
>      {.name = NULL}
> @@ -491,7 +493,9 @@ static const vshCmdOptDef opts_srv_threadpool_set[] = {
>      {.name = "server",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = vshAdmServerCompleter,
> +#endif
>       .help = N_("Server to alter threadpool attributes on."),
>      },
>      {.name = "min-workers",
> @@ -598,7 +602,9 @@ static const vshCmdOptDef opts_srv_clients_list[] = {
>      {.name = "server",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = vshAdmServerCompleter,
> +#endif
>       .help = N_("server which to list connected clients from"),
>      },
>      {.name = NULL}
> @@ -680,7 +686,9 @@ static const vshCmdOptDef opts_client_info[] = {
>      {.name = "server",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = vshAdmServerCompleter,
> +#endif
>       .help = N_("server to which <client> is connected to"),
>      },
>      {.name = "client",
> @@ -768,7 +776,9 @@ static const vshCmdOptDef opts_client_disconnect[] = {
>      {.name = "server",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = vshAdmServerCompleter,
> +#endif
>       .help = N_("server which the client is currently connected to"),
>      },
>      {.name = "client",
> @@ -834,7 +844,9 @@ static const vshCmdOptDef opts_srv_clients_info[] = {
>      {.name = "server",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = vshAdmServerCompleter,
> +#endif
>       .help = N_("Server to retrieve the client limits from."),
>      },
>      {.name = NULL}
> @@ -894,7 +906,9 @@ static const vshCmdOptDef opts_srv_clients_set[] = {
>      {.name = "server",
>       .type = VSH_OT_DATA,
>       .flags = VSH_OFLAG_REQ,
> +#ifdef WITH_READLINE
>       .completer = vshAdmServerCompleter,
> +#endif
>       .help = N_("Server to alter the client-related configuration limits on."),
>      },
>      {.name = "max-clients",
> diff --git a/tools/vsh.c b/tools/vsh.c
> index 4426c08d6..59c8a440e 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -3500,7 +3500,7 @@ const vshCmdInfo info_complete[] = {
>  };
>  
>  bool
> -cmdComplete(vshControl *ctl, const vshCmd *cmd)
> +cmdComplete(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd ATTRIBUTE_UNUSED)
>  {
>      bool ret = false;
>  #ifdef WITH_READLINE
> 




More information about the libvir-list mailing list