diff options
author | Thomas Haller <thaller@redhat.com> | 2019-07-02 17:53:41 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2019-07-02 17:53:41 +0200 |
commit | e92dca357f8d34b3b5a4a13999ee4ea4ce41363d (patch) | |
tree | 3b2a2ac0e3fb7bc4785b1e60176d2ec8fb1f38e5 | |
parent | 26317ec7e30753c9b0f2db7eabe236b39af4aa13 (diff) | |
parent | c163207b077a0e66e344b2352ccbe392c76e63ed (diff) | |
download | NetworkManager-e92dca357f8d34b3b5a4a13999ee4ea4ce41363d.tar.gz |
all: merge branch 'th/various-settings-cleanup-4'
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/192
-rw-r--r-- | shared/nm-glib-aux/nm-c-list.h | 12 | ||||
-rw-r--r-- | src/devices/nm-device-bond.c | 40 | ||||
-rw-r--r-- | src/devices/nm-device-bridge.c | 10 | ||||
-rw-r--r-- | src/devices/team/nm-device-team.c | 13 | ||||
-rw-r--r-- | src/nm-manager.c | 51 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 93 |
6 files changed, 140 insertions, 79 deletions
diff --git a/shared/nm-glib-aux/nm-c-list.h b/shared/nm-glib-aux/nm-c-list.h index 96b0279469..7512730d81 100644 --- a/shared/nm-glib-aux/nm-c-list.h +++ b/shared/nm-glib-aux/nm-c-list.h @@ -67,6 +67,18 @@ nm_c_list_elem_free (NMCListElem *elem) return nm_c_list_elem_free_full (elem, NULL); } +static inline void * +nm_c_list_elem_free_steal (NMCListElem *elem) +{ + gpointer data; + + if (!elem) + return NULL; + data = elem->data; + nm_c_list_elem_free_full (elem, NULL); + return data; +} + static inline void nm_c_list_elem_free_all (CList *head, GDestroyNotify free_fcn) { diff --git a/src/devices/nm-device-bond.c b/src/devices/nm-device-bond.c index 53a53f91fc..fd79348d2c 100644 --- a/src/devices/nm-device-bond.c +++ b/src/devices/nm-device-bond.c @@ -413,6 +413,12 @@ release_slave (NMDevice *device, NMDeviceBond *self = NM_DEVICE_BOND (device); gboolean success; gs_free char *address = NULL; + int ifindex_slave; + + ifindex_slave = nm_device_get_ip_ifindex (slave); + + if (ifindex_slave <= 0) + _LOGD (LOGD_TEAM, "bond slave %s is already released", nm_device_get_ip_iface (slave)); if (configure) { /* When the last slave is released the bond MAC will be set to a random @@ -420,16 +426,18 @@ release_slave (NMDevice *device, */ address = g_strdup (nm_device_get_hw_address (device)); - success = nm_platform_link_release (nm_device_get_platform (device), - nm_device_get_ip_ifindex (device), - nm_device_get_ip_ifindex (slave)); - - if (success) { - _LOGI (LOGD_BOND, "released bond slave %s", - nm_device_get_ip_iface (slave)); - } else { - _LOGW (LOGD_BOND, "failed to release bond slave %s", - nm_device_get_ip_iface (slave)); + if (ifindex_slave > 0) { + success = nm_platform_link_release (nm_device_get_platform (device), + nm_device_get_ip_ifindex (device), + ifindex_slave); + + if (success) { + _LOGI (LOGD_BOND, "released bond slave %s", + nm_device_get_ip_iface (slave)); + } else { + _LOGW (LOGD_BOND, "failed to release bond slave %s", + nm_device_get_ip_iface (slave)); + } } nm_platform_process_events (nm_device_get_platform (device)); @@ -440,11 +448,15 @@ release_slave (NMDevice *device, * IFF_UP), so we must bring it back up here to ensure carrier changes and * other state is noticed by the now-released slave. */ - if (!nm_device_bring_up (slave, TRUE, NULL)) - _LOGW (LOGD_BOND, "released bond slave could not be brought up."); + if (ifindex_slave > 0) { + if (!nm_device_bring_up (slave, TRUE, NULL)) + _LOGW (LOGD_BOND, "released bond slave could not be brought up."); + } } else { - _LOGI (LOGD_BOND, "bond slave %s was released", - nm_device_get_ip_iface (slave)); + if (ifindex_slave > 0) { + _LOGI (LOGD_BOND, "bond slave %s was released", + nm_device_get_ip_iface (slave)); + } } } diff --git a/src/devices/nm-device-bridge.c b/src/devices/nm-device-bridge.c index 3b8f9809b1..ade9eb0d8f 100644 --- a/src/devices/nm-device-bridge.c +++ b/src/devices/nm-device-bridge.c @@ -625,11 +625,19 @@ release_slave (NMDevice *device, { NMDeviceBridge *self = NM_DEVICE_BRIDGE (device); gboolean success; + int ifindex_slave; + + ifindex_slave = nm_device_get_ip_ifindex (slave); + + if (ifindex_slave <= 0) { + _LOGD (LOGD_TEAM, "bond slave %s is already released", nm_device_get_ip_iface (slave)); + return; + } if (configure) { success = nm_platform_link_release (nm_device_get_platform (device), nm_device_get_ip_ifindex (device), - nm_device_get_ip_ifindex (slave)); + ifindex_slave); if (success) { _LOGI (LOGD_BRIDGE, "detached bridge port %s", diff --git a/src/devices/team/nm-device-team.c b/src/devices/team/nm-device-team.c index 3c693ff4be..a60a9fda5d 100644 --- a/src/devices/team/nm-device-team.c +++ b/src/devices/team/nm-device-team.c @@ -774,11 +774,19 @@ release_slave (NMDevice *device, NMDeviceTeam *self = NM_DEVICE_TEAM (device); NMDeviceTeamPrivate *priv = NM_DEVICE_TEAM_GET_PRIVATE (self); gboolean success; + int ifindex_slave; + + ifindex_slave = nm_device_get_ip_ifindex (slave); + + if (ifindex_slave <= 0) { + _LOGD (LOGD_TEAM, "team port %s is already released", nm_device_get_ip_iface (slave)); + return; + } if (configure) { success = nm_platform_link_release (nm_device_get_platform (device), nm_device_get_ip_ifindex (device), - nm_device_get_ip_ifindex (slave)); + ifindex_slave); if (success) _LOGI (LOGD_TEAM, "released team port %s", nm_device_get_ip_iface (slave)); @@ -789,9 +797,10 @@ release_slave (NMDevice *device, * IFF_UP), so we must bring it back up here to ensure carrier changes and * other state is noticed by the now-released port. */ - if (!nm_device_bring_up (slave, TRUE, NULL)) + if (!nm_device_bring_up (slave, TRUE, NULL)) { _LOGW (LOGD_TEAM, "released team port %s could not be brought up", nm_device_get_ip_iface (slave)); + } nm_clear_g_source (&priv->teamd_read_timeout); priv->teamd_read_timeout = g_timeout_add_seconds (5, diff --git a/src/nm-manager.c b/src/nm-manager.c index c961ebc045..d2dd2f4d51 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -176,6 +176,9 @@ typedef struct { NMSettings *settings; + CList connection_changed_on_idle_lst; + guint connection_changed_on_idle_id; + RadioState radio_states[RFKILL_TYPE_MAX]; NMVpnManager *vpn_manager; @@ -2064,6 +2067,7 @@ static void connection_changed (NMManager *self, NMSettingsConnection *sett_conn) { + NMManagerPrivate *priv; NMConnection *connection; NMDevice *device; @@ -2071,6 +2075,11 @@ connection_changed (NMManager *self, NM_SETTINGS_CONNECTION_INT_FLAGS_VOLATILE)) return; + priv = NM_MANAGER_GET_PRIVATE (self); + + if (!nm_settings_has_connection (priv->settings, sett_conn)) + return; + connection = nm_settings_connection_get_connection (sett_conn); if (!nm_connection_is_virtual (connection)) @@ -2086,12 +2095,46 @@ connection_changed (NMManager *self, retry_connections_for_parent_device (self, device); } +static gboolean +connection_changed_on_idle_cb (gpointer user_data) +{ + NMManager *self = user_data; + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + NMCListElem *elem; + + priv->connection_changed_on_idle_id = 0; + + while ((elem = c_list_first_entry (&priv->connection_changed_on_idle_lst, NMCListElem, lst))) { + gs_unref_object NMSettingsConnection *sett_conn = NULL; + + sett_conn = nm_c_list_elem_free_steal (elem); + connection_changed (self, sett_conn); + } + + return G_SOURCE_REMOVE; +} + +static void +connection_changed_on_idle (NMManager *self, + NMSettingsConnection *sett_conn) +{ + NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); + + if (priv->connection_changed_on_idle_id == 0) + priv->connection_changed_on_idle_id = g_idle_add (connection_changed_on_idle_cb, self); + + if (!nm_c_list_elem_find_first (&priv->connection_changed_on_idle_lst, sett_conn)) { + c_list_link_tail (&priv->connection_changed_on_idle_lst, + &nm_c_list_elem_new_stale (g_object_ref (sett_conn))->lst); + } +} + static void connection_added_cb (NMSettings *settings, NMSettingsConnection *sett_conn, NMManager *self) { - connection_changed (self, sett_conn); + connection_changed_on_idle (self, sett_conn); } static void @@ -2100,7 +2143,7 @@ connection_updated_cb (NMSettings *settings, gboolean by_user, NMManager *self) { - connection_changed (self, sett_conn); + connection_changed_on_idle (self, sett_conn); } /*****************************************************************************/ @@ -7357,6 +7400,7 @@ nm_manager_init (NMManager *self) c_list_init (&priv->active_connections_lst_head); c_list_init (&priv->async_op_lst_head); c_list_init (&priv->delete_volatile_connection_lst_head); + c_list_init (&priv->connection_changed_on_idle_lst); priv->platform = g_object_ref (NM_PLATFORM_GET); @@ -7658,6 +7702,9 @@ dispose (GObject *object) g_clear_object (&priv->policy); } + nm_clear_g_source (&priv->connection_changed_on_idle_id); + nm_c_list_elem_free_all (&priv->connection_changed_on_idle_lst, g_object_unref); + if (priv->settings) { g_signal_handlers_disconnect_by_func (priv->settings, settings_startup_complete_changed, self); g_signal_handlers_disconnect_by_func (priv->settings, system_unmanaged_devices_changed_cb, self); diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index e6a08f7158..0e0fa25894 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -3332,50 +3332,6 @@ do_write_to_disk (NMConnection *connection, return TRUE; } -static gboolean -do_write_reread (NMConnection *connection, - const char *ifcfg_name, - NMConnection **out_reread, - gboolean *out_reread_same, - GError **error) -{ - gs_unref_object NMConnection *reread = NULL; - gs_free_error GError *local = NULL; - gs_free char *unhandled = NULL; - gboolean reread_same = FALSE; - - nm_assert (!out_reread || !*out_reread); - - reread = connection_from_file (ifcfg_name, &unhandled, &local, NULL); - - if (!reread) { - g_propagate_error (error, local); - local = NULL; - return FALSE; - } - if (unhandled) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, - "connection is unhandled"); - return FALSE; - } - if (out_reread_same) { - if (nm_connection_compare (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT)) - reread_same = TRUE; - - nm_assert (reread_same == nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT)); - nm_assert (reread_same == ({ - gs_unref_hashtable GHashTable *_settings = NULL; - - ( nm_connection_diff (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT, &_settings) - && !_settings); - })); - } - - NM_SET_OUT (out_reread, g_steal_pointer (&reread)); - NM_SET_OUT (out_reread_same, reread_same); - return TRUE; -} - gboolean nms_ifcfg_rh_writer_write_connection (NMConnection *connection, const char *ifcfg_dir, @@ -3394,7 +3350,6 @@ nms_ifcfg_rh_writer_write_connection (NMConnection *connection, nm_auto_free_gstring GString *route6_content = NULL; gs_unref_hashtable GHashTable *secrets = NULL; gs_unref_hashtable GHashTable *blobs = NULL; - GError *local = NULL; nm_assert (!out_reread || !*out_reread); @@ -3431,28 +3386,46 @@ nms_ifcfg_rh_writer_write_connection (NMConnection *connection, /* Note that we just wrote the connection to disk, and re-read it from there. * That is racy if somebody else modifies the connection. + * That race is why we must not tread a failure to re-read the profile + * as an error. * - * A better solution might be, to re-read the connection only based on the - * in-memory representation of what we collected above. But the reader + * FIXME: a much better solution might be, to re-read the connection only based + * on the in-memory representation of what we collected above. But the reader * does not yet allow to inject the configuration. */ - if (out_reread || out_reread_same) { - if (!do_write_reread (connection, - svFileGetName (ifcfg), - out_reread, - out_reread_same, - &local)) { + if ( out_reread + || out_reread_same) { + gs_unref_object NMConnection *reread = NULL; + gboolean reread_same = FALSE; + gs_free_error GError *local = NULL; + gs_free char *unhandled = NULL; + + reread = connection_from_file (svFileGetName (ifcfg), + &unhandled, + &local, + NULL); + nm_assert ((NM_IS_CONNECTION (reread) && !local) || (!reread && local)); + + if (!reread) { _LOGW ("write: failure to re-read connection \"%s\": %s", svFileGetName (ifcfg), local->message); - g_clear_error (&local); + } else if (unhandled) { + g_clear_object (&reread); + _LOGW ("write: failure to re-read connection \"%s\": %s", + svFileGetName (ifcfg), "connection is unhandled"); } else { - if ( out_reread_same - && !*out_reread_same) { - _LOGD ("write: connection %s (%s) was modified by persisting it to \"%s\" ", - nm_connection_get_id (connection), - nm_connection_get_uuid (connection), - svFileGetName (ifcfg)); + if (out_reread_same) { + reread_same = nm_connection_compare (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT); + if (!reread_same) { + _LOGD ("write: connection %s (%s) was modified by persisting it to \"%s\" ", + nm_connection_get_id (connection), + nm_connection_get_uuid (connection), + svFileGetName (ifcfg)); + } } } + + NM_SET_OUT (out_reread, g_steal_pointer (&reread)); + NM_SET_OUT (out_reread_same, reread_same); } /* Only return the filename if this was a newly written ifcfg */ |