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! -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list