Re: [PATCH v2 09/13] virsh: Add mac completion to net-dhcp-leases command

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

On 11/10/20 10:51 AM, Lin Ma wrote:
> Signed-off-by: Lin Ma <lma@xxxxxxxx>
> ---
>   tools/virsh-completer-network.c | 44 +++++++++++++++++++++++++++++++++
>   tools/virsh-completer-network.h |  4 +++
>   tools/virsh-network.c           | 15 +++++++----
>   3 files changed, 58 insertions(+), 5 deletions(-)
>
> diff --git a/tools/virsh-completer-network.c b/tools/virsh-completer-network.c
> index 73f7115ab2..3094b9d5d6 100644
> --- a/tools/virsh-completer-network.c
> +++ b/tools/virsh-completer-network.c
> @@ -174,3 +174,47 @@ virshNetworkUUIDCompleter(vshControl *ctl,
>       g_free(nets);
>       return ret;
>   }
> +
> +
> +char **
> +virshNetworkDhcpMacCompleter(vshControl *ctl,
> +                             const vshCmd *cmd,
> +                             unsigned int flags)
> +{
> +    virshControlPtr priv = ctl->privData;
> +    virNetworkDHCPLeasePtr *leases = NULL;
> +    virNetworkPtr network = NULL;
> +    int nleases;
> +    size_t i = 0;
> +    char **ret = NULL;
> +    VIR_AUTOSTRINGLIST tmp = NULL;
> +
> +    virCheckFlags(0, NULL);
> +
> +    if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
> +        return NULL;
> +
> +    if (!(network = virshCommandOptNetwork(ctl, cmd, NULL)))
> +        return NULL;
> +
> +    if ((nleases = virNetworkGetDHCPLeases(network, NULL, &leases, flags)) < 0)
> +        goto cleanup;
> +
> +    tmp = g_new0(char *, nleases + 1);
> +
> +    for (i = 0; i < nleases; i++) {
> +        virNetworkDHCPLeasePtr lease = leases[i];
> +        tmp[i] = g_strdup(lease->mac);
> +    }
> +
> +    ret = g_steal_pointer(&tmp);
> +
> + cleanup:
> +    if (leases) {
> +        for (i = 0; i < nleases; i++)
> +            virNetworkDHCPLeaseFree(leases[i]);
> +        VIR_FREE(leases);
> +    }
> +    virNetworkFree(network);
> +    return ret;
> +}
> diff --git a/tools/virsh-completer-network.h b/tools/virsh-completer-network.h
> index 8910e4525c..80df5c468e 100644
> --- a/tools/virsh-completer-network.h
> +++ b/tools/virsh-completer-network.h
> @@ -37,3 +37,7 @@ char ** virshNetworkPortUUIDCompleter(vshControl *ctl,
>   char ** virshNetworkUUIDCompleter(vshControl *ctl,
>                                     const vshCmd *cmd,
>                                     unsigned int flags);
> +
> +char ** virshNetworkDhcpMacCompleter(vshControl *ctl,
> +                                     const vshCmd *cmd,
> +                                     unsigned int flags);
> diff --git a/tools/virsh-network.c b/tools/virsh-network.c
> index 745afc537d..30b231f7d6 100644
> --- a/tools/virsh-network.c
> +++ b/tools/virsh-network.c
> @@ -61,6 +61,15 @@
>        .completer_flags = cflags, \
>       }
>   
> +#define VIRSH_COMMON_OPT_NETWORK_DHCP_MAC(_helpstr, cflags) \
> +    {.name = "mac", \
> +     .type = VSH_OT_STRING, \
> +     .flags = VSH_OFLAG_NONE, \
> +     .help = _helpstr, \
> +     .completer = virshNetworkDhcpMacCompleter, \
> +     .completer_flags = cflags, \
> +    }
> +

You don't seem to use this macro anywhere else. I think we can just add 
.completer ...

>   
>   virNetworkPtr
>   virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd,
> @@ -1373,11 +1382,7 @@ static const vshCmdInfo info_network_dhcp_leases[] = {
>   
>   static const vshCmdOptDef opts_network_dhcp_leases[] = {
>       VIRSH_COMMON_OPT_NETWORK_FULL(VIR_CONNECT_LIST_NETWORKS_ACTIVE),
> -    {.name = "mac",
> -     .type = VSH_OT_STRING,
> -     .flags = VSH_OFLAG_NONE,
> -     .help = N_("MAC address")

.. here.

> -    },
> +    VIRSH_COMMON_OPT_NETWORK_DHCP_MAC(N_("MAC address"), 0),
>       {.name = NULL}
>   };
>   

Michal