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@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list