Re: [PATCH 1/3] Remove unused SOL_NETLINK macro

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

 



On 06/21/2016 04:17 AM, Ján Tomko wrote:
On Mon, Jun 20, 2016 at 01:43:30PM -0400, Laine Stump wrote:
On 06/20/2016 11:28 AM, Ján Tomko wrote:
Introduced by commit d575679, unused at the time.
---
  src/util/virnetlink.c | 4 ----
  1 file changed, 4 deletions(-)

diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 5491ece..513f36e 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -42,10 +42,6 @@
  #include "virmacaddr.h"
  #include "virerror.h"

-#ifndef SOL_NETLINK
-# define SOL_NETLINK 270
-#endif
-
  #define VIR_FROM_THIS VIR_FROM_NET

  VIR_LOG_INIT("util.netlink");

Provisional ACK since you are correct that it isn't visibly used, but I
can't help thinking the author had some reason for adding it.

An earlier version of the patchset used it to call:
 setsockopt(fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP,
https://www.redhat.com/archives/libvir-list/2012-August/msg01457.html

In a later version it was changed to nl_socket_add_membership without
removing the macro (since we disable -Wunused-macros by default):
https://www.redhat.com/archives/libvir-list/2012-August/msg01541.html

Okay, so that explains how it got there, and proves that it's unneeded, so the provisional ACK is now full :-)


Sadly, this function is also present in libnl 1.1, which is our minimum
required version, so we can't drop the #ifdev HAVE_LIBNL1 code yet.

I don't understand the relationship. LIBNL1 is used in RHEL6 because for a very long time libnl3 wasn't available there. At some point relatively far into the life of RHEL6 the libnl-3 package was added (but appears to only be used by libiwpm and sssd-common; I had *thought* that NetworkManager in RHEL6 had been the reason for the addition, but I guess I remembered wrong). At any rate it's probably not a good idea to switch even upstream RHEL6/CentOS6 builds (which are likely the only reason left to keep libnl1) over to the RHEL6 libnl-3 library, because it was added so later, is used almost nowhere else, and is such an old version. So unfortunately we will have to keep the libnl1 support around for a long while yet.


It looks
like SOL_NETLINK wasn't present in kernel 2.6 (RHEL6 vintage - although
it is mentioned in a comment of one of the system #includes), but was
there by the time of kernel 3.10 (RHEL7).


It was introduced by commit 9a4595bc7e released in kernel v2.6.14,
so RHEL6 based on 2.6.32 had it from the start.

It may seem that way, but actually isn't. The problem is that whatever file it was originally put in didn't get into the kernel-headers package. So "find /etc -name \*.h | xargs grep SOL_NETLINK" finds nothing, and if you try to use SOL_NETLINK in a RHEL6 build of libvirt with these extra lines removed, it will fail. (Yes, I tried it)

(That's not a reason to not rip out these lines, just illustrating that all is not always as it seems :-)

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]