[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2] macvtap: log an error if on failure to connect to netlink socket



On 03/16/2011 10:02 AM, Laine Stump wrote:
> (v2 change: add log messages for 2 other previously silent error conditions in nlComm)
>  
> A bug in libnl (see https://bugzilla.redhat.com/show_bug.cgi?id=677724
> and https://bugzilla.redhat.com/show_bug.cgi?id=677725) makes it very
> easy to create a failure to connect to the netlink socket when trying
> to open a macvtap network device ("type='direct'" in domain interface
> XML). When that error occurred (during a call to libnl's nl_connect()
> from libvirt's nlComm(), there was no log message, leading virsh (for
> example) to report "unknown error".
> 
> There were two other cases in nlComm where an error in a libnl
> function might return with failure but no error reported. In all three
> cases, this patch logs a message which will hopefully be more useful.
> 
> Note that more detailed information about the failure might be
> available from libnl's nl_geterror() function, but it calls
> strerror(), which is not threadsafe, so we can't use it.

Have we opened (yet another) libnl bug about that bad use of strerror()?

> ---
>  src/util/macvtap.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/src/util/macvtap.c b/src/util/macvtap.c
> index a71db86..00ee4ee 100644
> --- a/src/util/macvtap.c
> +++ b/src/util/macvtap.c
> @@ -123,10 +123,15 @@ int nlComm(struct nl_msg *nl_msg,
>      struct nl_handle *nlhandle = nl_handle_alloc();
>      struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg);

Swap these two lines, since nlmsg_hdr() might change errno...

>  
> -    if (!nlhandle)
> +    if (!nlhandle) {
> +        virReportSystemError(errno,
> +                             "%s", _("cannot allocate nlhandle for netlink"));

but you wanted the errno from nl_handle_alloc.

>          return -1;
> +    }
>  
>      if (nl_connect(nlhandle, NETLINK_ROUTE) < 0) {
> +        virReportSystemError(errno,
> +                             "%s", _("cannot connect to netlink socket"));
>          rc = -1;
>          goto err_exit;
>      }
> @@ -161,9 +166,11 @@ int nlComm(struct nl_msg *nl_msg,
>      }
>  
>      *respbuflen = nl_recv(nlhandle, &nladdr, respbuf, NULL);
> -    if (*respbuflen <= 0)
> +    if (*respbuflen <= 0) {
> +        virReportSystemError(errno,
> +                             "%s", _("nl_recv failed"));
>          rc = -1;
> -
> +    }
>  err_exit:
>      if (rc == -1) {
>          VIR_FREE(*respbuf);

ACK with that nit fixed.

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]