From 1940be410cc0272de2f690542f43ef7dcb7bc4e1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 19 Apr 2017 15:45:31 +0200 Subject: config: remove unused NMConfig self argument from nm_config_device_state_*() API nm_config_device_state_*() always access the file system directly, they don't cache data in NMConfig. Hence, they don't use the @self argument. Maybe those functions don't belong to nm-config.h, anyway. For lack of a better place they are there. --- src/devices/nm-device.c | 2 +- src/nm-config.c | 11 +++-------- src/nm-config.h | 8 +++----- src/nm-manager.c | 10 +++------- 4 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index b83d4e08d7..160e5a7259 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -12997,7 +12997,7 @@ nm_device_update_permanent_hw_address (NMDevice *self, gboolean force_freeze) { gs_free NMConfigDeviceStateData *dev_state = NULL; - dev_state = nm_config_device_state_load (nm_config_get (), ifindex); + dev_state = nm_config_device_state_load (ifindex); if ( dev_state && dev_state->perm_hw_addr_fake && nm_utils_hwaddr_aton (dev_state->perm_hw_addr_fake, buf, priv->hw_addr_len) diff --git a/src/nm-config.c b/src/nm-config.c index 2470244d2b..7c3c886861 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -1926,15 +1926,13 @@ _config_device_state_data_new (int ifindex, GKeyFile *kf) /** * nm_config_device_state_load: - * @self: the NMConfig instance * @ifindex: the ifindex for which the state is to load * * Returns: (transfer full): a run state object. * Must be freed with g_free(). */ NMConfigDeviceStateData * -nm_config_device_state_load (NMConfig *self, - int ifindex) +nm_config_device_state_load (int ifindex) { NMConfigDeviceStateData *device_state; char path[NM_STRLEN (NM_CONFIG_DEVICE_STATE_DIR) + 60]; @@ -1972,8 +1970,7 @@ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_device_state_managed_type_to_str, NMConfigDe ); gboolean -nm_config_device_state_write (NMConfig *self, - int ifindex, +nm_config_device_state_write (int ifindex, NMConfigDeviceStateManagedType managed, const char *perm_hw_addr_fake, const char *connection_uuid) @@ -1982,7 +1979,6 @@ nm_config_device_state_write (NMConfig *self, GError *local = NULL; gs_unref_keyfile GKeyFile *kf = NULL; - g_return_val_if_fail (NM_IS_CONFIG (self), FALSE); g_return_val_if_fail (ifindex > 0, FALSE); g_return_val_if_fail (!connection_uuid || *connection_uuid, FALSE); g_return_val_if_fail (managed == NM_CONFIG_DEVICE_STATE_MANAGED_TYPE_MANAGED || !connection_uuid, FALSE); @@ -2027,8 +2023,7 @@ nm_config_device_state_write (NMConfig *self, } void -nm_config_device_state_prune_unseen (NMConfig *self, - GHashTable *seen_ifindexes) +nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes) { GDir *dir; const char *fn; diff --git a/src/nm-config.h b/src/nm-config.h index bfb4383a63..54d3a1ed90 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -204,14 +204,12 @@ struct _NMConfigDeviceStateData { const char *perm_hw_addr_fake; }; -NMConfigDeviceStateData *nm_config_device_state_load (NMConfig *self, - int ifindex); -gboolean nm_config_device_state_write (NMConfig *self, - int ifindex, +NMConfigDeviceStateData *nm_config_device_state_load (int ifindex); +gboolean nm_config_device_state_write (int ifindex, NMConfigDeviceStateManagedType managed, const char *perm_hw_addr_fake, const char *connection_uuid); -void nm_config_device_state_prune_unseen (NMConfig *self, GHashTable *seen_ifindexes); +void nm_config_device_state_prune_unseen (GHashTable *seen_ifindexes); /*****************************************************************************/ diff --git a/src/nm-manager.c b/src/nm-manager.c index 7b9da96da3..01303ca93d 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -2404,7 +2404,6 @@ platform_link_cb (NMPlatform *platform, static void platform_query_devices (NMManager *self) { - NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); GArray *links_array; NMPlatformLink *links; int i; @@ -2414,8 +2413,7 @@ platform_query_devices (NMManager *self) for (i = 0; i < links_array->len; i++) { gs_free NMConfigDeviceStateData *dev_state = NULL; - dev_state = nm_config_device_state_load (priv->config, - links[i].ifindex); + dev_state = nm_config_device_state_load (links[i].ifindex); platform_link_added (self, links[i].ifindex, @@ -4976,16 +4974,14 @@ nm_manager_write_device_state (NMManager *self) if (perm_hw_addr_fake && !perm_hw_addr_is_fake) perm_hw_addr_fake = NULL; - if (nm_config_device_state_write (priv->config, - ifindex, + if (nm_config_device_state_write (ifindex, managed_type, perm_hw_addr_fake, uuid)) g_hash_table_add (seen_ifindexes, GINT_TO_POINTER (ifindex)); } - nm_config_device_state_prune_unseen (priv->config, - seen_ifindexes); + nm_config_device_state_prune_unseen (seen_ifindexes); } static gboolean -- cgit v1.2.1 From 2131954a190244714e8b27b48567594c7b103fb9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 19 Apr 2017 15:56:24 +0200 Subject: config: add first_start paramter to NMConfig to detect restart --- src/main-utils.c | 3 +++ src/main.c | 9 +++++++-- src/nm-config.c | 22 +++++++++++++++++++++- src/nm-config.h | 4 +++- src/tests/config/test-config.c | 2 +- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/src/main-utils.c b/src/main-utils.c index 9e3aa7bd55..c84f1e4c16 100644 --- a/src/main-utils.c +++ b/src/main-utils.c @@ -149,6 +149,9 @@ nm_main_utils_ensure_rundir () exit (1); } + /* NM_CONFIG_DEVICE_STATE_DIR is used to determine whether NM is restarted or not. + * It is important to set NMConfigCmdLineOptions.first_start before creating + * the directory. */ nm_assert (g_str_has_prefix (NM_CONFIG_DEVICE_STATE_DIR, NMRUNDIR"/")); if (g_mkdir (NM_CONFIG_DEVICE_STATE_DIR, 0755) != 0) { errsv = errno; diff --git a/src/main.c b/src/main.c index 52f3200e75..52f0b7c8f9 100644 --- a/src/main.c +++ b/src/main.c @@ -239,7 +239,11 @@ main (int argc, char *argv[]) main_loop = g_main_loop_new (NULL, FALSE); - config_cli = nm_config_cmd_line_options_new (); + /* we determine a first-start (contrary to a restart during the same boot) + * based on the existence of NM_CONFIG_DEVICE_STATE_DIR directory. */ + config_cli = nm_config_cmd_line_options_new (!g_file_test (NM_CONFIG_DEVICE_STATE_DIR, + G_FILE_TEST_IS_DIR)); + do_early_setup (&argc, &argv, config_cli); if (global_opt.g_fatal_warnings) @@ -356,7 +360,8 @@ main (int argc, char *argv[]) NM_CONFIG_GET_VALUE_STRIP | NM_CONFIG_GET_VALUE_NO_EMPTY), nm_config_get_is_debug (config)); - nm_log_info (LOGD_CORE, "NetworkManager (version " NM_DIST_VERSION ") is starting..."); + nm_log_info (LOGD_CORE, "NetworkManager (version " NM_DIST_VERSION ") is starting... (%s)", + nm_config_get_first_start (config) ? "for the first time" : "after a restart"); nm_log_info (LOGD_CORE, "Read config: %s", nm_config_data_get_config_description (nm_config_get_data (config))); nm_config_data_log (nm_config_get_data (config), "CONFIG: ", " ", NULL); diff --git a/src/nm-config.c b/src/nm-config.c index 7c3c886861..15d016a21b 100644 --- a/src/nm-config.c +++ b/src/nm-config.c @@ -60,6 +60,15 @@ struct NMConfigCmdLineOptions { */ int connectivity_interval; char *connectivity_response; + + /* @first_start is not provided by command line. It is a convenient hack + * to pass in an argument to NMConfig. This makes NMConfigCmdLineOptions a + * misnomer. + * + * It is true, if NM is started the first time -- contrary to a restart + * during the same boot up. That is determined by the content of the + * /var/run/NetworManager state directory. */ + bool first_start; }; typedef struct { @@ -291,6 +300,12 @@ nm_config_get_is_debug (NMConfig *config) return NM_CONFIG_GET_PRIVATE (config)->cli.is_debug; } +gboolean +nm_config_get_first_start (NMConfig *config) +{ + return NM_CONFIG_GET_PRIVATE (config)->cli.first_start; +} + /*****************************************************************************/ static char ** @@ -412,6 +427,7 @@ _nm_config_cmd_line_options_clear (NMConfigCmdLineOptions *cli) g_clear_pointer (&cli->connectivity_uri, g_free); g_clear_pointer (&cli->connectivity_response, g_free); cli->connectivity_interval = -1; + cli->first_start = FALSE; } static void @@ -434,14 +450,18 @@ _nm_config_cmd_line_options_copy (const NMConfigCmdLineOptions *cli, NMConfigCmd dst->connectivity_uri = g_strdup (cli->connectivity_uri); dst->connectivity_response = g_strdup (cli->connectivity_response); dst->connectivity_interval = cli->connectivity_interval; + dst->first_start = cli->first_start; } NMConfigCmdLineOptions * -nm_config_cmd_line_options_new () +nm_config_cmd_line_options_new (gboolean first_start) { NMConfigCmdLineOptions *cli = g_new0 (NMConfigCmdLineOptions, 1); _nm_config_cmd_line_options_clear (cli); + + cli->first_start = first_start; + return cli; } diff --git a/src/nm-config.h b/src/nm-config.h index 54d3a1ed90..283d6a1b4b 100644 --- a/src/nm-config.h +++ b/src/nm-config.h @@ -125,13 +125,15 @@ const char *nm_config_get_log_domains (NMConfig *config); gboolean nm_config_get_configure_and_quit (NMConfig *config); gboolean nm_config_get_is_debug (NMConfig *config); +gboolean nm_config_get_first_start (NMConfig *config); + void nm_config_set_values (NMConfig *self, GKeyFile *keyfile_intern_new, gboolean allow_write, gboolean force_rewrite); /* for main.c only */ -NMConfigCmdLineOptions *nm_config_cmd_line_options_new (void); +NMConfigCmdLineOptions *nm_config_cmd_line_options_new (gboolean first_start); void nm_config_cmd_line_options_free (NMConfigCmdLineOptions *cli); void nm_config_cmd_line_options_add_to_entries (NMConfigCmdLineOptions *cli, GOptionContext *opt_ctx); diff --git a/src/tests/config/test-config.c b/src/tests/config/test-config.c index dd2a58a06a..edb5c1aa23 100644 --- a/src/tests/config/test-config.c +++ b/src/tests/config/test-config.c @@ -97,7 +97,7 @@ setup_config (GError **error, const char *config_file, const char *intern_config argv = (char **)args->pdata; argc = args->len; - cli = nm_config_cmd_line_options_new (); + cli = nm_config_cmd_line_options_new (FALSE); context = g_option_context_new (NULL); nm_config_cmd_line_options_add_to_entries (cli, context); -- cgit v1.2.1 From 27b2477cb7dad2410c88c7dfca51f3aad208b881 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 19 Apr 2017 16:16:12 +0200 Subject: device: assume matching connections during first start Since commit 2d1b85f (th/assume-vs-unmanaged-bgo746440), we clearly distinguish between two modes when encountering devices with external IP configuration: a) external devices. For those devices we generate a volatile in-memory connection and pretend it's active. However, the device must not be touched by NetworkManager in any way. b) assume, seamless take over. Mostly for restart of NetworkManager, we activate a connection gracefully without going through an down-up cycle. After the device reaches activated state, the device is considered fully managed. For this only an existing, non volatile connection can be used. Before 'th/assume-vs-unmanaged-bgo746440', the behaviors were not clearly separated. Since then, we only choose to assume a connection (b) when the state file indicates a matching connection. Now, extend this to also assume connections when: - during first-start (not after a restart) when there is no state file yet. - and, if we have an existing, non volatile, connection which matches the device's configuration. This patch lets NetworkManager assume connection also on first start. That is for example useful when handing over network configuration from initrd. This only applies to existing, permanent, matching(!) connections, so it is a good guess that the user wants NM to take over this interface. This brings us closer to the previous behavior before 'th/assume-vs-unmanaged-bgo746440'. https://bugzilla.redhat.com/show_bug.cgi?id=1439220 --- src/nm-manager.c | 90 +++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 24 deletions(-) diff --git a/src/nm-manager.c b/src/nm-manager.c index 01303ca93d..2490025574 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -450,6 +450,8 @@ _get_activatable_connections_filter (NMSettings *settings, NMSettingsConnection *connection, gpointer user_data) { + if (nm_settings_connection_get_volatile (connection)) + return FALSE; return !active_connection_find_first (user_data, connection, NULL, NM_ACTIVE_CONNECTION_STATE_DEACTIVATING); } @@ -1677,16 +1679,12 @@ done: g_clear_error (&error); } -static gboolean -match_connection_filter (NMConnection *connection, gpointer user_data) -{ - return nm_device_check_connection_compatible (NM_DEVICE (user_data), connection); -} - /** * get_existing_connection: * @manager: #NMManager instance * @device: #NMDevice instance + * @guess_assume: whether to employ a heuristic to search for a matching + * connection to assume. * @assume_connection_uuid: if present, try to assume a connection with this * UUID. If no uuid is given or no matching connection is found, we * only do external activation. @@ -1698,6 +1696,7 @@ match_connection_filter (NMConnection *connection, gpointer user_data) static NMSettingsConnection * get_existing_connection (NMManager *self, NMDevice *device, + gboolean guess_assume, const char *assume_connection_uuid, gboolean *out_generated) { @@ -1708,6 +1707,7 @@ get_existing_connection (NMManager *self, NMDevice *master = NULL; int ifindex = nm_device_get_ifindex (device); NMSettingsConnection *matched; + NMSettingsConnection *connection_checked = NULL; if (out_generated) *out_generated = FALSE; @@ -1752,11 +1752,12 @@ get_existing_connection (NMManager *self, * the generated connection instead. */ if ( assume_connection_uuid - && (matched = nm_settings_get_connection_by_uuid (priv->settings, assume_connection_uuid)) - && !active_connection_find_first (self, matched, NULL, - NM_ACTIVE_CONNECTION_STATE_DEACTIVATING)) { + && (connection_checked = nm_settings_get_connection_by_uuid (priv->settings, assume_connection_uuid)) + && !active_connection_find_first (self, connection_checked, NULL, + NM_ACTIVE_CONNECTION_STATE_DEACTIVATING) + && nm_device_check_connection_compatible (device, NM_CONNECTION (connection_checked))) { NMConnection *const connections[] = { - NM_CONNECTION (matched), + NM_CONNECTION (connection_checked), NULL, }; @@ -1765,17 +1766,50 @@ get_existing_connection (NMManager *self, nm_device_has_carrier (device), nm_device_get_ip4_route_metric (device), nm_device_get_ip6_route_metric (device), - match_connection_filter, - device)); - if (matched) { - _LOGI (LOGD_DEVICE, "(%s): found matching connection '%s'", - nm_device_get_iface (device), - nm_settings_connection_get_id (matched)); - g_object_unref (connection); - return matched; + NULL, NULL)); + } else + matched = NULL; + + if (!matched && guess_assume) { + gs_free NMSettingsConnection **connections = NULL; + guint len, i, j; + + /* the state file doesn't indicate a connection UUID to assume. Search the + * persistent connections for a matching candidate. */ + connections = nm_manager_get_activatable_connections (self, &len, FALSE); + if (len > 0) { + for (i = 0, j = 0; i < len; i++) { + NMConnection *con = NM_CONNECTION (connections[i]); + + if ( con != NM_CONNECTION (connection_checked) + && nm_device_check_connection_compatible (device, con)) + connections[j++] = connections[i]; + } + connections[j] = NULL; + len = j; + g_qsort_with_data (connections, len, sizeof (connections[0]), + nm_settings_connection_cmp_timestamp_p_with_data, NULL); + + matched = NM_SETTINGS_CONNECTION (nm_utils_match_connection ((NMConnection *const*) connections, + connection, + nm_device_has_carrier (device), + nm_device_get_ip4_route_metric (device), + nm_device_get_ip6_route_metric (device), + NULL, NULL)); } } + if (matched) { + _LOGI (LOGD_DEVICE, "(%s): found matching connection '%s' (%s)%s", + nm_device_get_iface (device), + nm_settings_connection_get_id (matched), + nm_settings_connection_get_uuid (matched), + assume_connection_uuid && nm_streq (assume_connection_uuid, nm_settings_connection_get_uuid (matched)) + ? " (indicated)" : " (guessed)"); + g_object_unref (connection); + return matched; + } + _LOGD (LOGD_DEVICE, "(%s): generated connection '%s'", nm_device_get_iface (device), nm_connection_get_id (connection)); @@ -1803,6 +1837,7 @@ get_existing_connection (NMManager *self, static gboolean recheck_assume_connection (NMManager *self, NMDevice *device, + gboolean guess_assume, const char *assume_connection_uuid) { NMSettingsConnection *connection; @@ -1826,7 +1861,7 @@ recheck_assume_connection (NMManager *self, if (nm_device_sys_iface_state_get (device) != NM_DEVICE_SYS_IFACE_STATE_EXTERNAL) return FALSE; - connection = get_existing_connection (self, device, assume_connection_uuid, &generated); + connection = get_existing_connection (self, device, guess_assume, assume_connection_uuid, &generated); if (!connection) { _LOGD (LOGD_DEVICE, "(%s): can't assume; no connection", nm_device_get_iface (device)); @@ -1906,7 +1941,7 @@ recheck_assume_connection (NMManager *self, static void recheck_assume_connection_cb (NMDevice *device, gpointer user_data) { - recheck_assume_connection (user_data, device, NULL); + recheck_assume_connection (user_data, device, FALSE, NULL); } static void @@ -2000,6 +2035,7 @@ static void _device_realize_finish (NMManager *self, NMDevice *device, const NMPlatformLink *plink, + gboolean guess_assume, const char *connection_uuid_to_assume) { g_return_if_fail (NM_IS_MANAGER (self)); @@ -2010,7 +2046,7 @@ _device_realize_finish (NMManager *self, if (!nm_device_get_managed (device, FALSE)) return; - if (recheck_assume_connection (self, device, connection_uuid_to_assume)) + if (recheck_assume_connection (self, device, guess_assume, connection_uuid_to_assume)) return; /* if we failed to assume a connection for the managed device, but the device @@ -2168,7 +2204,7 @@ factory_device_added_cb (NMDeviceFactory *factory, NULL, &error)) { add_device (self, device, NULL); - _device_realize_finish (self, device, NULL, NULL); + _device_realize_finish (self, device, NULL, FALSE, NULL); } else { _LOGW (LOGD_DEVICE, "(%s): failed to realize device: %s", nm_device_get_iface (device), error->message); @@ -2214,6 +2250,7 @@ static void platform_link_added (NMManager *self, int ifindex, const NMPlatformLink *plink, + gboolean guess_assume, const NMConfigDeviceStateData *dev_state) { NMDeviceFactory *factory; @@ -2245,7 +2282,7 @@ platform_link_added (NMManager *self, &compatible, &error)) { /* Success */ - _device_realize_finish (self, candidate, plink, NULL); + _device_realize_finish (self, candidate, plink, FALSE, NULL); return; } @@ -2317,6 +2354,7 @@ platform_link_added (NMManager *self, &error)) { add_device (self, device, NULL); _device_realize_finish (self, device, plink, + guess_assume, dev_state ? dev_state->connection_uuid : NULL); } else { _LOGW (LOGD_DEVICE, "%s: failed to realize device: %s", @@ -2347,7 +2385,7 @@ _platform_link_cb_idle (PlatformLinkCbData *data) NMPlatformLink pllink; pllink = *l; /* make a copy of the link instance */ - platform_link_added (self, data->ifindex, &pllink, NULL); + platform_link_added (self, data->ifindex, &pllink, FALSE, NULL); } else { NMDevice *device; GError *error = NULL; @@ -2407,6 +2445,9 @@ platform_query_devices (NMManager *self) GArray *links_array; NMPlatformLink *links; int i; + gboolean guess_assume; + + guess_assume = nm_config_get_first_start (nm_config_get ()); links_array = nm_platform_link_get_all (NM_PLATFORM_GET); links = (NMPlatformLink *) links_array->data; @@ -2418,6 +2459,7 @@ platform_query_devices (NMManager *self) platform_link_added (self, links[i].ifindex, &links[i], + guess_assume && (!dev_state || !dev_state->connection_uuid), dev_state); } -- cgit v1.2.1