10:47 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.
(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