[libvirt] [PATCH 2/2 (v1)] util: fallback to ioctl(SIOCBRDELBR) if netlink RTM_DELLINK fails

[Laine Stump]

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.

(Ugly Hack Alert: because the RTM_DELLINK message is sent from a
different function (virNetDevDelLink()), not directly from
virNetDevBridgeDelete(), and the other caller of that function needs
to *not* have special behavior on EOPNOTSUPP, virNetlinkDelLink() has
an added "silentEOPNOTSUPP" argument, which is set to true when
virNetlinkDelLink should be silent when encountering EOPNOTSUPP, and
instead return -2 so that the caller can try to recover before logging
an error. This was done to avoid duplicating the entire body of
virNetlinkDelLink() inside virNetDevBridgeDelete(). I am also
preparing a patch that does this duplication, but avoids the tricky
special boolean and return value. If anyone objects in the slightest
to this version, I will gladly use the other. Mostly I wrote this version to illustrate how ugly it would be to eliminate the duplicate code :-))

This resolves a similar issue to that reported in:

  https://bugzilla.redhat.com/show_bug.cgi?id=1252780
---
 src/util/virnetdevbridge.c  | 48 +++++++++++++++++++++++++++++++++++----------
 src/util/virnetdevmacvlan.c |  2 +-
 src/util/virnetlink.c       | 17 ++++++++++++----
 src/util/virnetlink.h       |  2 +-
 4 files changed, 53 insertions(+), 16 deletions(-)

diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
index ae38901..19eaf7b 100644
--- a/src/util/virnetdevbridge.c
+++ b/src/util/virnetdevbridge.c
@@ -558,16 +558,9 @@ 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;
@@ -587,6 +580,41 @@ 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. If it fails
+     * with EOPNOTSUPP, itwill return a special "retry with something
+     * older" code, -2.
+     */
+    int ret;
+
+# if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
+    ret = virNetlinkDelLink(brname, true);
+    if (ret == -2) {
+        ignore_value(virNetDevSetOnline(brname, false));
+        ret = virNetDevBridgeDeleteWithIoctl(brname);
+    }
+# else
+    ret = virNetlinkDelLink(brname, false);
+# endif
+    return ret;
+}
+
+
+#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)
 {
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 213b8eb..48403b3 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, false);
 }
 
 
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c
index 0052ef9..ae7b9c8 100644
--- a/src/util/virnetlink.c
+++ b/src/util/virnetlink.c
@@ -281,15 +281,17 @@ int virNetlinkCommand(struct nl_msg *nl_msg,
  * virNetlinkDelLink:
  *
  * @ifname: Name of the link
+ * @silentEOPNOTSUPP
  *
  * delete a network "link" (aka interface aka device) with the given
  * name. This works for many different types of network devices,
  * including macvtap and bridges.
  *
- * Returns 0 on success, -1 on fatal error.
+ * Returns 0 on success, -1 on fatal error, -2 if EOPNOTSUPP (meaning
+ * the caller will need to retry the operation with an alternate method)
  */
 int
-virNetlinkDelLink(const char *ifname)
+virNetlinkDelLink(const char *ifname, bool silentEOPNOTSUPP)
 {
     int rc = -1;
     struct nlmsghdr *resp = NULL;
@@ -324,7 +326,13 @@ virNetlinkDelLink(const char *ifname)
         err = (struct nlmsgerr *)NLMSG_DATA(resp);
         if (resp->nlmsg_len < NLMSG_LENGTH(sizeof(*err)))
             goto malformed_resp;
-
+        if (err->error == -EOPNOTSUPP && silentEOPNOTSUPP) {
+            /* give the caller a chance to recover without
+             * logging an error.
+             */
+            rc = -2;
+            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,
+                  bool silentEOPNOTSUPP 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..bf9f914 100644
--- a/src/util/virnetlink.h
+++ b/src/util/virnetlink.h
@@ -51,7 +51,7 @@ 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);
+int virNetlinkDelLink(const char *ifname, bool silentEOPNOTSUPP);
 
 int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen);
 
-- 
2.1.0