Re: [libvirt] [PATCH 1/1] Support libnl-3 as well as libnl-1
Quoting Stefan Berger (stefanb(a)linux.vnet.ibm.com):
On 04/30/2012 06:59 PM, Serge Hallyn wrote:
>configure.ac:
>Check for libnl-3. If found, find libnl-route-3. If not found,
>do the original check to look for libnl-1.
>
>
[...]
>--- a/src/util/virnetlink.c
>+++ b/src/util/virnetlink.c
>@@ -67,7 +67,11 @@ struct _virNetlinkEventSrvPrivate {
> virMutex lock;
> int eventwatch;
> int netlinkfd;
>+#ifdef HAVE_LIBNL1
> struct nl_handle *netlinknh;
>+#else
>+ struct nl_sock *netlinksock;
>+#endif
Since the two members are treated similarly, could you give these
structure members the same name and with that we could get rid of a
couple of the #ifdef's below. I suppose the major change between v1
and v3 that we are touching upon here is that of nl_handle to
nl_sock.
I could - that's what I was referring to later in the commit message.
I would worry that it would over time not be robust, and would get
all the more confusing if it needed to be unwound at some point.
But, while I think it would be short-sighted to do this just to
shorten the patch, it would also make the flow of the rest of the
code cleaner, so it may be worth it.
For that matter, with a simple wrapper function or two we should be able
to hide the remaining ifdefs, if that's what you'd like.
> /*Events*/
> int handled;
> size_t handlesCount;
>@@ -121,15 +125,31 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
> int fd;
> int n;
> struct nlmsghdr *nlmsg = nlmsg_hdr(nl_msg);
>+#ifdef HAVE_LIBNL1
> struct nl_handle *nlhandle = nl_handle_alloc();
>+#else
>+ struct nl_sock *nlsock = nl_socket_alloc();
>+#endif
>
Also same name here.
>+#ifdef HAVE_LIBNL1
> if (!nlhandle) {
>+#else
>+ if (!nlsock) {
>+#endif
This could then be just one test.
> virReportSystemError(errno,
>+#ifdef HAVE_LIBNL1
> "%s", _("cannot allocate nlhandle for
netlink"));
>+#else
>+ "%s", _("cannot allocate nlsock for
netlink"));
>+#endif
> return -1;
> }
>
>+#ifdef HAVE_LIBNL1
> if (nl_connect(nlhandle, NETLINK_ROUTE)< 0) {
>+#else
>+ if (nl_connect(nlsock, NETLINK_ROUTE)< 0) {
>+#endif
... this one also ...
> virReportSystemError(errno,
> "%s", _("cannot connect to netlink
socket"));
> rc = -1;
>@@ -140,7 +160,11 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
>
> nlmsg->nlmsg_pid = getpid();
>
>+#ifdef HAVE_LIBNL1
> nbytes = nl_send_auto_complete(nlhandle, nl_msg);
>+#else
>+ nbytes = nl_send_auto_complete(nlsock, nl_msg);
>+#endif
as well as this function call and from what I can see pretty much
all of the rest too except for the destroy/free calls.
Regards,
Stefan