team: merge branch 'bg/team-cloned-mac-rh1424641'

https://bugzilla.redhat.com/show_bug.cgi?id=1424641

5 files changed, 202 insertions, 35 deletions

diff --git a/Makefile.am b/Makefile.am
index 670fb9d54e..8576215744 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -491,8 +491,11 @@ dflt_cppflags_libnm_core = \
 	-I$(srcdir)/libnm-core \
 	-I$(builddir)/libnm-core \
 	$(CODE_COVERAGE_CFLAGS) \
-	$(GLIB_CFLAGS) \
-	$(JANSSON_CFLAGS)
+	$(GLIB_CFLAGS)
+
+if WITH_JANSSON
+dflt_cppflags_libnm_core += $(JANSSON_CFLAGS)
+endif
 
 if WITH_GNUTLS
 dflt_cppflags_libnm_core += $(GNUTLS_CFLAGS)
@@ -551,8 +554,11 @@ nodist_libnm_core_libnm_core_la_SOURCES = \
 
 libnm_core_libnm_core_la_LIBADD = \
 	$(GLIB_LIBS) \
-	$(UUID_LIBS) \
-	$(JANSSON_LIBS)
+	$(UUID_LIBS)
+
+if WITH_JANSSON
+libnm_core_libnm_core_la_LIBADD += $(JANSSON_LIBS)
+endif
 
 libnm_core_libnm_core_la_LDFLAGS = \
 	$(CODE_COVERAGE_LDFLAGS)
@@ -2566,6 +2572,7 @@ src_devices_team_libnm_device_plugin_team_la_CPPFLAGS = \
 	-DG_LOG_DOMAIN=\""NetworkManager"\" \
 	-DNETWORKMANAGER_COMPILATION=NM_NETWORKMANAGER_COMPILATION_INSIDE_DAEMON \
 	$(LIBTEAMDCTL_CFLAGS) \
+	$(JANSSON_CFLAGS) \
 	$(GLIB_CFLAGS)
 
 src_devices_team_libnm_device_plugin_team_la_LDFLAGS = \
@@ -2575,6 +2582,7 @@ src_devices_team_libnm_device_plugin_team_la_LDFLAGS = \
 src_devices_team_libnm_device_plugin_team_la_LIBADD = \
 	introspection/libnmdbus.la \
 	$(LIBTEAMDCTL_LIBS) \
+	$(JANSSON_LIBS) \
 	$(GLIB_LIBS) \
 	$(GUDEV_LIBS)
 
diff --git a/configure.ac b/configure.ac
index 25af0beeb2..354db306e8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -547,12 +547,23 @@ PKG_CHECK_MODULES(LIBNL, libnl-3.0 >= 3.2.8)
 PKG_CHECK_MODULES(UUID, uuid)
 
 # Teamd control checks
+
+PKG_CHECK_MODULES(JANSSON, [jansson], [have_jansson=yes], [have_jansson=no])
 PKG_CHECK_MODULES(LIBTEAMDCTL, [libteamdctl >= 1.9], [have_teamdctl=yes],[have_teamdctl=no])
+if test "$have_jansson" = "yes" -a "$have_teamdctl" = "yes"; then
+	have_team_prereq=yes
+else
+	have_team_prereq=no
+fi
+
 AC_ARG_ENABLE(teamdctl, AS_HELP_STRING([--enable-teamdctl], [enable Teamd control support]),
-                     [enable_teamdctl=${enableval}], [enable_teamdctl=${have_teamdctl}])
+                     [enable_teamdctl=${enableval}], [enable_teamdctl=${have_team_prereq}])
 if (test "${enable_teamdctl}" = "yes"); then
-	if test x"$have_teamdctl" = x"no"; then
-		AC_MSG_ERROR(Teamd control is required)
+	if test "$have_teamdctl" = "no"; then
+		AC_MSG_ERROR(Libteamdctl is required for team support)
+	fi
+	if test "$have_jansson" = "no"; then
+		AC_MSG_ERROR(Jansson is required for team support)
 	fi
 	# temporary bug workaround
 	LIBTEAMDCTL_CFLAGS=`echo $LIBTEAMDCTL_CFLAGS | sed -e 's:/teamdctl.h::'`
@@ -563,14 +574,18 @@ fi
 AM_CONDITIONAL(WITH_TEAMDCTL, test "${enable_teamdctl}" = "yes")
 
 # Jansson for team configuration validation
-AC_ARG_ENABLE(json-validation, AS_HELP_STRING([--enable-json-validation], [Enable JSON validation in libnm]))
+AC_ARG_ENABLE(json-validation, AS_HELP_STRING([--enable-json-validation], [Enable JSON validation in libnm]),
+			       [enable_json_validation=${enableval}],
+			       [enable_json_validation=${have_jansson}])
 if (test "${enable_json_validation}" == "no"); then
     AC_DEFINE(WITH_JANSSON, 0, [Define if JANSSON is enabled])
 else
-    PKG_CHECK_MODULES(JANSSON, jansson,,
-                      AC_MSG_ERROR([jansson is needed for team configuration validation. Use --disable-json-validation to build without it.]))
+    if test "$have_jansson" = "no"; then
+        AC_MSG_ERROR([jansson is needed for team configuration validation. Use --disable-json-validation to build without it.])
+    fi
     AC_DEFINE(WITH_JANSSON, 1, [Define if JANSSON is enabled])
 fi
+AM_CONDITIONAL(WITH_JANSSON, test "${enable_json_validation}" != "no")
 
 # we usually compile with polkit support. --enable-polkit=yes|no only sets the
 # default configuration for main.auth-polkit. User can always enable/disable polkit
@@ -1237,6 +1252,6 @@ echo "  valgrind: $with_valgrind   $with_valgrind_suppressions"
 echo "  code coverage: $enable_code_coverage"
 echo "  LTO: $enable_lto"
 echo "  linker garbage collection: $enable_ld_gc"
-echo "  JSON validation: $enable_json_validation"
+echo "  JSON validation for libnm: $enable_json_validation"
 echo "  sanitizers: $sanitizers"
 echo
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 95a5004ada..d12bdf752c 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -12733,49 +12733,92 @@ nm_device_hw_addr_set (NMDevice *self,
 	return _hw_addr_set (self, addr, "set", detail);
 }
 
-gboolean
-nm_device_hw_addr_set_cloned (NMDevice *self, NMConnection *connection, gboolean is_wifi)
+/*
+ * _hw_addr_get_cloned:
+ * @self: a #NMDevice
+ * @connection: a #NMConnection
+ * @is_wifi: whether the device is Wi-Fi
+ * @preserve: (out): whether the address must be reset to initial one
+ * @hwaddr: (out): the cloned MAC address to set on interface
+ * @hwaddr_type: (out): the type of address to set
+ * @hwaddr_detail: (out): the detail (origin) of address to set
+ * @error: (out): on return, an error or %NULL
+ *
+ * Computes the MAC to be set on a interface. On success, one of the
+ * following exclusive conditions are verified:
+ *
+ *  - @preserve is %TRUE: the address must be reset to the initial one
+ *  - @hwaddr is not %NULL: the given address must be set on the device
+ *  - @hwaddr is %NULL and @preserve is %FALSE: no action needed
+ *
+ * Returns: %FALSE in case of error in determining the cloned MAC address,
+ * %TRUE otherwise
+ */
+static gboolean
+_hw_addr_get_cloned (NMDevice *self, NMConnection *connection, gboolean is_wifi,
+                     gboolean *preserve, char **hwaddr, HwAddrType *hwaddr_type,
+                     char **hwaddr_detail, GError **error)
 {
 	NMDevicePrivate *priv;
-	gs_free char *hw_addr_tmp = NULL;
+	gs_free char *addr_setting_free = NULL;
 	gs_free char *hw_addr_generated = NULL;
 	gs_free char *generate_mac_address_mask_tmp = NULL;
 	const char *addr, *addr_setting;
+	char *addr_out;
+	HwAddrType type_out;
 
 	g_return_val_if_fail (NM_IS_DEVICE (self), FALSE);
+	g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE);
+	g_return_val_if_fail (!error || !*error, FALSE);
 
 	priv = NM_DEVICE_GET_PRIVATE (self);
 
 	if (!connection)
 		g_return_val_if_reached (FALSE);
 
-	addr = addr_setting = _get_cloned_mac_address_setting (self, connection, is_wifi, &hw_addr_tmp);
+	addr = addr_setting = _get_cloned_mac_address_setting (self, connection, is_wifi, &addr_setting_free);
 
 	if (nm_streq (addr, NM_CLONED_MAC_PRESERVE)) {
 		/* "preserve" means to reset the initial MAC address. */
-		return nm_device_hw_addr_reset (self, addr_setting);
+		NM_SET_OUT (preserve, TRUE);
+		NM_SET_OUT (hwaddr, NULL);
+		NM_SET_OUT (hwaddr_type, HW_ADDR_TYPE_UNSET);
+		NM_SET_OUT (hwaddr_detail, g_steal_pointer (&addr_setting_free) ?: g_strdup (addr_setting));
+		return TRUE;
 	}
 
 	if (nm_streq (addr, NM_CLONED_MAC_PERMANENT)) {
 		addr = nm_device_get_permanent_hw_address (self);
-		if (!addr)
+		if (!addr) {
+			g_set_error_literal (error,
+			                     NM_DEVICE_ERROR,
+			                     NM_DEVICE_ERROR_FAILED,
+			                     "failed to retrieve permanent address");
 			return FALSE;
-		priv->hw_addr_type = HW_ADDR_TYPE_PERMANENT;
+		}
+		addr_out = g_strdup (addr);
+		type_out = HW_ADDR_TYPE_PERMANENT;
 	} else if (NM_IN_STRSET (addr, NM_CLONED_MAC_RANDOM)) {
 		if (priv->hw_addr_type == HW_ADDR_TYPE_GENERATED) {
 			/* hm, we already use a generate MAC address. Most certainly, that is from the same
 			 * activation request, so we should not create a new random address, instead keep
 			 * the current. */
-			return TRUE;
+			goto out_no_action;
 		}
 		hw_addr_generated = nm_utils_hw_addr_gen_random_eth (nm_device_get_initial_hw_address (self),
-		                                                     _get_generate_mac_address_mask_setting (self, connection, is_wifi, &generate_mac_address_mask_tmp));
+		                                                     _get_generate_mac_address_mask_setting (self, connection,
+		                                                                                             is_wifi,
+		                                                                                             &generate_mac_address_mask_tmp));
 		if (!hw_addr_generated) {
-			_LOGW (LOGD_DEVICE, "set-hw-addr: failed to generate %s MAC address", "random");
+			g_set_error (error,
+			             NM_DEVICE_ERROR,
+			             NM_DEVICE_ERROR_FAILED,
+			             "failed to generate %s MAC address", "random");
 			return FALSE;
 		}
-		priv->hw_addr_type = HW_ADDR_TYPE_GENERATED;
-		addr = hw_addr_generated;
+
+		addr_out = g_steal_pointer (&hw_addr_generated);
+		type_out = HW_ADDR_TYPE_GENERATED;
 	} else if (NM_IN_STRSET (addr, NM_CLONED_MAC_STABLE)) {
 		NMUtilsStableType stable_type;
 		const char *stable_id;
@@ -12783,7 +12826,7 @@ nm_device_hw_addr_set_cloned (NMDevice *self, NMConnection *connection, gboolean
 		if (priv->hw_addr_type == HW_ADDR_TYPE_GENERATED) {
 			/* hm, we already use a generate MAC address. Most certainly, that is from the same
 			 * activation request, so let's skip creating the stable address anew. */
-			return TRUE;
+			goto out_no_action;
 		}
 
 		stable_id = _get_stable_id (self, connection, &stable_type);
@@ -12794,19 +12837,72 @@ nm_device_hw_addr_set_cloned (NMDevice *self, NMConnection *connection, gboolean
 			                                                     _get_generate_mac_address_mask_setting (self, connection, is_wifi, &generate_mac_address_mask_tmp));
 		}
 		if (!hw_addr_generated) {
-			_LOGW (LOGD_DEVICE, "set-hw-addr: failed to generate %s MAC address", "stable");
+			g_set_error (error,
+			             NM_DEVICE_ERROR,
+			             NM_DEVICE_ERROR_FAILED,
+			             "failed to generate %s MAC address", "stable");
 			return FALSE;
 		}
-		priv->hw_addr_type = HW_ADDR_TYPE_GENERATED;
-		addr = hw_addr_generated;
+
+		addr_out = g_steal_pointer (&hw_addr_generated);
+		type_out = HW_ADDR_TYPE_GENERATED;
 	} else {
 		/* this must be a valid address. Otherwise, we shouldn't come here. */
 		if (!nm_utils_hwaddr_valid (addr, -1))
 			g_return_val_if_reached (FALSE);
-		priv->hw_addr_type = HW_ADDR_TYPE_EXPLICIT;
+
+		addr_out = g_strdup (addr);
+		type_out = HW_ADDR_TYPE_EXPLICIT;
 	}
 
-	return _hw_addr_set (self, addr, "set-cloned", addr_setting);
+	NM_SET_OUT (preserve, FALSE);
+	NM_SET_OUT (hwaddr, addr_out);
+	NM_SET_OUT (hwaddr_type, type_out);
+	NM_SET_OUT (hwaddr_detail, g_steal_pointer (&addr_setting_free) ?: g_strdup (addr_setting));
+	return TRUE;
+out_no_action:
+	NM_SET_OUT (preserve, FALSE);
+	NM_SET_OUT (hwaddr, NULL);
+	NM_SET_OUT (hwaddr_type, HW_ADDR_TYPE_UNSET);
+	NM_SET_OUT (hwaddr_detail, NULL);
+	return TRUE;
+}
+
+gboolean
+nm_device_hw_addr_get_cloned (NMDevice *self, NMConnection *connection, gboolean is_wifi,
+                              char **hwaddr, gboolean *preserve, GError **error)
+{
+	if (!_hw_addr_get_cloned (self, connection, is_wifi, preserve, hwaddr, NULL, NULL, error))
+		return FALSE;
+
+	return TRUE;
+}
+
+gboolean
+nm_device_hw_addr_set_cloned (NMDevice *self, NMConnection *connection, gboolean is_wifi)
+{
+	NMDevicePrivate *priv;
+	gboolean preserve = FALSE;
+	gs_free char *hwaddr = NULL;
+	gs_free char *detail = NULL;
+	HwAddrType type = HW_ADDR_TYPE_UNSET;
+	gs_free_error GError *error = NULL;
+
+	g_return_val_if_fail (NM_IS_DEVICE (self), FALSE);
+	priv = NM_DEVICE_GET_PRIVATE (self);
+
+	if (!_hw_addr_get_cloned (self, connection, is_wifi, &preserve, &hwaddr, &type, &detail, &error)) {
+		_LOGW (LOGD_DEVICE, "set-hw-addr: %s", error->message);
+		return FALSE;
+	}
+
+	if (preserve)
+		return nm_device_hw_addr_reset (self, detail);
+
+	if (hwaddr)
+		return _hw_addr_set (self, hwaddr, "set-cloned", detail);
+
+	return TRUE;
 }
 
 gboolean
diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h
index a6ceb703ab..955edcd7da 100644
--- a/src/devices/nm-device.h
+++ b/src/devices/nm-device.h
@@ -641,5 +641,11 @@ void nm_device_update_initial_hw_address (NMDevice *self);
 void nm_device_update_permanent_hw_address (NMDevice *self, gboolean force_freeze);
 void nm_device_update_dynamic_ip_setup (NMDevice *self);
 guint nm_device_get_supplicant_timeout (NMDevice *self);
+gboolean nm_device_hw_addr_get_cloned (NMDevice *self,
+                                       NMConnection *connection,
+                                       gboolean is_wifi,
+                                       char **hwaddr,
+                                       gboolean *preserve,
+                                       GError **error);
 
 #endif /* __NETWORKMANAGER_DEVICE_H__ */
diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c
index a3f621f467..5a94e76df2 100644
--- a/src/devices/team/nm-device-team.c
+++ b/src/devices/team/nm-device-team.c
@@ -28,6 +28,7 @@
 #include <sys/wait.h>
 #include <teamdctl.h>
 #include <stdlib.h>
+#include <jansson.h>
 
 #include "NetworkManagerUtils.h"
 #include "devices/nm-device-private.h"
@@ -72,7 +73,7 @@ G_DEFINE_TYPE (NMDeviceTeam, nm_device_team, NM_TYPE_DEVICE)
 
 /*****************************************************************************/
 
-static gboolean teamd_start (NMDevice *device, NMSettingTeam *s_team);
+static gboolean teamd_start (NMDevice *device, NMConnection *connection);
 
 /*****************************************************************************/
 
@@ -449,7 +450,7 @@ teamd_dbus_vanished (GDBusConnection *dbus_connection,
 		NMConnection *connection = nm_device_get_applied_connection (device);
 
 		g_assert (connection);
-		if (!teamd_start (device, nm_connection_get_setting_team (connection)))
+		if (!teamd_start (device, connection))
 			nm_device_state_changed (device, NM_DEVICE_STATE_FAILED, NM_DEVICE_STATE_REASON_TEAMD_CONTROL_FAILED);
 	}
 }
@@ -513,7 +514,7 @@ teamd_kill (NMDeviceTeam *self, const char *teamd_binary, GError **error)
 }
 
 static gboolean
-teamd_start (NMDevice *device, NMSettingTeam *s_team)
+teamd_start (NMDevice *device, NMConnection *connection)
 {
 	NMDeviceTeam *self = NM_DEVICE_TEAM (device);
 	NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self);
@@ -523,6 +524,12 @@ teamd_start (NMDevice *device, NMSettingTeam *s_team)
 	gs_free char *tmp_str = NULL;
 	const char *teamd_binary;
 	const char *config;
+	nm_auto_free const char *config_free = NULL;
+	NMSettingTeam *s_team;
+	gs_free char *cloned_mac = NULL;
+
+	s_team = nm_connection_get_setting_team (connection);
+	g_return_val_if_fail (s_team, FALSE);
 
 	teamd_binary = nm_utils_find_helper ("teamd", NULL, NULL);
 	if (!teamd_binary) {
@@ -548,7 +555,39 @@ teamd_start (NMDevice *device, NMSettingTeam *s_team)
 	g_ptr_array_add (argv, (gpointer) "-t");
 	g_ptr_array_add (argv, (gpointer) iface);
 
-	config = nm_setting_team_get_config(s_team);
+	config = nm_setting_team_get_config (s_team);
+	if (!nm_device_hw_addr_get_cloned (device, connection, FALSE, &cloned_mac, NULL, &error)) {
+		_LOGW (LOGD_DEVICE, "set-hw-addr: %s", error->message);
+		return FALSE;
+	}
+
+	if (cloned_mac) {
+		json_t *json, *hwaddr;
+		json_error_t jerror;
+
+		/* Inject the hwaddr property into the JSON configuration.
+		 * While doing so, detect potential conflicts */
+
+		json = json_loads (config ?: "{}", 0, &jerror);
+		g_return_val_if_fail (json, FALSE);
+
+		hwaddr = json_object_get (json, "hwaddr");
+		if (hwaddr) {
+			if (   !json_is_string (hwaddr)
+			    || !nm_streq0 (json_string_value (hwaddr), cloned_mac))
+				_LOGW (LOGD_TEAM, "set-hw-addr: can't set team cloned-mac-address as the JSON configuration already contains \"hwaddr\"");
+		} else {
+			hwaddr = json_string (cloned_mac);
+			json_object_set (json, "hwaddr", hwaddr);
+			config = config_free = json_dumps (json, JSON_INDENT(0) |
+			                                         JSON_ENSURE_ASCII |
+			                                         JSON_SORT_KEYS);
+			_LOGD (LOGD_TEAM, "set-hw-addr: injected \"hwaddr\" \"%s\" into team configuration", cloned_mac);
+			json_decref (hwaddr);
+		}
+		json_decref (json);
+	}
+
 	if (config) {
 		g_ptr_array_add (argv, (gpointer) "-c");
 		g_ptr_array_add (argv, (gpointer) config);
@@ -587,13 +626,16 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
 	NMActStageReturn ret = NM_ACT_STAGE_RETURN_SUCCESS;
 	gs_free_error GError *error = NULL;
 	NMSettingTeam *s_team;
+	NMConnection *connection;
 	const char *cfg;
 
 	ret = NM_DEVICE_CLASS (nm_device_team_parent_class)->act_stage1_prepare (device, out_failure_reason);
 	if (ret != NM_ACT_STAGE_RETURN_SUCCESS)
 		return ret;
 
-	s_team = (NMSettingTeam *) nm_device_get_applied_setting (device, NM_TYPE_SETTING_TEAM);
+	connection = nm_device_get_applied_connection (device);
+	g_return_val_if_fail (connection, NM_ACT_STAGE_RETURN_FAILURE);
+	s_team = nm_connection_get_setting_team (connection);
 	g_return_val_if_fail (s_team, NM_ACT_STAGE_RETURN_FAILURE);
 
 	if (priv->tdc) {
@@ -621,7 +663,7 @@ act_stage1_prepare (NMDevice *device, NMDeviceStateReason *out_failure_reason)
 		teamd_cleanup (device, TRUE);
 	}
 
-	return teamd_start (device, s_team) ?
+	return teamd_start (device, connection) ?
 		NM_ACT_STAGE_RETURN_POSTPONE : NM_ACT_STAGE_RETURN_FAILURE;
 }