2:05 p.m.
commit 09778e09 switched from using ioctl(SIOCBRDELBR) for bridge
device deletion to using a netlink RTM_DELLINK message, which is the
more modern way to delete a bridge (and also doesn't require the
bridge to be ~IFF_UP to succeed). However, although older kernels
(e.g. 2.6.32, in RHEL6/CentOS6) support deleting *some* link types
with RTM_NEWLINK, they don't support deleting bridges, and there is no
compile-time way to figure this out.
This patch moves the body of the SIOCBRDELBR version of
virNetDevBridgeDelete() into a static function, calls the new function
from the original, and also calls the new function from the
RTM_DELLINK version if the RTM_DELLINK message generates an EOPNOTSUPP
error. Since RTM_DELLINK is done from the subordinate function
virNetlinkDelLink, which is also called for other purposes (deleting a
macvtap interface), a function pointer called "fallback" has been
added to the arglist of virNetlinkDelLink() - if that arg != NULL, the
provided function will be called when (and only when) RTM_DELLINK
fails with EOPNOTSUPP.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1252780 (part 2)
---
Another idea for doing this I came up with during review of the others...
src/util/virnetdevbridge.c | 41 ++++++++++++++++++++++++++++++-----------
src/util/virnetdevmacvlan.c | 2 +-
src/util/virnetlink.c | 13 +++++++++++--
src/util/virnetlink.h | 5 ++++-
4 files changed, 46 insertions(+), 15 deletions(-)
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ae38901..3b06829 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -558,20 +558,15 @@ int virNetDevBridgeCreate(const char *brname)
*
* Returns 0 in case of success or an errno code in case of failure.
*/
-#if defined(__linux__) && defined(HAVE_LIBNL)
-int virNetDevBridgeDelete(const char *brname)
-{
- /* If netlink is available, use it, as it is successful at
- * deleting a bridge even if it is currently IFF_UP.
- */
- return virNetlinkDelLink(brname);
-}
-#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
-int virNetDevBridgeDelete(const char *brname)
+#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
+static int
+virNetDevBridgeDeleteWithIoctl(const char *brname)
{
int fd = -1;
int ret = -1;
+ ignore_value(virNetDevSetOnline(brname, false));
+
if ((fd = virNetDevSetupControl(NULL, NULL)) < 0)
return -1;
@@ -587,8 +582,32 @@ int virNetDevBridgeDelete(const char *brname)
VIR_FORCE_CLOSE(fd);
return ret;
}
+#endif
+
+
+#if defined(__linux__) && defined(HAVE_LIBNL)
+int
+virNetDevBridgeDelete(const char *brname)
+{
+ /* If netlink is available, use it, as it is successful at
+ * deleting a bridge even if it is currently IFF_UP. fallback to
+ * using ioctl(SIOCBRDELBR) if netlink fails with EOPNOTSUPP.
+ */
+ return virNetlinkDelLink(brname, virNetDevBridgeDeleteWithIoctl);
+}
+
+
+#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
+int
+virNetDevBridgeDelete(const char *brname)
+{
+ return virNetDevBridgeDeleteWithIoctl(brname);
+}
+
+
#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFDESTROY)
-int virNetDevBridgeDelete(const char *brname)
+int
+virNetDevBridgeDelete(const char *brname)
{
int s;
struct ifreq ifr;
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 9c4da1f..954736a 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -220,7 +220,7 @@ virNetDevMacVLanCreate(const char *ifname,
*/
int virNetDevMacVLanDelete(const char *ifname)
{
- return virNetlinkDelLink(ifname);
+ return virNetlinkDelLink(ifname, NULL);
}
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0052ef9..0276522 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -281,6 +281,10 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
* virNetlinkDelLink:
*
* @ifname: Name of the link
+ * @fallback: pointer to an alternate function that will
+ * be called to perform the delete if RTM_DELLINK fails
+ * with EOPNOTSUPP (any other error will simply be treated
+ * as an error).
*
* delete a network "link" (aka interface aka device) with the given
* name. This works for many different types of network devices,
@@ -289,7 +293,7 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
* Returns 0 on success, -1 on fatal error.
*/
int
-virNetlinkDelLink(const char *ifname)
+virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback)
{
int rc = -1;
struct nlmsghdr *resp = NULL;
@@ -325,6 +329,10 @@ virNetlinkDelLink(const char *ifname)
if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
goto malformed_resp;
+ if (-err->error == EOPNOTSUPP && fallback) {
+ rc = fallback(ifname);
+ goto cleanup;
+ }
if (err->error) {
virReportSystemError(-err->error,
_("error destroying network device %s"),
@@ -886,7 +894,8 @@ int virNetlinkCommand(struct nl_msg *nl_msg ATTRIBUTE_UNUSED,
int
-virNetlinkDelLink(const char *ifname ATTRIBUTE_UNUSED)
+virNetlinkDelLink(const char *ifname ATTRIBUTE_UNUSED,
+ virNetlinkDelLinkFallback fallback ATTRIBUTE_UNUSED)
{
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _(unsupported));
return -1;
diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h
index 06c3cd0..0664a7a 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -51,7 +51,10 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
struct nlmsghdr **resp, unsigned int *respbuflen,
uint32_t src_pid, uint32_t dst_pid,
unsigned int protocol, unsigned int groups);
-int virNetlinkDelLink(const char *ifname);
+
+typedef int (*virNetlinkDelLinkFallback)(const char *ifname);
+
+int virNetlinkDelLink(const char *ifname, virNetlinkDelLinkFallback fallback);
int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen);
--
2.1.0