util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails

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)

diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ae389016ba4c1360070d596640d7158c054dbc5f..ef1f4cc42fd106d3e18f8e7359a4974d7b7b86d3 100644 (file)
--- 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,36 @@ 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.
+     */
+# if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
+    return virNetlinkDelLink(brname, virNetDevBridgeDeleteWithIoctl);
+# else
+    return virNetlinkDelLink(brname, NULL);
+# endif
+}
+
+
+#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 213b8eb637e511b6c42560f87bbd00573f424192..3bc3d735ae59dd8e6b45e7fb8a6bc9fde32c98cd 100644 (file)
--- 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 0052ef9970b24c95d6526d4efd005389f2e7c0ba..0276522a2cafa3245bc066802cca01bd7ed2c3fe 100644 (file)
--- 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 06c3cd089f0891df38e53a0d49cac512140f89be..0664a7ac51a07a8ed3b2c45763c1acfd6daa51d0 100644 (file)
--- 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);