Quoting Stefan Berger (stefanb@xxxxxxxxxxxxxxxxxx):
> 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
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list