[libvirt] [PATCH 1/3] Remove unused SOL_NETLINK macro

[Laine Stump]

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 :-)