[libvirt] [PATCH 23/28] util: make messages consistent in both versions of virNetDevIPRouteAdd()

[John Ferlan]

On 06/22/2016 01:37 PM, Laine Stump wrote:
> ---
>  src/util/virnetdevip.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
> index 6889f81..5daeed5 100644
> --- a/src/util/virnetdevip.c
> +++ b/src/util/virnetdevip.c
> @@ -315,7 +315,6 @@ virNetDevIPRouteAdd(const char *ifname,
>  
>      /* If we have no valid network address, then use the default one */
>      if (!addr || !VIR_SOCKET_ADDR_VALID(addr)) {
> -        VIR_DEBUG("computing default address");
>          int family = VIR_SOCKET_ADDR_FAMILY(gateway);
>          if (family == AF_INET) {
>              if (virSocketAddrParseIPv4(&defaultAddr, VIR_SOCKET_ADDR_IPV4_ALL) < 0)
> @@ -328,9 +327,11 @@ virNetDevIPRouteAdd(const char *ifname,
>          actualAddr = &defaultAddr;
>      }
>  
> -    toStr = virSocketAddrFormat(actualAddr);
> -    viaStr = virSocketAddrFormat(gateway);
> -    VIR_DEBUG("Adding route %s/%d via %s", toStr, prefix, viaStr);
> +    if (!(toStr = virSocketAddrFormat(actualAddr)))
> +        goto cleanup;
> +    if (!(viaStr = virSocketAddrFormat(gateway)))
> +        goto cleanup;
> +    VIR_DEBUG("Adding route to %s/%u via %s on %s", toStr, prefix, viaStr, ifname);
>  
>      if (virNetDevGetIPAddressBinary(actualAddr, &addrData, &addrDataLen) < 0 ||
>          virNetDevGetIPAddressBinary(gateway, &gatewayData, &addrDataLen) < 0)
> @@ -376,7 +377,9 @@ virNetDevIPRouteAdd(const char *ifname,
>          goto cleanup;
>  
>      if ((errCode = virNetlinkGetErrorCode(resp, recvbuflen)) < 0) {
> -        virReportSystemError(errCode, _("Error adding route to %s"), ifname);
> +        virReportSystemError(errCode,
> +                             _("Error adding route to %s/%u via %s on %s"),
> +                             toStr, prefix, viaStr, ifname);
>          goto cleanup;
>      }
>  
> @@ -617,7 +620,8 @@ virNetDevIPRouteAdd(const char *ifname,
>                      unsigned int metric)
>  {
>      virCommandPtr cmd = NULL;
> -    char *addrstr = NULL, *gatewaystr = NULL;
> +    char *toStr = NULL;
> +    char *viaStr = NULL;
>      virSocketAddr defaultAddr;
>      virSocketAddrPtr actualAddr;
>      int ret = -1;
> @@ -638,14 +642,16 @@ virNetDevIPRouteAdd(const char *ifname,
>          actualAddr = &defaultAddr;
>      }
>  
> -    if (!(addrstr = virSocketAddrFormat(actualAddr)))
> +    if (!(toStr = virSocketAddrFormat(actualAddr)))
>          goto cleanup;
> -    if (!(gatewaystr = virSocketAddrFormat(gateway)))
> +    if (!(viaStr = virSocketAddrFormat(gateway)))
>          goto cleanup;
> +    VIR_DEBUG("Adding route to %s/%u via %s on %s", toStr, prefix, viaStr, ifname);
> +
>      cmd = virCommandNew(IP_PATH);
>      virCommandAddArgList(cmd, "route", "add", NULL);
> -    virCommandAddArgFormat(cmd, "%s/%u", addrstr, prefix);
> -    virCommandAddArgList(cmd, "via", gatewaystr, "dev", ifname,
> +    virCommandAddArgFormat(cmd, "%s/%u", toStr, prefix);
> +    virCommandAddArgList(cmd, "via", viaStr, "dev", ifname,
>                                "proto", "static", "metric", NULL);
>      virCommandAddArgFormat(cmd, "%u", metric);
>  

If you add "&exitstatus" as the 2nd parameter to virCommandRun, then you
can also add a message to the virCommandRun failure similar to the error
for the virNetlinkCommand failure.

Additionally adding virCommandSetOutputBuffer and/or
virCommandSetErrorBuffer can really help you become more verbose about
what the failure was.

ACK - with or without the virCommandRun adjustments - just figured I'd
note that this path does have an error path difference w/r/t verbosity
if there's an error that virNetlinkGetErrorCode finds...


John


> @@ -654,8 +660,8 @@ virNetDevIPRouteAdd(const char *ifname,
>  
>      ret = 0;
>   cleanup:
> -    VIR_FREE(addrstr);
> -    VIR_FREE(gatewaystr);
> +    VIR_FREE(toStr);
> +    VIR_FREE(viaStr);
>      virCommandFree(cmd);
>      return ret;
>  }
>