[libvirt] [PATCH] openvzDomainSetNetwork: use virCommand

[Ján Tomko]

On 05/19/2013 11:52 AM, Michal Privoznik wrote:
> Currently, the openvzDomainSetNetwork function constructs an
> array of strings representing a command line for VZCTL binary.
> This is a overkill since our virCommand APIs can cover all the
> functionality.
> ---
>  src/openvz/openvz_driver.c | 56 ++++++++++------------------------------------
>  1 file changed, 12 insertions(+), 44 deletions(-)
> 
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index 129e328..db41d72 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c

snip snip snip

> @@ -922,37 +892,35 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid,
>          if (!(opt = virBufferContentAndReset(&buf)))
>              goto no_memory;
>  
> -        ADD_ARG_LIT(opt) ;
> +        /* --netif_add ifname[,mac,host_ifname,host_mac] */
> +        virCommandAddArgList(cmd, "--netif_add", opt, NULL);
>          VIR_FREE(opt);
>      } else if (net->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
>                net->data.ethernet.ipaddr != NULL) {
>          /* --ipadd ip */
> -        ADD_ARG_LIT("--ipadd") ;
> -        ADD_ARG_LIT(net->data.ethernet.ipaddr) ;
> +        virCommandAddArgList(cmd, "--ipadd", net->data.ethernet.ipaddr, NULL);
>      }
>  
>      /* TODO: processing NAT and physical device */
>  
> -    if (prog[0] != NULL) {
> -        ADD_ARG_LIT("--save");
> -        if (virRun(prog, NULL) < 0) {
> +    if (cmd) {

cmd may be NULL here on OOM too.

> +        virCommandAddArg(cmd, "--save");
> +        if (virCommandRun(cmd, NULL) < 0) {
>             rc = -1;
>             goto exit;

Redundant goto (unless you initialize rc to -1 and use it to skip over
"rc = 0").

>          }
>      }
>  
>   exit:
> -    cmdExecFree(prog);
> +    virCommandFree(cmd);
>      return rc;
>  
>   no_memory:
>      VIR_FREE(opt);

opt is NULL here.

>      virReportError(VIR_ERR_INTERNAL_ERROR,
>                     _("Could not put argument to %s"), VZCTL);

I think a standard "out of memory" message would be more fitting here.
You could also rename 'exit' to 'cleanup' and get rid of the 'no_memory' label
completely.

Jan