Re: [libvirt] [PATCH 1/3] Remove unused SOL_NETLINK macro
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 :-)