merge: branch 'lr/ovs-failures'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/182

3 files changed, 96 insertions, 31 deletions

diff --git a/src/devices/ovs/nm-ovs-factory.c b/src/devices/ovs/nm-ovs-factory.c
index 2124b2a0b1..fdf07bd375 100644
--- a/src/devices/ovs/nm-ovs-factory.c
+++ b/src/devices/ovs/nm-ovs-factory.c
@@ -26,7 +26,9 @@
 #include "nm-device-ovs-bridge.h"
 #include "platform/nm-platform.h"
 #include "nm-core-internal.h"
+#include "settings/nm-settings.h"
 #include "devices/nm-device-factory.h"
+#include "devices/nm-device-private.h"
 
 /*****************************************************************************/
 
@@ -51,7 +53,13 @@ G_DEFINE_TYPE (NMOvsFactory, nm_ovs_factory, NM_TYPE_DEVICE_FACTORY)
 /*****************************************************************************/
 
 #define _NMLOG_DOMAIN      LOGD_DEVICE
-#define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "ovs", __VA_ARGS__)
+#define _NMLOG(level, ifname, con_uuid, ...) \
+        G_STMT_START { \
+                nm_log ((level), _NMLOG_DOMAIN, (ifname), (con_uuid), \
+                        "ovs: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__) \
+                        _NM_UTILS_MACRO_REST(__VA_ARGS__)); \
+        } G_STMT_END
+
 
 /*****************************************************************************/
 
@@ -139,6 +147,36 @@ ovsdb_device_removed (NMOvsdb *ovsdb, const char *name, NMDeviceType device_type
 }
 
 static void
+ovsdb_interface_failed (NMOvsdb *ovsdb,
+                        const char *name,
+                        const char *connection_uuid,
+                        const char *error,
+                        NMDeviceFactory *self)
+{
+	NMDevice *device = NULL;
+	NMSettingsConnection *connection = NULL;
+
+	_LOGI (name, connection_uuid, "ovs interface \"%s\" (%s) failed: %s", name, connection_uuid, error);
+
+	device = nm_manager_get_device (nm_manager_get (), name, NM_DEVICE_TYPE_OVS_INTERFACE);
+	if (!device)
+		return;
+
+	if (connection_uuid)
+		connection = nm_settings_get_connection_by_uuid (nm_device_get_settings (device), connection_uuid);
+
+	if (connection) {
+		nm_settings_connection_autoconnect_blocked_reason_set (connection,
+		                                                       NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_FAILED,
+		                                                       TRUE);
+	}
+
+	nm_device_state_changed (device,
+	                         NM_DEVICE_STATE_FAILED,
+	                         NM_DEVICE_STATE_REASON_OVSDB_FAILED);
+}
+
+static void
 start (NMDeviceFactory *self)
 {
 	NMOvsdb *ovsdb;
@@ -147,6 +185,7 @@ start (NMDeviceFactory *self)
 
 	g_signal_connect_object (ovsdb, NM_OVSDB_DEVICE_ADDED, G_CALLBACK (ovsdb_device_added), self, (GConnectFlags) 0);
 	g_signal_connect_object (ovsdb, NM_OVSDB_DEVICE_REMOVED, G_CALLBACK (ovsdb_device_removed), self, (GConnectFlags) 0);
+	g_signal_connect_object (ovsdb, NM_OVSDB_INTERFACE_FAILED, G_CALLBACK (ovsdb_interface_failed), self, (GConnectFlags) 0);
 }
 
 static NMDevice *
diff --git a/src/devices/ovs/nm-ovsdb.c b/src/devices/ovs/nm-ovsdb.c
index 5b50f84069..4bb9928758 100644
--- a/src/devices/ovs/nm-ovsdb.c
+++ b/src/devices/ovs/nm-ovsdb.c
@@ -58,7 +58,7 @@ typedef struct {
 enum {
 	DEVICE_ADDED,
 	DEVICE_REMOVED,
-	DEVICE_CHANGED,
+	INTERFACE_FAILED,
 	LAST_SIGNAL
 };
 
@@ -534,9 +534,14 @@ _add_interface (NMOvsdb *self, json_t *params,
 
 			json_array_append_new (ports, json_pack ("[s, s]", "uuid", port_uuid));
 
-			if (   g_strcmp0 (ovs_port->name, nm_connection_get_interface_name (port)) != 0
-			    || g_strcmp0 (ovs_port->connection_uuid, nm_connection_get_uuid (port)) != 0)
+			if (!ovs_port) {
+				/* This would be a violation of ovsdb's reference integrity (a bug). */
+				_LOGW ("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid);
 				continue;
+			} else if (   strcmp (ovs_port->name, nm_connection_get_interface_name (port)) != 0
+			           || g_strcmp0 (ovs_port->connection_uuid, nm_connection_get_uuid (port)) != 0) {
+				continue;
+			}
 
 			for (ii = 0; ii < ovs_port->interfaces->len; ii++) {
 				interface_uuid = g_ptr_array_index (ovs_port->interfaces, ii);
@@ -544,9 +549,13 @@ _add_interface (NMOvsdb *self, json_t *params,
 
 				json_array_append_new (interfaces, json_pack ("[s, s]", "uuid", interface_uuid));
 
-				if (   g_strcmp0 (ovs_interface->name, nm_connection_get_interface_name (interface)) == 0
-				    && g_strcmp0 (ovs_interface->connection_uuid, nm_connection_get_uuid (interface)) == 0)
+				if (!ovs_interface) {
+					/* This would be a violation of ovsdb's reference integrity (a bug). */
+					_LOGW ("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid);
+				} else if (   strcmp (ovs_interface->name, nm_connection_get_interface_name (interface)) == 0
+				           && g_strcmp0 (ovs_interface->connection_uuid, nm_connection_get_uuid (interface)) == 0) {
 					has_interface = TRUE;
+				}
 			}
 
 			break;
@@ -642,16 +651,27 @@ _delete_interface (NMOvsdb *self, json_t *params, const char *ifname)
 
 			interfaces_changed = FALSE;
 
+			if (!ovs_port) {
+				/* This would be a violation of ovsdb's reference integrity (a bug). */
+				_LOGW ("Unknown port '%s' in bridge '%s'", port_uuid, bridge_uuid);
+				continue;
+			}
+
 			for (ii = 0; ii < ovs_port->interfaces->len; ii++) {
 				interface_uuid = g_ptr_array_index (ovs_port->interfaces, ii);
 				ovs_interface = g_hash_table_lookup (priv->interfaces, interface_uuid);
 
 				json_array_append_new (interfaces, json_pack ("[s,s]", "uuid", interface_uuid));
 
-				if (strcmp (ovs_interface->name, ifname) == 0) {
-					/* skip the interface */
-					interfaces_changed = TRUE;
-					continue;
+				if (ovs_interface) {
+					if (strcmp (ovs_interface->name, ifname) == 0) {
+						/* skip the interface */
+						interfaces_changed = TRUE;
+						continue;
+					}
+				} else {
+					/* This would be a violation of ovsdb's reference integrity (a bug). */
+					_LOGW ("Unknown interface '%s' in port '%s'", interface_uuid, port_uuid);
 				}
 
 				json_array_append_new (new_interfaces, json_pack ("[s,s]", "uuid", interface_uuid));
@@ -718,14 +738,14 @@ ovsdb_next_command (NMOvsdb *self)
 		msg = json_pack ("{s:i, s:s, s:[s, n, {"
 		                 "  s:[{s:[s, s, s]}],"
 		                 "  s:[{s:[s, s, s]}],"
-		                 "  s:[{s:[s, s, s]}],"
+		                 "  s:[{s:[s, s, s, s]}],"
 		                 "  s:[{s:[]}]"
 		                 "}]}",
 		                 "id", call->id,
 		                 "method", "monitor", "params", "Open_vSwitch",
 		                 "Bridge", "columns", "name", "ports", "external_ids",
 		                 "Port", "columns", "name", "interfaces", "external_ids",
-		                 "Interface", "columns", "name", "type", "external_ids",
+		                 "Interface", "columns", "name", "type", "external_ids", "error",
 		                 "Open_vSwitch", "columns");
 		break;
 	case OVSDB_ADD_INTERFACE:
@@ -864,21 +884,25 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
 
 	/* Interfaces */
 	json_object_foreach (interface, key, value) {
+		json_t *error = NULL;
 		gboolean old = FALSE;
 		gboolean new = FALSE;
 
 		if (json_unpack (value, "{s:{}}", "old") == 0)
 			old = TRUE;
 
-		if (json_unpack (value, "{s:{s:s, s:s, s:o}}", "new",
+		if (json_unpack (value, "{s:{s:s, s:s, s?:o, s:o}}", "new",
 		                 "name", &name,
 		                 "type", &type,
+		                 "error", &error,
 		                 "external_ids", &external_ids) == 0)
 			new = TRUE;
 
 		if (old) {
 			ovs_interface = g_hash_table_lookup (priv->interfaces, key);
-			if (!new || g_strcmp0 (ovs_interface->name, name) != 0) {
+			if (!ovs_interface) {
+				_LOGW ("Interface '%s' was not seen", key);
+			} else if (!new || strcmp (ovs_interface->name, name) != 0) {
 				old = FALSE;
 				_LOGT ("removed an '%s' interface: %s%s%s",
 				       ovs_interface->type, ovs_interface->name,
@@ -899,12 +923,11 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
 			ovs_interface->name = g_strdup (name);
 			ovs_interface->type = g_strdup (type);
 			ovs_interface->connection_uuid = _connection_uuid_from_external_ids (external_ids);
+			g_hash_table_insert (priv->interfaces, g_strdup (key), ovs_interface);
 			if (old) {
 				_LOGT ("changed an '%s' interface: %s%s%s", type, ovs_interface->name,
 				       ovs_interface->connection_uuid ? ", " : "",
 				       ovs_interface->connection_uuid ?: "");
-				g_signal_emit (self, signals[DEVICE_CHANGED], 0,
-				               "ovs-interface", ovs_interface->name);
 			} else {
 				_LOGT ("added an '%s' interface: %s%s%s",
 				       ovs_interface->type, ovs_interface->name,
@@ -917,7 +940,14 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
 					               ovs_interface->name, NM_DEVICE_TYPE_OVS_INTERFACE);
 				}
 			}
-			g_hash_table_insert (priv->interfaces, g_strdup (key), ovs_interface);
+			/* The error is a string. No error is indicated by an empty set,
+			 * because why the fuck not: [ "set": [] ] */
+			if (error && json_is_string (error)) {
+				g_signal_emit (self, signals[INTERFACE_FAILED], 0,
+				               ovs_interface->name,
+				               ovs_interface->connection_uuid,
+				               json_string_value (error));
+			}
 		}
 	}
 
@@ -954,12 +984,11 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
 			ovs_port->connection_uuid = _connection_uuid_from_external_ids (external_ids);
 			ovs_port->interfaces = g_ptr_array_new_with_free_func (g_free);
 			_uuids_to_array (ovs_port->interfaces, items);
+			g_hash_table_insert (priv->ports, g_strdup (key), ovs_port);
 			if (old) {
 				_LOGT ("changed a port: %s%s%s", ovs_port->name,
 				       ovs_port->connection_uuid ? ", " : "",
 				       ovs_port->connection_uuid ?: "");
-				g_signal_emit (self, signals[DEVICE_CHANGED], 0,
-				               NM_SETTING_OVS_PORT_SETTING_NAME, ovs_port->name);
 			} else {
 				_LOGT ("added a port: %s%s%s", ovs_port->name,
 				       ovs_port->connection_uuid ? ", " : "",
@@ -967,7 +996,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
 				g_signal_emit (self, signals[DEVICE_ADDED], 0,
 				               ovs_port->name, NM_DEVICE_TYPE_OVS_PORT);
 			}
-			g_hash_table_insert (priv->ports, g_strdup (key), ovs_port);
 		}
 	}
 
@@ -1004,12 +1032,11 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
 			ovs_bridge->connection_uuid = _connection_uuid_from_external_ids (external_ids);
 			ovs_bridge->ports = g_ptr_array_new_with_free_func (g_free);
 			_uuids_to_array (ovs_bridge->ports, items);
+			g_hash_table_insert (priv->bridges, g_strdup (key), ovs_bridge);
 			if (old) {
 				_LOGT ("changed a bridge: %s%s%s", ovs_bridge->name,
 				       ovs_bridge->connection_uuid ? ", " : "",
 				       ovs_bridge->connection_uuid ?: "");
-				g_signal_emit (self, signals[DEVICE_CHANGED], 0,
-				               NM_SETTING_OVS_BRIDGE_SETTING_NAME, ovs_bridge->name);
 			} else {
 				_LOGT ("added a bridge: %s%s%s", ovs_bridge->name,
 				       ovs_bridge->connection_uuid ? ", " : "",
@@ -1017,7 +1044,6 @@ ovsdb_got_update (NMOvsdb *self, json_t *msg)
 				g_signal_emit (self, signals[DEVICE_ADDED], 0,
 				               ovs_bridge->name, NM_DEVICE_TYPE_OVS_BRIDGE);
 			}
-			g_hash_table_insert (priv->bridges, g_strdup (key), ovs_bridge);
 		}
 	}
 
@@ -1562,19 +1588,19 @@ nm_ovsdb_class_init (NMOvsdbClass *klass)
 		              G_OBJECT_CLASS_TYPE (object_class),
 		              G_SIGNAL_RUN_LAST,
 		              0, NULL, NULL, NULL,
-		              G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_UINT);
+		              G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_UINT);
 
 	signals[DEVICE_REMOVED] =
 		g_signal_new (NM_OVSDB_DEVICE_REMOVED,
 		              G_OBJECT_CLASS_TYPE (object_class),
 		              G_SIGNAL_RUN_LAST,
 		              0, NULL, NULL, NULL,
-		              G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_UINT);
+		              G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_UINT);
 
-	signals[DEVICE_CHANGED] =
-		g_signal_new (NM_OVSDB_DEVICE_CHANGED,
+	signals[INTERFACE_FAILED] =
+		g_signal_new (NM_OVSDB_INTERFACE_FAILED,
 		              G_OBJECT_CLASS_TYPE (object_class),
 		              G_SIGNAL_RUN_LAST,
 		              0, NULL, NULL, NULL,
-		              G_TYPE_NONE, 2, G_TYPE_POINTER, G_TYPE_UINT);
+		              G_TYPE_NONE, 3, G_TYPE_STRING, G_TYPE_STRING, G_TYPE_STRING);
 }
diff --git a/src/devices/ovs/nm-ovsdb.h b/src/devices/ovs/nm-ovsdb.h
index cf9fe2a21b..279155a498 100644
--- a/src/devices/ovs/nm-ovsdb.h
+++ b/src/devices/ovs/nm-ovsdb.h
@@ -27,9 +27,9 @@
 #define NM_IS_OVSDB_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE ((klass), NM_TYPE_OVSDB))
 #define NM_OVSDB_GET_CLASS(obj)  (G_TYPE_INSTANCE_GET_CLASS ((obj), NM_TYPE_OVSDB, NMOvsdbClass))
 
-#define NM_OVSDB_DEVICE_ADDED   "device-added"
-#define NM_OVSDB_DEVICE_REMOVED "device-removed"
-#define NM_OVSDB_DEVICE_CHANGED "device-changed"
+#define NM_OVSDB_DEVICE_ADDED      "device-added"
+#define NM_OVSDB_DEVICE_REMOVED    "device-removed"
+#define NM_OVSDB_INTERFACE_FAILED  "interface-failed"
 
 typedef struct _NMOvsdb NMOvsdb;
 typedef struct _NMOvsdbClass NMOvsdbClass;