From 653ba8b759f3926d5244f3eb845bc1b3940e0bc1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 16 Jun 2017 12:46:41 +0200 Subject: [PATCH 1/3] core: log changes to NMSettingsConnection's flags (cherry picked from commit 2656ba8d1d9400e79f0d9646f3f7b0e4acd93298) --- src/settings/nm-settings-connection.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index bc6dcfce13..45a1b6649a 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -2129,6 +2129,7 @@ nm_settings_connection_set_flags_all (NMSettingsConnection *self, NMSettingsConn old_flags = priv->flags; if (old_flags != flags) { + _LOGT ("update settings-connection flags to 0x%x (was 0x%x)", (guint) flags, (guint) priv->flags); priv->flags = flags; _notify (self, PROP_FLAGS); if (NM_FLAGS_HAS (old_flags, NM_SETTINGS_CONNECTION_FLAGS_UNSAVED) != NM_FLAGS_HAS (flags, NM_SETTINGS_CONNECTION_FLAGS_UNSAVED)) From 08c58c759a6d1aa761e325a2fb0d14aa0cdaf532 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 16 Jun 2017 14:34:37 +0200 Subject: [PATCH 2/3] core: fix registering notify-flags hook in NMActiveConnection We react on changes to NMSettingsConnection.flags, so that we can update from an external activation to a managed one. However, previously we would only register the _settings_connection_notify_flags callback during _set_settings_connection(). So, if via constructor properties we first set PROP_SETTINGS_CONNECTION and later PROP_ACTIVATION_TYPE, we wouldn't register the callback. (cherry picked from commit b84da2571372d1efc70750e358bf8713b1ca7934) --- src/nm-active-connection.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index 19c0343fed..f7088fa080 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -111,8 +111,7 @@ static void _device_cleanup (NMActiveConnection *self); static void _settings_connection_notify_flags (NMSettingsConnection *settings_connection, GParamSpec *param, NMActiveConnection *self); -static void _set_activation_type (NMActiveConnection *self, - NMActivationType activation_type); +static void _set_activation_type_managed (NMActiveConnection *self); /*****************************************************************************/ @@ -236,7 +235,7 @@ nm_active_connection_set_state (NMActiveConnection *self, /* assuming connections mean to gracefully take over an externally * configured device. Once activation is complete, an assumed * activation *is* the same as a full activation. */ - _set_activation_type (self, NM_ACTIVATION_TYPE_MANAGED); + _set_activation_type_managed (self); } old_state = priv->state; @@ -755,13 +754,31 @@ _set_activation_type (NMActiveConnection *self, if (priv->activation_type == activation_type) return; - _LOGD ("update activation type from %s to %s", - nm_activation_type_to_string (priv->activation_type), - nm_activation_type_to_string (activation_type)); priv->activation_type = activation_type; - if ( priv->activation_type == NM_ACTIVATION_TYPE_MANAGED - && priv->device + if (priv->settings_connection) { + if (activation_type == NM_ACTIVATION_TYPE_EXTERNAL) + g_signal_connect (priv->settings_connection, "notify::"NM_SETTINGS_CONNECTION_FLAGS, (GCallback) _settings_connection_notify_flags, self); + else + g_signal_handlers_disconnect_by_func (priv->settings_connection, _settings_connection_notify_flags, self); + } +} + +static void +_set_activation_type_managed (NMActiveConnection *self) +{ + NMActiveConnectionPrivate *priv = NM_ACTIVE_CONNECTION_GET_PRIVATE (self); + + if (priv->activation_type == NM_ACTIVATION_TYPE_MANAGED) + return; + + _LOGD ("update activation type from %s to %s", + nm_activation_type_to_string (priv->activation_type), + nm_activation_type_to_string (NM_ACTIVATION_TYPE_MANAGED)); + + _set_activation_type (self, NM_ACTIVATION_TYPE_MANAGED); + + if ( priv->device && self == NM_ACTIVE_CONNECTION (nm_device_get_act_request (priv->device)) && NM_IN_SET (nm_device_sys_iface_state_get (priv->device), NM_DEVICE_SYS_IFACE_STATE_EXTERNAL, @@ -784,8 +801,7 @@ _settings_connection_notify_flags (NMSettingsConnection *settings_connection, if (nm_settings_connection_get_nm_generated (settings_connection)) return; - g_signal_handlers_disconnect_by_func (settings_connection, _settings_connection_notify_flags, self); - _set_activation_type (self, NM_ACTIVATION_TYPE_MANAGED); + _set_activation_type_managed (self); nm_device_reapply_settings_immediately (nm_active_connection_get_device (self)); } @@ -1147,7 +1163,7 @@ set_property (GObject *object, guint prop_id, NM_ACTIVATION_TYPE_ASSUME, NM_ACTIVATION_TYPE_EXTERNAL)) g_return_if_reached (); - priv->activation_type = (NMActivationType) i; + _set_activation_type (self, (NMActivationType) i); break; case PROP_SPECIFIC_OBJECT: tmp = g_value_get_string (value); From 368c7329f801ca212215344ce6146de02743e8b4 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 16 Jun 2017 13:06:26 +0200 Subject: [PATCH 3/3] device: fix taking over device after modifying external connection For externally managed interfaces, we create an in-memory connection and keep the device with sys-iface-state=external. When the user actively modifies the connection, we persist it to storage. But we also must take over managing the device. One problem is that nm_device_reapply() errors out if the device is still activating. It's unclear how to reapply the connection while the device is in the process of activation. So, if the user modifies the created connection very quickly, reapplying the settings will fail. https://bugzilla.redhat.com/show_bug.cgi?id=1462223 (cherry picked from commit 10c0632df0b576df100d7ab0bb6b30822f2c382b) --- src/devices/nm-device.c | 24 ++++++++++++++++++++---- src/devices/nm-device.h | 3 +++ src/nm-active-connection.c | 10 +++++++++- 3 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 22084fc0f5..fbf315ed3b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -9161,10 +9161,12 @@ check_and_reapply_connection (NMDevice *self, NM_SETTING_COMPARE_FLAG_IGNORE_SECRETS, &diffs); - if (diffs && nm_audit_manager_audit_enabled (nm_audit_manager_get ())) - *audit_args = nm_utils_format_con_diff_for_audit (diffs); - else - *audit_args = NULL; + if (audit_args) { + if (diffs && nm_audit_manager_audit_enabled (nm_audit_manager_get ())) + *audit_args = nm_utils_format_con_diff_for_audit (diffs); + else + *audit_args = NULL; + } /************************************************************************** * check for unsupported changes and reject to reapply @@ -9266,6 +9268,20 @@ check_and_reapply_connection (NMDevice *self, return TRUE; } +gboolean +nm_device_reapply (NMDevice *self, + NMConnection *connection, + GError **error) +{ + g_return_val_if_fail (NM_IS_DEVICE (self), FALSE); + + return check_and_reapply_connection (self, + connection, + 0, + NULL, + error); +} + typedef struct { NMConnection *connection; guint64 version_id; diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 50d9b9780f..6d17d7c9d3 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -699,6 +699,9 @@ const NMPlatformIP6Route *nm_device_get_ip6_default_route (NMDevice *self, gbool void nm_device_spawn_iface_helper (NMDevice *self); +gboolean nm_device_reapply (NMDevice *self, + NMConnection *connection, + GError **error); void nm_device_reapply_settings_immediately (NMDevice *self); void nm_device_update_firewall_zone (NMDevice *self); diff --git a/src/nm-active-connection.c b/src/nm-active-connection.c index f7088fa080..862754f9b5 100644 --- a/src/nm-active-connection.c +++ b/src/nm-active-connection.c @@ -793,6 +793,8 @@ _settings_connection_notify_flags (NMSettingsConnection *settings_connection, GParamSpec *param, NMActiveConnection *self) { + GError *error = NULL; + nm_assert (NM_IS_ACTIVE_CONNECTION (self)); nm_assert (NM_IS_SETTINGS_CONNECTION (settings_connection)); nm_assert (nm_active_connection_get_activation_type (self) == NM_ACTIVATION_TYPE_EXTERNAL); @@ -802,7 +804,13 @@ _settings_connection_notify_flags (NMSettingsConnection *settings_connection, return; _set_activation_type_managed (self); - nm_device_reapply_settings_immediately (nm_active_connection_get_device (self)); + if (!nm_device_reapply (nm_active_connection_get_device (self), + NM_CONNECTION (nm_active_connection_get_settings_connection (self)), + &error)) { + _LOGW ("failed to reapply new device settings on previously externally managed device: %s", + error->message); + g_error_free (error); + } } /*****************************************************************************/