[libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()

[Laine Stump]

On 03/24/2015 06:25 PM, John Ferlan wrote:
>
> On 03/23/2015 03:43 PM, Laine Stump wrote:
>> These two functions are identical, so no sense in having the
>> duplication. I resisted the temptation to replace calls to
>> virNetDevMacVLanDelete() with calls to virNetlinkDelLink() just in
>> case some mythical future platform has macvtap devices that aren't
>> managed with netlink (or in case we some day need to do more than just
>> tell the kernel to delete the device).
>> ---
>>  src/util/virnetdevmacvlan.c | 67 ++-------------------------------------------
>>  1 file changed, 2 insertions(+), 65 deletions(-)
>>
>
> hmm interesting... Started reading the next patch and noticed
> something... Perhaps I was quick on the trigger finger for this one...
>
> This module was compiled with "if WITH_MACVTAP", but the API being
> replaced/called uses "#if defined(__linux__) && defined(HAVE_LIBNL)"
> thus this module never cared about linux & HAVE_LIBNL, but now depends
> on an API that does. My reading comprehension of the Makefile in order
> to determine whether anything matters is "limited"...
> Of course this module has libnl calls in it already so perhaps either
> WITH_MACVTAP implies __linux__ and HAVE_LIBNL or perhaps that something
> that needs to be adjusted.

configure assures that HAVE_LIBNL is true if --with-macvtap is
requested, so it's not possible to have WITH_MACVTAP without HAVE_LIBNL.

As for __linux__, I'm not even sure why that got in to begin with. I
don't know that libnl (or netlink) is available on anything that isn't
Linux. Definitely macvtap isn't on anything non-Linux though, so I'd say
we're safe there as well.

>   Not my specialty!

but thankfully you weren't afraid to review it anyway. Thanks!