Re: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
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!