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! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
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. Not my specialty! John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
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(-) > ACK - John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] util: replace body of virNetDevMacVLanDelete() with virNetlinkDelLink()
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(-)
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index ec959a9..5fd2097 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2014 Red Hat, Inc.
+ * Copyright (C) 2010-2015 Red Hat, Inc.
* Copyright (C) 2010-2012 IBM Corporation
*
* This library is free software; you can redistribute it and/or
@@ -220,70 +220,7 @@ virNetDevMacVLanCreate(const char *ifname,
*/
int virNetDevMacVLanDelete(const char *ifname)
{
-int rc = -1;
-struct nlmsghdr *resp = NULL;
-struct nlmsgerr *err;
-struct ifinfomsg ifinfo = { .ifi_family = AF_UNSPEC };
-unsigned int recvbuflen;
-struct nl_msg *nl_msg;
-
-nl_msg = nlmsg_alloc_simple(RTM_DELLINK,
-NLM_F_REQUEST | NLM_F_CREATE | NLM_F_EXCL);
-if (!nl_msg) {
-virReportOOMError();
-return -1;
-}
-
-if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
-goto buffer_too_small;
-
-if (nla_put(nl_msg, IFLA_IFNAME, strlen(ifname)+1, ifname) < 0)
-goto buffer_too_small;
-
-if (virNetlinkCommand(nl_msg, &resp, &recvbuflen, 0, 0,
- NETLINK_ROUTE, 0) < 0) {
-goto cleanup;
-}
-
-if (recvbuflen < NLMSG_LENGTH(0) || resp == NULL)
-goto malformed_resp;
-
-switch (resp->nlmsg_type) {
-case NLMSG_ERROR:
-err = (struct nlmsgerr *)NLMSG_DATA(resp);
-if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
-goto malformed_resp;
-
-if (err->error) {
-virReportSystemError(-err->error,
- _("error destroying %s interface"),
- ifname);
-goto cleanup;
-}
-break;
-
-case NLMSG_DONE:
-break;
-
-default:
-goto malformed_resp;
-}
-
-rc = 0;
- cleanup:
-nlmsg_free(nl_msg);
-VIR_FREE(resp);
-return rc;
-
- malformed_resp:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("malformed netlink response message"));
-goto cleanup;
-
- buffer_too_small:
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("allocated netlink buffer is too small"));
-goto cleanup;
+return virNetlinkDelLink(ifname);
}
--
2.1.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
