diff options
author | Thomas Haller <thaller@redhat.com> | 2015-01-13 16:58:24 +0100 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2015-03-05 12:56:10 +0100 |
commit | f00ad4452f3ed2a8a9c461539b517451321b664e (patch) | |
tree | 547feaca7e0c2c858c255e590def22e3d71ad582 | |
parent | ae500e4ee0428fee09ef3d6d21fe0e8d4acfadde (diff) | |
parent | 2384955bbd8cb77dc4745efdcab85ae7a29037de (diff) | |
download | NetworkManager-th/uuid-duplicate-rh1171751.tar.gz |
settings: merge branch 'th/uuid-duplicate-rh1171751'th/uuid-duplicate-rh1171751
https://bugzilla.redhat.com/show_bug.cgi?id=1171751
(cherry picked from commit 29eb46b126f111a68ae811aa69603f47b3a90c7a)
-rw-r--r-- | src/NetworkManagerUtils.c | 8 | ||||
-rw-r--r-- | src/nm-properties-changed-signal.c | 4 | ||||
-rw-r--r-- | src/settings/nm-settings-connection.c | 25 | ||||
-rw-r--r-- | src/settings/nm-settings-connection.h | 1 | ||||
-rw-r--r-- | src/settings/nm-settings.c | 18 | ||||
-rw-r--r-- | src/settings/plugins/example/nm-example-connection.c | 1 | ||||
-rw-r--r-- | src/settings/plugins/ibft/nm-ibft-connection.c | 1 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/common.h | 2 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c | 1 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/plugin.c | 538 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh-utils.c | 2 | ||||
-rw-r--r-- | src/settings/plugins/ifcfg-rh/utils.h | 7 | ||||
-rw-r--r-- | src/settings/plugins/ifnet/nm-ifnet-connection.c | 1 | ||||
-rw-r--r-- | src/settings/plugins/ifnet/plugin.c | 7 | ||||
-rw-r--r-- | src/settings/plugins/keyfile/common.h | 2 | ||||
-rw-r--r-- | src/settings/plugins/keyfile/nm-keyfile-connection.c | 1 | ||||
-rw-r--r-- | src/settings/plugins/keyfile/plugin.c | 372 | ||||
-rw-r--r-- | src/settings/plugins/keyfile/utils.h | 7 | ||||
-rw-r--r-- | src/settings/plugins/keyfile/writer.c | 7 |
19 files changed, 633 insertions, 372 deletions
diff --git a/src/NetworkManagerUtils.c b/src/NetworkManagerUtils.c index b2049cf886..68c0dac303 100644 --- a/src/NetworkManagerUtils.c +++ b/src/NetworkManagerUtils.c @@ -2091,9 +2091,9 @@ nm_utils_log_connection_diff (NMConnection *connection, NMConnection *diff_base, connection_diff_are_same = nm_connection_diff (connection, diff_base, NM_SETTING_COMPARE_FLAG_EXACT | NM_SETTING_COMPARE_FLAG_DIFF_RESULT_NO_DEFAULT, &connection_diff); if (connection_diff_are_same) { if (diff_base) - nm_log (level, domain, "%sconnection '%s' (%p and %p): no difference", prefix, name, connection, diff_base); + nm_log (level, domain, "%sconnection '%s' (%p/%s and %p/%s): no difference", prefix, name, connection, G_OBJECT_TYPE_NAME (connection), diff_base, G_OBJECT_TYPE_NAME (diff_base)); else - nm_log (level, domain, "%sconnection '%s' (%p): no properties set", prefix, name, connection); + nm_log (level, domain, "%sconnection '%s' (%p/%s): no properties set", prefix, name, connection, G_OBJECT_TYPE_NAME (connection)); g_assert (!connection_diff); return; } @@ -2128,9 +2128,9 @@ nm_utils_log_connection_diff (NMConnection *connection, NMConnection *diff_base, GError *err_verify = NULL; if (diff_base) - nm_log (level, domain, "%sconnection '%s' (%p < %p):", prefix, name, connection, diff_base); + nm_log (level, domain, "%sconnection '%s' (%p/%s < %p/%s):", prefix, name, connection, G_OBJECT_TYPE_NAME (connection), diff_base, G_OBJECT_TYPE_NAME (diff_base)); else - nm_log (level, domain, "%sconnection '%s' (%p):", prefix, name, connection); + nm_log (level, domain, "%sconnection '%s' (%p/%s):", prefix, name, connection, G_OBJECT_TYPE_NAME (connection)); print_header = FALSE; if (!nm_connection_verify (connection, &err_verify)) { diff --git a/src/nm-properties-changed-signal.c b/src/nm-properties-changed-signal.c index 127da794e7..c6dbb3d532 100644 --- a/src/nm-properties-changed-signal.c +++ b/src/nm-properties-changed-signal.c @@ -148,8 +148,8 @@ notify (GObject *object, GParamSpec *pspec) break; } if (!dbus_property_name) { - nm_log_dbg (LOGD_DBUS_PROPS, "ignoring notification for prop %s on type %s", - pspec->name, G_OBJECT_TYPE_NAME (object)); + nm_log_trace (LOGD_DBUS_PROPS, "ignoring notification for prop %s on type %s", + pspec->name, G_OBJECT_TYPE_NAME (object)); return; } diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index f62000a63d..ec2052f0a0 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -447,6 +447,7 @@ gboolean nm_settings_connection_replace_settings (NMSettingsConnection *self, NMConnection *new_connection, gboolean update_unsaved, + const char *log_diff_name, GError **error) { NMSettingsConnectionPrivate *priv; @@ -460,6 +461,15 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, if (!nm_connection_normalize (new_connection, NULL, NULL, error)) return FALSE; + if ( nm_connection_get_path (NM_CONNECTION (self)) + && g_strcmp0 (nm_connection_get_uuid (NM_CONNECTION (self)), nm_connection_get_uuid (new_connection)) != 0) { + /* Updating the UUID is not allowed once the path is exported. */ + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "connection %s cannot change the UUID from %s to %s", nm_connection_get_id (NM_CONNECTION (self)), + nm_connection_get_uuid (NM_CONNECTION (self)), nm_connection_get_uuid (new_connection)); + return FALSE; + } + /* Do nothing if there's nothing to update */ if (nm_connection_compare (NM_CONNECTION (self), new_connection, @@ -472,7 +482,8 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self, */ g_signal_handlers_block_by_func (self, G_CALLBACK (changed_cb), GUINT_TO_POINTER (TRUE)); - nm_utils_log_connection_diff (new_connection, NM_CONNECTION (self), LOGL_DEBUG, LOGD_CORE, "update connection", "++ "); + if (log_diff_name) + nm_utils_log_connection_diff (new_connection, NM_CONNECTION (self), LOGL_DEBUG, LOGD_CORE, log_diff_name, "++ "); nm_connection_replace_settings_from_connection (NM_CONNECTION (self), new_connection); nm_settings_connection_set_flags (self, @@ -532,9 +543,10 @@ replace_and_commit (NMSettingsConnection *self, { GError *error = NULL; - if (nm_settings_connection_replace_settings (self, new_connection, TRUE, &error)) { + if (nm_settings_connection_replace_settings (self, new_connection, TRUE, "replace-and-commit-disk", &error)) nm_settings_connection_commit_changes (self, callback, user_data); - } else { + else { + g_assert (error); if (callback) callback (self, error, user_data); g_clear_error (&error); @@ -1364,11 +1376,8 @@ update_auth_cb (NMSettingsConnection *self, con_update_cb, info); } else { - /* Do nothing if there's nothing to update */ - if (!nm_connection_compare (NM_CONNECTION (self), info->new_settings, NM_SETTING_COMPARE_FLAG_EXACT)) { - if (!nm_settings_connection_replace_settings (self, info->new_settings, TRUE, &local)) - g_assert (local); - } + if (!nm_settings_connection_replace_settings (self, info->new_settings, TRUE, "replace-and-commit-memory", &local)) + g_assert (local); con_update_cb (self, local, info); g_clear_error (&local); } diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index 8875097215..49661f38c6 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -123,6 +123,7 @@ void nm_settings_connection_commit_changes (NMSettingsConnection *connection, gboolean nm_settings_connection_replace_settings (NMSettingsConnection *self, NMConnection *new_connection, gboolean update_unsaved, + const char *log_diff_name, GError **error); void nm_settings_connection_replace_and_commit (NMSettingsConnection *self, diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index bc123b4686..9d40b06e89 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -887,6 +887,7 @@ claim_connection (NMSettings *self, GHashTableIter iter; gpointer data; char *path; + NMSettingsConnection *existing; g_return_if_fail (NM_IS_SETTINGS_CONNECTION (connection)); g_return_if_fail (nm_connection_get_path (NM_CONNECTION (connection)) == NULL); @@ -905,6 +906,23 @@ claim_connection (NMSettings *self, return; } + existing = nm_settings_get_connection_by_uuid (self, nm_connection_get_uuid (NM_CONNECTION (connection))); + if (existing) { + /* Cannot add duplicate connections per UUID. Just return without action and + * log a warning. + * + * This means, that plugins must not provide duplicate connections (UUID). + * In fact, none of the plugins currently would do that. + * + * But globaly, over different setting plugins, there could be duplicates + * without the individual plugins being aware. Don't handle that at all, just + * error out. That should not happen unless the admin misconfigured the system + * to create conflicting connections. */ + nm_log_warn (LOGD_SETTINGS, "plugin provided duplicate connection with UUID %s", + nm_connection_get_uuid (NM_CONNECTION (connection))); + return; + } + /* Read timestamp from look-aside file and put it into the connection's data */ nm_settings_connection_read_and_fill_timestamp (connection); diff --git a/src/settings/plugins/example/nm-example-connection.c b/src/settings/plugins/example/nm-example-connection.c index 2f0b20a08b..e125961130 100644 --- a/src/settings/plugins/example/nm-example-connection.c +++ b/src/settings/plugins/example/nm-example-connection.c @@ -82,6 +82,7 @@ nm_example_connection_new (const char *full_path, if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), tmp, TRUE, + NULL, error)) { g_object_unref (object); object = NULL; diff --git a/src/settings/plugins/ibft/nm-ibft-connection.c b/src/settings/plugins/ibft/nm-ibft-connection.c index 5459a70504..c6a9054e47 100644 --- a/src/settings/plugins/ibft/nm-ibft-connection.c +++ b/src/settings/plugins/ibft/nm-ibft-connection.c @@ -46,6 +46,7 @@ nm_ibft_connection_new (const GPtrArray *block, GError **error) if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), source, FALSE, + NULL, error)) g_clear_object (&object); diff --git a/src/settings/plugins/ifcfg-rh/common.h b/src/settings/plugins/ifcfg-rh/common.h index 7736d2632e..0ec355ee4e 100644 --- a/src/settings/plugins/ifcfg-rh/common.h +++ b/src/settings/plugins/ifcfg-rh/common.h @@ -41,7 +41,7 @@ #define IFCFG_DIR SYSCONFDIR"/sysconfig/network-scripts" #define IFCFG_PLUGIN_NAME "ifcfg-rh" -#define IFCFG_PLUGIN_INFO "(c) 2007 - 2013 Red Hat, Inc. To report bugs please use the NetworkManager mailing list." +#define IFCFG_PLUGIN_INFO "(c) 2007 - 2015 Red Hat, Inc. To report bugs please use the NetworkManager mailing list." #define TYPE_ETHERNET "Ethernet" #define TYPE_WIRELESS "Wireless" diff --git a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c index 014f69e44a..9005a375f0 100644 --- a/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c +++ b/src/settings/plugins/ifcfg-rh/nm-ifcfg-connection.c @@ -235,6 +235,7 @@ nm_ifcfg_connection_new (NMConnection *source, if (nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), tmp, update_unsaved, + NULL, error)) nm_ifcfg_connection_check_devtimeout (NM_IFCFG_CONNECTION (object)); else diff --git a/src/settings/plugins/ifcfg-rh/plugin.c b/src/settings/plugins/ifcfg-rh/plugin.c index 3594399fb7..b7efa8643a 100644 --- a/src/settings/plugins/ifcfg-rh/plugin.c +++ b/src/settings/plugins/ifcfg-rh/plugin.c @@ -58,10 +58,31 @@ #include "reader.h" #include "writer.h" #include "utils.h" +#include "gsystem-local-alloc.h" #define DBUS_SERVICE_NAME "com.redhat.ifcfgrh1" #define DBUS_OBJECT_PATH "/com/redhat/ifcfgrh1" + +#define _LOG_DEFAULT_DOMAIN LOGD_SETTINGS + +#define _LOG(level, domain, ...) \ + G_STMT_START { \ + nm_log ((level), (domain), \ + "%s" _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \ + "ifcfg-rh: " \ + _NM_UTILS_MACRO_REST(__VA_ARGS__)); \ + } G_STMT_END + +#define _LOGT(...) _LOG (LOGL_TRACE, _LOG_DEFAULT_DOMAIN, __VA_ARGS__) +#define _LOGD(...) _LOG (LOGL_DEBUG, _LOG_DEFAULT_DOMAIN, __VA_ARGS__) +#define _LOGI(...) _LOG (LOGL_INFO, _LOG_DEFAULT_DOMAIN, __VA_ARGS__) +#define _LOGW(...) _LOG (LOGL_WARN, _LOG_DEFAULT_DOMAIN, __VA_ARGS__) +#define _LOGE(...) _LOG (LOGL_ERR, _LOG_DEFAULT_DOMAIN, __VA_ARGS__) + +#define ERR_GET_MSG(err) (((err) && (err)->message) ? (err)->message : "(unknown)") + + static gboolean impl_ifcfgrh_get_ifcfg_details (SCPluginIfcfg *plugin, const char *in_ifcfg, const char **out_uuid, @@ -70,10 +91,13 @@ static gboolean impl_ifcfgrh_get_ifcfg_details (SCPluginIfcfg *plugin, #include "nm-ifcfg-rh-glue.h" -static void connection_new_or_changed (SCPluginIfcfg *plugin, - const char *path, - NMIfcfgConnection *existing, - char **out_old_path); +static NMIfcfgConnection *update_connection (SCPluginIfcfg *plugin, + NMConnection *source, + const char *full_path, + NMIfcfgConnection *connection, + gboolean protect_existing_connection, + GHashTable *protected_connections, + GError **error); static void system_config_interface_init (NMSystemConfigInterface *system_config_interface_class); @@ -104,13 +128,22 @@ typedef struct { static void connection_ifcfg_changed (NMIfcfgConnection *connection, gpointer user_data) { - SCPluginIfcfg *plugin = SC_PLUGIN_IFCFG (user_data); + SCPluginIfcfg *self = SC_PLUGIN_IFCFG (user_data); + SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (self); const char *path; path = nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection)); g_return_if_fail (path != NULL); - connection_new_or_changed (plugin, path, connection, NULL); + + if (!priv->ifcfg_monitor) { + _LOGD ("connection_ifcfg_changed("NM_IFCFG_CONNECTION_LOG_FMTD"): %s", NM_IFCFG_CONNECTION_LOG_ARGD (connection), "ignore event"); + return; + } + + _LOGD ("connection_ifcfg_changed("NM_IFCFG_CONNECTION_LOG_FMTD"): %s", NM_IFCFG_CONNECTION_LOG_ARGD (connection), "reload"); + + update_connection (self, NULL, path, connection, TRUE, NULL, NULL); } static void @@ -120,59 +153,6 @@ connection_removed_cb (NMSettingsConnection *obj, gpointer user_data) nm_connection_get_uuid (NM_CONNECTION (obj))); } -static NMIfcfgConnection * -_internal_new_connection (SCPluginIfcfg *self, - const char *path, - NMConnection *source, - GError **error) -{ - SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (self); - NMIfcfgConnection *connection; - const char *cid; - - if (!source) - nm_log_info (LOGD_SETTINGS, "parsing %s ... ", path); - - connection = nm_ifcfg_connection_new (source, path, error); - if (!connection) - return NULL; - - cid = nm_connection_get_id (NM_CONNECTION (connection)); - g_assert (cid); - - g_hash_table_insert (priv->connections, - g_strdup (nm_connection_get_uuid (NM_CONNECTION (connection))), - connection); - nm_log_info (LOGD_SETTINGS, " read connection '%s'", cid); - g_signal_connect (connection, NM_SETTINGS_CONNECTION_REMOVED, - G_CALLBACK (connection_removed_cb), - self); - - if (nm_ifcfg_connection_get_unmanaged_spec (connection)) { - const char *spec; - const char *device_id; - - spec = nm_ifcfg_connection_get_unmanaged_spec (connection); - device_id = strchr (spec, ':'); - if (device_id) - device_id++; - else - device_id = spec; - nm_log_warn (LOGD_SETTINGS, " Ignoring connection '%s' / device '%s' due to NM_CONTROLLED=no.", - cid, device_id); - } else if (nm_ifcfg_connection_get_unrecognized_spec (connection)) { - nm_log_warn (LOGD_SETTINGS, " Ignoring connection '%s' of unrecognized type.", cid); - } - - /* watch changes of ifcfg hardlinks */ - g_signal_connect (G_OBJECT (connection), "ifcfg-changed", - G_CALLBACK (connection_ifcfg_changed), self); - - return connection; -} - -/* Monitoring */ - static void remove_connection (SCPluginIfcfg *self, NMIfcfgConnection *connection) { @@ -182,6 +162,8 @@ remove_connection (SCPluginIfcfg *self, NMIfcfgConnection *connection) g_return_if_fail (self != NULL); g_return_if_fail (connection != NULL); + _LOGI ("remove "NM_IFCFG_CONNECTION_LOG_FMT, NM_IFCFG_CONNECTION_LOG_ARG (connection)); + unmanaged = !!nm_ifcfg_connection_get_unmanaged_spec (connection); unrecognized = !!nm_ifcfg_connection_get_unrecognized_spec (connection); @@ -215,142 +197,220 @@ find_by_path (SCPluginIfcfg *self, const char *path) } static NMIfcfgConnection * -find_by_uuid_from_path (SCPluginIfcfg *self, const char *path) +update_connection (SCPluginIfcfg *self, + NMConnection *source, + const char *full_path, + NMIfcfgConnection *connection, + gboolean protect_existing_connection, + GHashTable *protected_connections, + GError **error) { SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (self); - char *uuid; - - g_return_val_if_fail (path != NULL, NULL); + NMIfcfgConnection *connection_new; + NMIfcfgConnection *connection_by_uuid; + GError *local = NULL; + const char *new_unmanaged = NULL, *old_unmanaged = NULL; + const char *new_unrecognized = NULL, *old_unrecognized = NULL; + gboolean unmanaged_changed = FALSE, unrecognized_changed = FALSE; + const char *uuid; - uuid = uuid_from_file (path); - if (uuid) - return g_hash_table_lookup (priv->connections, uuid); - else + g_return_val_if_fail (!source || NM_IS_CONNECTION (source), NULL); + g_return_val_if_fail (full_path || source, NULL); + + if (full_path) + _LOGD ("loading from file \"%s\"...", full_path); + + /* Create a NMIfcfgConnection instance, either by reading from @full_path or + * based on @source. */ + connection_new = nm_ifcfg_connection_new (source, full_path, error); + if (!connection_new) { + /* Unexpected failure. Probably the file is invalid? */ + if ( connection + && !protect_existing_connection + && (!protected_connections || !g_hash_table_contains (protected_connections, connection))) + remove_connection (self, connection); return NULL; -} + } -static void -connection_new_or_changed (SCPluginIfcfg *self, - const char *path, - NMIfcfgConnection *existing, - char **out_old_path) -{ - SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (self); - NMIfcfgConnection *new; - GError *error = NULL; - const char *new_unmanaged = NULL, *old_unmanaged = NULL; - const char *new_unrecognized = NULL, *old_unrecognized = NULL; - gboolean unmanaged_changed, unrecognized_changed; + uuid = nm_connection_get_uuid (NM_CONNECTION (connection_new)); + connection_by_uuid = g_hash_table_lookup (priv->connections, uuid); - g_return_if_fail (self != NULL); - g_return_if_fail (path != NULL); + if ( connection + && connection != connection_by_uuid) { - if (out_old_path) - *out_old_path = NULL; - - if (!existing) { - /* See if it's a rename */ - existing = find_by_uuid_from_path (self, path); - if (existing) { - const char *old_path = nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (existing)); - nm_log_info (LOGD_SETTINGS, "renaming %s -> %s", old_path, path); - if (out_old_path) - *out_old_path = g_strdup (old_path); - nm_settings_connection_set_filename (NM_SETTINGS_CONNECTION (existing), path); - } - } + if ( (protect_existing_connection && connection_by_uuid != NULL) + || (protected_connections && g_hash_table_contains (protected_connections, connection))) { + NMIfcfgConnection *conflicting = (protect_existing_connection && connection_by_uuid != NULL) ? connection_by_uuid : connection; - if (!existing) { - /* New connection */ - new = _internal_new_connection (self, path, NULL, NULL); - if (new) { - if (nm_ifcfg_connection_get_unmanaged_spec (new)) - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED); - else if (nm_ifcfg_connection_get_unrecognized_spec (new)) - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNRECOGNIZED_SPECS_CHANGED); + if (source) + _LOGW ("cannot update protected connection "NM_IFCFG_CONNECTION_LOG_FMT" due to conflicting UUID %s", NM_IFCFG_CONNECTION_LOG_ARG (conflicting), uuid); else - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, new); + _LOGW ("cannot load %s due to conflicting UUID for "NM_IFCFG_CONNECTION_LOG_FMT, full_path, NM_IFCFG_CONNECTION_LOG_ARG (conflicting)); + g_object_unref (connection_new); + g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "Cannot update protected connection due to conflicting UUID"); + return NULL; } - return; - } - new = (NMIfcfgConnection *) nm_ifcfg_connection_new (NULL, path, NULL); - if (!new) { - /* errors reading connection; remove it */ - nm_log_info (LOGD_SETTINGS, "removed %s.", path); - remove_connection (self, existing); - return; + /* The new connection has a different UUID then the original one that we + * are about to update. Remove @connection. */ + remove_connection (self, connection); } - /* Successfully read connection changes */ + /* Check if the found connection with the same UUID is not protected from updating. */ + if ( connection_by_uuid + && ( (!connection && protect_existing_connection) + || (protected_connections && g_hash_table_contains (protected_connections, connection_by_uuid)))) { + if (source) + _LOGW ("cannot update connection due to conflicting UUID for "NM_IFCFG_CONNECTION_LOG_FMT, NM_IFCFG_CONNECTION_LOG_ARG (connection_by_uuid)); + else + _LOGW ("cannot load %s due to conflicting UUID for "NM_IFCFG_CONNECTION_LOG_FMT, full_path, NM_IFCFG_CONNECTION_LOG_ARG (connection_by_uuid)); + g_object_unref (connection_new); + g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "Skip updating protected connection during reload"); + return NULL; + } - old_unmanaged = nm_ifcfg_connection_get_unmanaged_spec (NM_IFCFG_CONNECTION (existing)); - new_unmanaged = nm_ifcfg_connection_get_unmanaged_spec (NM_IFCFG_CONNECTION (new)); + /* Evaluate unmanaged/unrecognized flags. */ + if (connection_by_uuid) + old_unmanaged = nm_ifcfg_connection_get_unmanaged_spec (connection_by_uuid); + new_unmanaged = nm_ifcfg_connection_get_unmanaged_spec (connection_new); unmanaged_changed = g_strcmp0 (old_unmanaged, new_unmanaged); - old_unrecognized = nm_ifcfg_connection_get_unrecognized_spec (NM_IFCFG_CONNECTION (existing)); - new_unrecognized = nm_ifcfg_connection_get_unrecognized_spec (NM_IFCFG_CONNECTION (new)); + if (connection_by_uuid) + old_unrecognized = nm_ifcfg_connection_get_unrecognized_spec (connection_by_uuid); + new_unrecognized = nm_ifcfg_connection_get_unrecognized_spec (connection_new); unrecognized_changed = g_strcmp0 (old_unrecognized, new_unrecognized); - if ( !unmanaged_changed - && !unrecognized_changed - && nm_connection_compare (NM_CONNECTION (existing), - NM_CONNECTION (new), - NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS | - NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) { - g_object_unref (new); - return; - } + if (connection_by_uuid) { + const char *old_path; + + old_path = nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection_by_uuid)); + + if ( !unmanaged_changed + && !unrecognized_changed + && nm_connection_compare (NM_CONNECTION (connection_by_uuid), + NM_CONNECTION (connection_new), + NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS | + NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) { + if (old_path && g_strcmp0 (old_path, full_path) != 0) + _LOGI ("rename \"%s\" to "NM_IFCFG_CONNECTION_LOG_FMT" without other changes", nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection_by_uuid)), NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + } else { + + /******************************************************* + * UPDATE + *******************************************************/ + + if (source) + _LOGI ("update "NM_IFCFG_CONNECTION_LOG_FMT" from %s", NM_IFCFG_CONNECTION_LOG_ARG (connection_new), NM_IFCFG_CONNECTION_LOG_PATH (old_path)); + else if (!g_strcmp0 (old_path, nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection_new)))) + _LOGI ("update "NM_IFCFG_CONNECTION_LOG_FMT, NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + else if (old_path) + _LOGI ("rename \"%s\" to "NM_IFCFG_CONNECTION_LOG_FMT, old_path, NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + else + _LOGI ("update and persist "NM_IFCFG_CONNECTION_LOG_FMT, NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + + g_object_set (connection_by_uuid, + NM_IFCFG_CONNECTION_UNMANAGED_SPEC, new_unmanaged, + NM_IFCFG_CONNECTION_UNRECOGNIZED_SPEC, new_unrecognized, + NULL); + + if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (connection_by_uuid), + NM_CONNECTION (connection_new), + FALSE, /* don't set Unsaved */ + "ifcfg-update", + &local)) { + /* Shouldn't ever get here as 'connection_new' was verified by the reader already + * and the UUID did not change. */ + g_assert_not_reached (); + } + g_assert_no_error (local); + + if (new_unmanaged || new_unrecognized) { + if (!old_unmanaged && !old_unrecognized) { + g_object_ref (connection_by_uuid); + /* Unexport the connection by telling the settings service it's + * been removed. + */ + nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection_by_uuid)); + /* Remove the path so that claim_connection() doesn't complain later when + * interface gets managed and connection is re-added. */ + nm_connection_set_path (NM_CONNECTION (connection_by_uuid), NULL); + + /* signal_remove() will end up removing the connection from our hash, + * so add it back now. + */ + g_hash_table_insert (priv->connections, + g_strdup (nm_connection_get_uuid (NM_CONNECTION (connection_by_uuid))), + connection_by_uuid); + } + } else { + if (old_unmanaged /* && !new_unmanaged */) { + _LOGI ("Managing connection "NM_IFCFG_CONNECTION_LOG_FMT" and its device because NM_CONTROLLED was true.", + NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection_by_uuid); + } else if (old_unrecognized /* && !new_unrecognized */) { + _LOGI ("Managing connection "NM_IFCFG_CONNECTION_LOG_FMT" because it is now a recognized type.", + NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection_by_uuid); + } + } - nm_log_info (LOGD_SETTINGS, "updating %s", path); - g_object_set (existing, - NM_IFCFG_CONNECTION_UNMANAGED_SPEC, new_unmanaged, - NM_IFCFG_CONNECTION_UNRECOGNIZED_SPEC, new_unrecognized, - NULL); - - if (new_unmanaged || new_unrecognized) { - if (!old_unmanaged && !old_unrecognized) { - g_object_ref (existing); - /* Unexport the connection by telling the settings service it's - * been removed. - */ - nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (existing)); - /* Remove the path so that claim_connection() doesn't complain later when - * interface gets managed and connection is re-added. */ - nm_connection_set_path (NM_CONNECTION (existing), NULL); - - /* signal_remove() will end up removing the connection from our hash, - * so add it back now. - */ - g_hash_table_insert (priv->connections, - g_strdup (nm_connection_get_uuid (NM_CONNECTION (existing))), - existing); + if (unmanaged_changed) + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED); + if (unrecognized_changed) + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNRECOGNIZED_SPECS_CHANGED); } + nm_settings_connection_set_filename (NM_SETTINGS_CONNECTION (connection_by_uuid), full_path); + g_object_unref (connection_new); + return connection_by_uuid; } else { - const char *cid = nm_connection_get_id (NM_CONNECTION (new)); - - if (old_unmanaged /* && !new_unmanaged */) { - nm_log_info (LOGD_SETTINGS, "Managing connection '%s' and its device because NM_CONTROLLED was true.", cid); - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, existing); - } else if (old_unrecognized /* && !new_unrecognized */) { - nm_log_info (LOGD_SETTINGS, "Managing connection '%s' because it is now a recognized type.", cid); - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, existing); - } - if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (existing), - NM_CONNECTION (new), - FALSE, /* don't set Unsaved */ - &error)) { - /* Shouldn't ever get here as 'new' was verified by the reader already */ - g_assert_no_error (error); + /******************************************************* + * ADD + *******************************************************/ + + if (source) + _LOGI ("add connection "NM_IFCFG_CONNECTION_LOG_FMT, NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + else + _LOGI ("new connection "NM_IFCFG_CONNECTION_LOG_FMT, NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + g_hash_table_insert (priv->connections, g_strdup (uuid), connection_new); + + g_signal_connect (connection_new, NM_SETTINGS_CONNECTION_REMOVED, + G_CALLBACK (connection_removed_cb), + self); + + if (nm_ifcfg_connection_get_unmanaged_spec (connection_new)) { + const char *spec; + const char *device_id; + + spec = nm_ifcfg_connection_get_unmanaged_spec (connection_new); + device_id = strchr (spec, ':'); + if (device_id) + device_id++; + else + device_id = spec; + _LOGW ("Ignoring connection "NM_IFCFG_CONNECTION_LOG_FMT" / device '%s' due to NM_CONTROLLED=no.", + NM_IFCFG_CONNECTION_LOG_ARG (connection_new), device_id); + } else if (nm_ifcfg_connection_get_unrecognized_spec (connection_new)) + _LOGW ("Ignoring connection "NM_IFCFG_CONNECTION_LOG_FMT" of unrecognized type.", NM_IFCFG_CONNECTION_LOG_ARG (connection_new)); + + /* watch changes of ifcfg hardlinks */ + g_signal_connect (G_OBJECT (connection_new), "ifcfg-changed", + G_CALLBACK (connection_ifcfg_changed), self); + + if (!source) { + /* Only raise the signal if we were called without source, i.e. if we read the connection from file. + * Otherwise, we were called by add_connection() which does not expect the signal. */ + if (nm_ifcfg_connection_get_unmanaged_spec (connection_new)) + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED); + else if (nm_ifcfg_connection_get_unrecognized_spec (connection_new)) + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNRECOGNIZED_SPECS_CHANGED); + else + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection_new); } + return connection_new; } - g_object_unref (new); - - if (unmanaged_changed) - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNMANAGED_SPECS_CHANGED); - if (unrecognized_changed) - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_UNRECOGNIZED_SPECS_CHANGED); } static void @@ -370,6 +430,8 @@ ifcfg_dir_changed (GFileMonitor *monitor, return; } + _LOGD ("ifcfg_dir_changed(%s) = %d", path, event_type); + base = g_file_get_basename (file); if (utils_is_ifcfg_alias_file (base, NULL)) { /* Alias file changed. Get the base ifcfg file from it */ @@ -382,14 +444,13 @@ ifcfg_dir_changed (GFileMonitor *monitor, connection = find_by_path (plugin, ifcfg_path); switch (event_type) { case G_FILE_MONITOR_EVENT_DELETED: - nm_log_info (LOGD_SETTINGS, "removed %s.", ifcfg_path); if (connection) remove_connection (plugin, connection); break; case G_FILE_MONITOR_EVENT_CREATED: case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT: /* Update or new */ - connection_new_or_changed (plugin, ifcfg_path, connection, NULL); + update_connection (plugin, NULL, ifcfg_path, connection, TRUE, NULL, NULL); break; default: break; @@ -418,6 +479,43 @@ setup_ifcfg_monitoring (SCPluginIfcfg *plugin) } } +static GHashTable * +_paths_from_connections (GHashTable *connections) +{ + GHashTableIter iter; + NMIfcfgConnection *connection; + GHashTable *paths = g_hash_table_new (g_str_hash, g_str_equal); + + g_hash_table_iter_init (&iter, connections); + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &connection)) { + const char *path = nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection)); + + if (path) + g_hash_table_add (paths, (void *) path); + } + return paths; +} + +static int +_sort_paths (const char **f1, const char **f2, GHashTable *paths) +{ + struct stat st; + gboolean c1, c2; + gint64 m1, m2; + + c1 = !!g_hash_table_contains (paths, *f1); + c2 = !!g_hash_table_contains (paths, *f2); + if (c1 != c2) + return c1 ? -1 : 1; + + m1 = stat (*f1, &st) == 0 ? (gint64) st.st_mtime : G_MININT64; + m2 = stat (*f2, &st) == 0 ? (gint64) st.st_mtime : G_MININT64; + if (m1 != m2) + return m1 > m2 ? -1 : 1; + + return strcmp (*f1, *f2); +} + static void read_connections (SCPluginIfcfg *plugin) { @@ -425,28 +523,26 @@ read_connections (SCPluginIfcfg *plugin) GDir *dir; GError *err = NULL; const char *item; - GHashTable *oldconns; + GHashTable *alive_connections; GHashTableIter iter; - gpointer key, value; NMIfcfgConnection *connection; + GPtrArray *dead_connections = NULL; + guint i; + GPtrArray *filenames; + GHashTable *paths; dir = g_dir_open (IFCFG_DIR, 0, &err); if (!dir) { - nm_log_warn (LOGD_SETTINGS, "Could not read directory '%s': %s", IFCFG_DIR, err->message); + _LOGW ("Could not read directory '%s': %s", IFCFG_DIR, err->message); g_error_free (err); return; } - oldconns = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); - g_hash_table_iter_init (&iter, priv->connections); - while (g_hash_table_iter_next (&iter, NULL, &value)) { - const char *ifcfg_path = nm_settings_connection_get_filename (value); - if (ifcfg_path) - g_hash_table_insert (oldconns, g_strdup (ifcfg_path), value); - } + alive_connections = g_hash_table_new (NULL, NULL); + filenames = g_ptr_array_new_with_free_func (g_free); while ((item = g_dir_read_name (dir))) { - char *full_path, *old_path; + char *full_path; if (utils_should_ignore_file (item, TRUE)) continue; @@ -455,31 +551,45 @@ read_connections (SCPluginIfcfg *plugin) full_path = g_build_filename (IFCFG_DIR, item, NULL); if (!utils_get_ifcfg_name (full_path, TRUE)) - goto next; + g_free (full_path); + else + g_ptr_array_add (filenames, full_path); + } + g_dir_close (dir); - connection = g_hash_table_lookup (oldconns, full_path); - g_hash_table_remove (oldconns, full_path); - connection_new_or_changed (plugin, full_path, connection, &old_path); + /* While reloading, we don't replace connections that we already loaded while + * iterating over the files. + * + * To have sensible, reproducible behavior, sort the paths by last modification + * time prefering older files. + */ + paths = _paths_from_connections (priv->connections); + g_ptr_array_sort_with_data (filenames, (GCompareDataFunc) _sort_paths, paths); + g_hash_table_destroy (paths); + + for (i = 0; i < filenames->len; i++) { + connection = update_connection (plugin, NULL, filenames->pdata[i], NULL, FALSE, alive_connections, NULL); + if (connection) + g_hash_table_add (alive_connections, connection); + } + g_ptr_array_free (filenames, TRUE); - if (old_path) { - g_hash_table_remove (oldconns, old_path); - g_free (old_path); + g_hash_table_iter_init (&iter, priv->connections); + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &connection)) { + if ( !g_hash_table_contains (alive_connections, connection) + && nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection))) { + if (!dead_connections) + dead_connections = g_ptr_array_new (); + g_ptr_array_add (dead_connections, connection); } - - next: - g_free (full_path); } + g_hash_table_destroy (alive_connections); - g_dir_close (dir); - - g_hash_table_iter_init (&iter, oldconns); - while (g_hash_table_iter_next (&iter, &key, &value)) { - nm_log_info (LOGD_SETTINGS, "removed %s.", (char *)key); - g_hash_table_iter_remove (&iter); - remove_connection (plugin, value); + if (dead_connections) { + for (i = 0; i < dead_connections->len; i++) + remove_connection (plugin, dead_connections->pdata[i]); + g_ptr_array_free (dead_connections, TRUE); } - - g_hash_table_destroy (oldconns); } static GSList * @@ -525,7 +635,7 @@ load_connection (NMSystemConfigInterface *config, return FALSE; connection = find_by_path (plugin, filename); - connection_new_or_changed (plugin, filename, connection, NULL); + update_connection (plugin, NULL, filename, connection, TRUE, NULL, NULL); if (!connection) connection = find_by_path (plugin, filename); @@ -590,8 +700,7 @@ add_connection (NMSystemConfigInterface *config, GError **error) { SCPluginIfcfg *self = SC_PLUGIN_IFCFG (config); - NMIfcfgConnection *added = NULL; - char *path = NULL; + gs_free char *path = NULL; /* Ensure we reject attempts to add the connection long before we're * asked to write it to disk. @@ -603,10 +712,7 @@ add_connection (NMSystemConfigInterface *config, if (!writer_new_connection (connection, IFCFG_DIR, &path, error)) return NULL; } - - added = _internal_new_connection (self, path, connection, error); - g_free (path); - return (NMSettingsConnection *) added; + return NM_SETTINGS_CONNECTION (update_connection (self, connection, path, NULL, FALSE, NULL, error)); } #define SC_NETWORK_FILE "/etc/sysconfig/network" @@ -626,7 +732,7 @@ plugin_get_hostname (SCPluginIfcfg *plugin) network = svOpenFile (SC_NETWORK_FILE, NULL); if (!network) { - nm_log_warn (LOGD_SETTINGS, "Could not get hostname: failed to read " SC_NETWORK_FILE); + _LOGW ("Could not get hostname: failed to read " SC_NETWORK_FILE); return NULL; } @@ -678,7 +784,7 @@ plugin_set_hostname (SCPluginIfcfg *plugin, const char *hostname) #endif if (!ret) { - nm_log_warn (LOGD_SETTINGS, "Could not save hostname: failed to create/open " HOSTNAME_FILE); + _LOGW ("Could not save hostname: failed to create/open " HOSTNAME_FILE); g_free (hostname_eol); return FALSE; } @@ -847,7 +953,7 @@ sc_plugin_ifcfg_init (SCPluginIfcfg *plugin) priv->bus = dbus_g_bus_get (DBUS_BUS_SYSTEM, &error); if (!priv->bus) { - nm_log_warn (LOGD_SETTINGS, "Couldn't connect to D-Bus: %s", error->message); + _LOGW ("Couldn't connect to D-Bus: %s", error->message); g_clear_error (&error); } else { DBusConnection *tmp; @@ -868,10 +974,10 @@ sc_plugin_ifcfg_init (SCPluginIfcfg *plugin) G_TYPE_INVALID, G_TYPE_UINT, &result, G_TYPE_INVALID)) { - nm_log_warn (LOGD_SETTINGS, "Couldn't acquire D-Bus service: %s", error->message); + _LOGW ("Couldn't acquire D-Bus service: %s", error->message); g_clear_error (&error); } else if (result != DBUS_REQUEST_NAME_REPLY_PRIMARY_OWNER) { - nm_log_warn (LOGD_SETTINGS, "Couldn't acquire ifcfgrh1 D-Bus service (already taken)"); + _LOGW ("Couldn't acquire ifcfgrh1 D-Bus service (already taken)"); } else success = TRUE; } @@ -1033,7 +1139,7 @@ nm_system_config_factory (void) dbus_g_connection_register_g_object (priv->bus, DBUS_OBJECT_PATH, G_OBJECT (singleton)); - nm_log_info (LOGD_SETTINGS, "Acquired D-Bus service %s", DBUS_SERVICE_NAME); + _LOGI ("Acquired D-Bus service %s", DBUS_SERVICE_NAME); } else g_object_ref (singleton); diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh-utils.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh-utils.c index d82a327032..8494c63825 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh-utils.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh-utils.c @@ -121,7 +121,7 @@ int main (int argc, char **argv) { char *base; - nmtst_init (&argc, &argv, TRUE); + nmtst_init_assert_logging (&argc, &argv); /* The tests */ test_get_ifcfg_name ("get-ifcfg-name-bad", "/foo/bar/adfasdfadf", FALSE, NULL); diff --git a/src/settings/plugins/ifcfg-rh/utils.h b/src/settings/plugins/ifcfg-rh/utils.h index 27c5e46104..445437c48b 100644 --- a/src/settings/plugins/ifcfg-rh/utils.h +++ b/src/settings/plugins/ifcfg-rh/utils.h @@ -25,6 +25,13 @@ #include <nm-connection.h> #include "shvar.h" #include "common.h" +#include "nm-logging.h" + +#define NM_IFCFG_CONNECTION_LOG_PATH(path) str_if_set (path,"in-memory") +#define NM_IFCFG_CONNECTION_LOG_FMT "%s (%s,\"%s\")" +#define NM_IFCFG_CONNECTION_LOG_ARG(con) NM_IFCFG_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_connection_get_uuid ((NMConnection *) (con)), nm_connection_get_id ((NMConnection *) (con)) +#define NM_IFCFG_CONNECTION_LOG_FMTD "%s (%s,\"%s\",%p)" +#define NM_IFCFG_CONNECTION_LOG_ARGD(con) NM_IFCFG_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_connection_get_uuid ((NMConnection *) (con)), nm_connection_get_id ((NMConnection *) (con)), (con) char *utils_single_quote_string (const char *str); diff --git a/src/settings/plugins/ifnet/nm-ifnet-connection.c b/src/settings/plugins/ifnet/nm-ifnet-connection.c index c84ad5ed95..693e653288 100644 --- a/src/settings/plugins/ifnet/nm-ifnet-connection.c +++ b/src/settings/plugins/ifnet/nm-ifnet-connection.c @@ -82,6 +82,7 @@ nm_ifnet_connection_new (NMConnection *source, const char *conn_name) nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), tmp, update_unsaved, + NULL, NULL); g_object_unref (tmp); diff --git a/src/settings/plugins/ifnet/plugin.c b/src/settings/plugins/ifnet/plugin.c index 6896a4da1a..33711639da 100644 --- a/src/settings/plugins/ifnet/plugin.c +++ b/src/settings/plugins/ifnet/plugin.c @@ -313,10 +313,13 @@ reload_connections (NMSystemConfigInterface *config) if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (old), NM_CONNECTION (new), FALSE, /* don't set Unsaved */ + "ifnet-update", &error)) { - /* Shouldn't ever get here as 'new' was verified by the reader already */ - g_assert_no_error (error); + /* Shouldn't ever get here as 'new' was verified by the reader already + * and the UUID did not change. */ + g_assert_not_reached (); } + g_assert_no_error (error); nm_log_info (LOGD_SETTINGS, "Connection %s updated", nm_connection_get_id (NM_CONNECTION (new))); } diff --git a/src/settings/plugins/keyfile/common.h b/src/settings/plugins/keyfile/common.h index db6569e54a..7bde4bf3a9 100644 --- a/src/settings/plugins/keyfile/common.h +++ b/src/settings/plugins/keyfile/common.h @@ -24,7 +24,7 @@ #include <glib.h> #define KEYFILE_PLUGIN_NAME "keyfile" -#define KEYFILE_PLUGIN_INFO "(c) 2007 - 2013 Red Hat, Inc. To report bugs please use the NetworkManager mailing list." +#define KEYFILE_PLUGIN_INFO "(c) 2007 - 2015 Red Hat, Inc. To report bugs please use the NetworkManager mailing list." #define KEYFILE_DIR NMCONFDIR "/system-connections" diff --git a/src/settings/plugins/keyfile/nm-keyfile-connection.c b/src/settings/plugins/keyfile/nm-keyfile-connection.c index 9ff65a20aa..8fb94302d7 100644 --- a/src/settings/plugins/keyfile/nm-keyfile-connection.c +++ b/src/settings/plugins/keyfile/nm-keyfile-connection.c @@ -76,6 +76,7 @@ nm_keyfile_connection_new (NMConnection *source, if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (object), tmp, update_unsaved, + NULL, error)) { g_object_unref (object); object = NULL; diff --git a/src/settings/plugins/keyfile/plugin.c b/src/settings/plugins/keyfile/plugin.c index ead074362e..fcdb3c3cf0 100644 --- a/src/settings/plugins/keyfile/plugin.c +++ b/src/settings/plugins/keyfile/plugin.c @@ -45,6 +45,7 @@ #include "writer.h" #include "common.h" #include "utils.h" +#include "gsystem-local-alloc.h" static char *plugin_get_hostname (SCPluginKeyfile *plugin); static void system_config_interface_init (NMSystemConfigInterface *system_config_interface_class); @@ -87,7 +88,7 @@ remove_connection (SCPluginKeyfile *self, NMKeyfileConnection *connection) g_return_if_fail (connection != NULL); - nm_log_info (LOGD_SETTINGS, "removed %s.", nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection))); + nm_log_info (LOGD_SETTINGS, "keyfile: removed " NM_KEYFILE_CONNECTION_LOG_FMT, NM_KEYFILE_CONNECTION_LOG_ARG (connection)); /* Removing from the hash table should drop the last reference */ g_object_ref (connection); @@ -100,40 +101,6 @@ remove_connection (SCPluginKeyfile *self, NMKeyfileConnection *connection) g_return_if_fail (removed); } -static void -update_connection (SCPluginKeyfile *self, - NMKeyfileConnection *connection, - const char *name) -{ - NMKeyfileConnection *tmp; - GError *error = NULL; - - tmp = nm_keyfile_connection_new (NULL, name, &error); - if (!tmp) { - /* Error; remove the connection */ - nm_log_warn (LOGD_SETTINGS, " error in connection %s: %s", name, - (error && error->message) ? error->message : "(unknown)"); - g_clear_error (&error); - remove_connection (self, connection); - return; - } - - if (!nm_connection_compare (NM_CONNECTION (connection), - NM_CONNECTION (tmp), - NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS | - NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) { - nm_log_info (LOGD_SETTINGS, "updating %s", name); - if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (connection), - NM_CONNECTION (tmp), - FALSE, /* don't set Unsaved */ - &error)) { - /* Shouldn't ever get here as 'new' was verified by the reader already */ - g_assert_no_error (error); - } - } - g_object_unref (tmp); -} - static NMKeyfileConnection * find_by_path (SCPluginKeyfile *self, const char *path) { @@ -151,52 +118,165 @@ find_by_path (SCPluginKeyfile *self, const char *path) return NULL; } -static void -new_connection (SCPluginKeyfile *self, - const char *name, - char **out_old_path) +/* update_connection: + * @self: the plugin instance + * @source: if %NULL, this re-reads the connection from @full_path + * and updates it. When passing @source, this adds a connection from + * memory. + * @full_path: the filename of the keyfile to be loaded + * @connection: an existing connection that might be updated. + * If given, @connection must be an existing connection that is currently + * owned by the plugin. + * @protect_existing_connection: if %TRUE, and !@connection, we don't allow updating + * an existing connection with the same UUID. + * If %TRUE and @connection, allow updating only if the reload would modify + * @connection (without changing its UUID) or if we would create a new connection. + * In other words, if this paramter is %TRUE, we only allow creating a + * new connection (with an unseen UUID) or updating the passed in @connection + * (whereas the UUID cannot change). + * Note, that this allows for @connection to be replaced by a new connection. + * @protected_connections: (allow-none): if given, we only update an + * existing connection if it is not contained in this hash. + * @error: error in case of failure + * + * Loads a connection from file @full_path. This can both be used to + * load a connection initially or to update an existing connection. + * + * If you pass in an existing connection and the reloaded file happens + * to have a different UUID, the connection is deleted. + * Beware, that means that after the function, you have a dangling pointer + * if the returned connection is different from @connection. + * + * Returns: the updated connection. + * */ +static NMKeyfileConnection * +update_connection (SCPluginKeyfile *self, + NMConnection *source, + const char *full_path, + NMKeyfileConnection *connection, + gboolean protect_existing_connection, + GHashTable *protected_connections, + GError **error) { SCPluginKeyfilePrivate *priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (self); - NMKeyfileConnection *tmp; - NMSettingsConnection *connection; - GError *error = NULL; + NMKeyfileConnection *connection_new; + NMKeyfileConnection *connection_by_uuid; + GError *local = NULL; const char *uuid; - if (out_old_path) - *out_old_path = NULL; + g_return_val_if_fail (!source || NM_IS_CONNECTION (source), NULL); + g_return_val_if_fail (full_path || source, NULL); - tmp = nm_keyfile_connection_new (NULL, name, &error); - if (!tmp) { - nm_log_warn (LOGD_SETTINGS, " error in connection %s: %s", name, - (error && error->message) ? error->message : "(unknown)"); - g_clear_error (&error); - return; + if (full_path) + nm_log_dbg (LOGD_SETTINGS, "keyfile: loading from file \"%s\"...", full_path); + + connection_new = nm_keyfile_connection_new (source, full_path, &local); + if (!connection_new) { + /* Error; remove the connection */ + if (source) + nm_log_warn (LOGD_SETTINGS, "keyfile: error creating connection %s: %s", nm_connection_get_uuid (source), local->message); + else + nm_log_warn (LOGD_SETTINGS, "keyfile: error loading connection from file %s: %s", full_path, local->message); + if ( connection + && !protect_existing_connection + && (!protected_connections || !g_hash_table_contains (protected_connections, connection))) + remove_connection (self, connection); + g_propagate_error (error, local); + return NULL; + } + + uuid = nm_connection_get_uuid (NM_CONNECTION (connection_new)); + connection_by_uuid = g_hash_table_lookup (priv->connections, uuid); + + if ( connection + && connection != connection_by_uuid) { + + if ( (protect_existing_connection && connection_by_uuid != NULL) + || (protected_connections && g_hash_table_contains (protected_connections, connection))) { + NMKeyfileConnection *conflicting = (protect_existing_connection && connection_by_uuid != NULL) ? connection_by_uuid : connection; + + if (source) + nm_log_warn (LOGD_SETTINGS, "keyfile: cannot update protected "NM_KEYFILE_CONNECTION_LOG_FMT" connection due to conflicting UUID %s", NM_KEYFILE_CONNECTION_LOG_ARG (conflicting), uuid); + else + nm_log_warn (LOGD_SETTINGS, "keyfile: cannot load %s due to conflicting UUID for "NM_KEYFILE_CONNECTION_LOG_FMT, full_path, NM_KEYFILE_CONNECTION_LOG_ARG (conflicting)); + g_object_unref (connection_new); + g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "Cannot update protected connection due to conflicting UUID"); + return NULL; + } + + /* The new connection has a different UUID then the original one. + * Remove @connection. */ + remove_connection (self, connection); } - /* Connection renames will show as different paths but same UUID */ - uuid = nm_connection_get_uuid (NM_CONNECTION (tmp)); - connection = g_hash_table_lookup (priv->connections, uuid); - if (connection) { - nm_log_info (LOGD_SETTINGS, "rename %s -> %s", nm_settings_connection_get_filename (connection), name); - if (!nm_settings_connection_replace_settings (connection, - NM_CONNECTION (tmp), - FALSE, /* don't set Unsaved */ - &error)) { - /* Shouldn't ever get here as 'tmp' was verified by the reader already */ - g_assert_no_error (error); + if ( connection_by_uuid + && ( (!connection && protect_existing_connection) + || (protected_connections && g_hash_table_contains (protected_connections, connection_by_uuid)))) { + if (source) + nm_log_warn (LOGD_SETTINGS, "keyfile: cannot update connection due to conflicting UUID for "NM_KEYFILE_CONNECTION_LOG_FMT, NM_KEYFILE_CONNECTION_LOG_ARG (connection_by_uuid)); + else + nm_log_warn (LOGD_SETTINGS, "keyfile: cannot load %s due to conflicting UUID for "NM_KEYFILE_CONNECTION_LOG_FMT, full_path, NM_KEYFILE_CONNECTION_LOG_ARG (connection_by_uuid)); + g_object_unref (connection_new); + g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, + "Skip updating protected connection during reload"); + return NULL; + } + + if (connection_by_uuid) { + const char *old_path; + + old_path = nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection_by_uuid)); + + if (nm_connection_compare (NM_CONNECTION (connection_by_uuid), + NM_CONNECTION (connection_new), + NM_SETTING_COMPARE_FLAG_IGNORE_AGENT_OWNED_SECRETS | + NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) { + /* Nothing to do... except updating the path. */ + if (old_path && g_strcmp0 (old_path, full_path) != 0) + nm_log_info (LOGD_SETTINGS, "keyfile: rename \"%s\" to "NM_KEYFILE_CONNECTION_LOG_FMT" without other changes", old_path, NM_KEYFILE_CONNECTION_LOG_ARG (connection_new)); + } else { + /* An existing connection changed. */ + if (source) + nm_log_info (LOGD_SETTINGS, "keyfile: update "NM_KEYFILE_CONNECTION_LOG_FMT" from %s", NM_KEYFILE_CONNECTION_LOG_ARG (connection_new), NM_KEYFILE_CONNECTION_LOG_PATH (old_path)); + else if (!g_strcmp0 (old_path, nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection_new)))) + nm_log_info (LOGD_SETTINGS, "keyfile: update "NM_KEYFILE_CONNECTION_LOG_FMT, NM_KEYFILE_CONNECTION_LOG_ARG (connection_new)); + else if (old_path) + nm_log_info (LOGD_SETTINGS, "keyfile: rename \"%s\" to "NM_KEYFILE_CONNECTION_LOG_FMT, old_path, NM_KEYFILE_CONNECTION_LOG_ARG (connection_new)); + else + nm_log_info (LOGD_SETTINGS, "keyfile: update and persist "NM_KEYFILE_CONNECTION_LOG_FMT, NM_KEYFILE_CONNECTION_LOG_ARG (connection_new)); + + if (!nm_settings_connection_replace_settings (NM_SETTINGS_CONNECTION (connection_by_uuid), + NM_CONNECTION (connection_new), + FALSE, /* don't set Unsaved */ + "keyfile-update", + &local)) { + /* Shouldn't ever get here as 'connection_new' was verified by the reader already + * and the UUID did not change. */ + g_assert_not_reached (); + } + g_assert_no_error (local); } - g_object_unref (tmp); - if (out_old_path) - *out_old_path = g_strdup (nm_settings_connection_get_filename (connection)); - nm_settings_connection_set_filename (connection, name); + nm_settings_connection_set_filename (NM_SETTINGS_CONNECTION (connection_by_uuid), full_path); + g_object_unref (connection_new); + return connection_by_uuid; } else { - nm_log_info (LOGD_SETTINGS, "new connection %s", name); - g_hash_table_insert (priv->connections, g_strdup (uuid), tmp); - g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, tmp); + if (source) + nm_log_info (LOGD_SETTINGS, "keyfile: add connection "NM_KEYFILE_CONNECTION_LOG_FMT, NM_KEYFILE_CONNECTION_LOG_ARG (connection_new)); + else + nm_log_info (LOGD_SETTINGS, "keyfile: new connection "NM_KEYFILE_CONNECTION_LOG_FMT, NM_KEYFILE_CONNECTION_LOG_ARG (connection_new)); + g_hash_table_insert (priv->connections, g_strdup (uuid), connection_new); - g_signal_connect (tmp, NM_SETTINGS_CONNECTION_REMOVED, + g_signal_connect (connection_new, NM_SETTINGS_CONNECTION_REMOVED, G_CALLBACK (connection_removed_cb), self); + + if (!source) { + /* Only raise the signal if we were called without source, i.e. if we read the connection from file. + * Otherwise, we were called by add_connection() which does not expect the signal. */ + g_signal_emit_by_name (self, NM_SYSTEM_CONFIG_INTERFACE_CONNECTION_ADDED, connection_new); + } + return connection_new; } } @@ -211,26 +291,28 @@ dir_changed (GFileMonitor *monitor, SCPluginKeyfile *self = SC_PLUGIN_KEYFILE (config); NMKeyfileConnection *connection; char *full_path; + gboolean exists; full_path = g_file_get_path (file); if (nm_keyfile_plugin_utils_should_ignore_file (full_path)) { g_free (full_path); return; } + exists = g_file_test (full_path, G_FILE_TEST_EXISTS); + + nm_log_dbg (LOGD_SETTINGS, "dir_changed(%s) = %d; file %s", full_path, event_type, exists ? "exists" : "does not exist"); connection = find_by_path (self, full_path); switch (event_type) { case G_FILE_MONITOR_EVENT_DELETED: - if (connection) + if (!exists && connection) remove_connection (SC_PLUGIN_KEYFILE (config), connection); break; case G_FILE_MONITOR_EVENT_CREATED: case G_FILE_MONITOR_EVENT_CHANGES_DONE_HINT: - if (connection) - update_connection (SC_PLUGIN_KEYFILE (config), connection, full_path); - else - new_connection (SC_PLUGIN_KEYFILE (config), full_path, NULL); + if (exists) + update_connection (SC_PLUGIN_KEYFILE (config), NULL, full_path, connection, TRUE, NULL, NULL); break; default: break; @@ -306,6 +388,43 @@ setup_monitoring (NMSystemConfigInterface *config) } } +static GHashTable * +_paths_from_connections (GHashTable *connections) +{ + GHashTableIter iter; + NMKeyfileConnection *connection; + GHashTable *paths = g_hash_table_new (g_str_hash, g_str_equal); + + g_hash_table_iter_init (&iter, connections); + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &connection)) { + const char *path = nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection)); + + if (path) + g_hash_table_add (paths, (void *) path); + } + return paths; +} + +static int +_sort_paths (const char **f1, const char **f2, GHashTable *paths) +{ + struct stat st; + gboolean c1, c2; + gint64 m1, m2; + + c1 = !!g_hash_table_contains (paths, *f1); + c2 = !!g_hash_table_contains (paths, *f2); + if (c1 != c2) + return c1 ? -1 : 1; + + m1 = stat (*f1, &st) == 0 ? (gint64) st.st_mtime : G_MININT64; + m2 = stat (*f2, &st) == 0 ? (gint64) st.st_mtime : G_MININT64; + if (m1 != m2) + return m1 > m2 ? -1 : 1; + + return strcmp (*f1, *f2); +} + static void read_connections (NMSystemConfigInterface *config) { @@ -314,13 +433,17 @@ read_connections (NMSystemConfigInterface *config) GDir *dir; GError *error = NULL; const char *item; - GHashTable *oldconns; + GHashTable *alive_connections; GHashTableIter iter; - gpointer data; + NMKeyfileConnection *connection; + GPtrArray *dead_connections = NULL; + guint i; + GPtrArray *filenames; + GHashTable *paths; dir = g_dir_open (KEYFILE_DIR, 0, &error); if (!dir) { - nm_log_warn (LOGD_SETTINGS, "Cannot read directory '%s': (%d) %s", + nm_log_warn (LOGD_SETTINGS, "keyfile: cannot read directory '%s': (%d) %s", KEYFILE_DIR, error ? error->code : -1, error && error->message ? error->message : "(unknown)"); @@ -328,45 +451,49 @@ read_connections (NMSystemConfigInterface *config) return; } - oldconns = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); - g_hash_table_iter_init (&iter, priv->connections); - while (g_hash_table_iter_next (&iter, NULL, &data)) { - const char *con_path = nm_settings_connection_get_filename (data); - if (con_path) - g_hash_table_insert (oldconns, g_strdup (con_path), data); - } + alive_connections = g_hash_table_new (NULL, NULL); + filenames = g_ptr_array_new_with_free_func (g_free); while ((item = g_dir_read_name (dir))) { - NMKeyfileConnection *connection; - char *full_path, *old_path; - if (nm_keyfile_plugin_utils_should_ignore_file (item)) continue; + g_ptr_array_add (filenames, g_build_filename (KEYFILE_DIR, item, NULL)); + } + g_dir_close (dir); - full_path = g_build_filename (KEYFILE_DIR, item, NULL); + /* While reloading, we don't replace connections that we already loaded while + * iterating over the files. + * + * To have sensible, reproducible behavior, sort the paths by last modification + * time prefering older files. + */ + paths = _paths_from_connections (priv->connections); + g_ptr_array_sort_with_data (filenames, (GCompareDataFunc) _sort_paths, paths); + g_hash_table_destroy (paths); + + for (i = 0; i < filenames->len; i++) { + connection = update_connection (self, NULL, filenames->pdata[i], NULL, FALSE, alive_connections, NULL); + if (connection) + g_hash_table_add (alive_connections, connection); + } + g_ptr_array_free (filenames, TRUE); - connection = g_hash_table_lookup (oldconns, full_path); - if (connection) { - g_hash_table_remove (oldconns, full_path); - update_connection (self, connection, full_path); - } else { - new_connection (self, full_path, &old_path); - if (old_path) { - g_hash_table_remove (oldconns, old_path); - g_free (old_path); - } + g_hash_table_iter_init (&iter, priv->connections); + while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &connection)) { + if ( !g_hash_table_contains (alive_connections, connection) + && nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (connection))) { + if (!dead_connections) + dead_connections = g_ptr_array_new (); + g_ptr_array_add (dead_connections, connection); } - - g_free (full_path); } - g_dir_close (dir); + g_hash_table_destroy (alive_connections); - g_hash_table_iter_init (&iter, oldconns); - while (g_hash_table_iter_next (&iter, NULL, &data)) { - g_hash_table_iter_remove (&iter); - remove_connection (self, data); + if (dead_connections) { + for (i = 0; i < dead_connections->len; i++) + remove_connection (self, dead_connections->pdata[i]); + g_ptr_array_free (dead_connections, TRUE); } - g_hash_table_destroy (oldconns); } /* Plugin */ @@ -400,13 +527,7 @@ load_connection (NMSystemConfigInterface *config, if (nm_keyfile_plugin_utils_should_ignore_file (filename + dir_len + 1)) return FALSE; - connection = find_by_path (self, filename); - if (connection) - update_connection (self, connection, filename); - else { - new_connection (self, filename, NULL); - connection = find_by_path (self, filename); - } + connection = update_connection (self, NULL, filename, find_by_path (self, filename), TRUE, NULL, NULL); return (connection != NULL); } @@ -424,26 +545,13 @@ add_connection (NMSystemConfigInterface *config, GError **error) { SCPluginKeyfile *self = SC_PLUGIN_KEYFILE (config); - SCPluginKeyfilePrivate *priv = SC_PLUGIN_KEYFILE_GET_PRIVATE (self); - NMSettingsConnection *added = NULL; - char *path = NULL; + gs_free char *path = NULL; if (save_to_disk) { if (!nm_keyfile_plugin_write_connection (connection, NULL, &path, error)) return NULL; } - - added = (NMSettingsConnection *) nm_keyfile_connection_new (connection, path, error); - if (added) { - g_hash_table_insert (priv->connections, - g_strdup (nm_connection_get_uuid (NM_CONNECTION (added))), - added); - g_signal_connect (added, NM_SETTINGS_CONNECTION_REMOVED, - G_CALLBACK (connection_removed_cb), - self); - } - g_free (path); - return added; + return NM_SETTINGS_CONNECTION (update_connection (self, connection, path, NULL, FALSE, NULL, error)); } static gboolean @@ -501,7 +609,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) } else if (!strncmp (udis[i], "interface-name:", 15) && nm_utils_iface_valid_name (udis[i] + 15)) { specs = g_slist_append (specs, udis[i]); } else { - nm_log_warn (LOGD_SETTINGS, "Error in file '%s': invalid unmanaged-devices entry: '%s'", priv->conf_file, udis[i]); + nm_log_warn (LOGD_SETTINGS, "keyfile: error in file '%s': invalid unmanaged-devices entry: '%s'", priv->conf_file, udis[i]); g_free (udis[i]); } } @@ -511,7 +619,7 @@ get_unmanaged_specs (NMSystemConfigInterface *config) out: if (error) { - nm_log_warn (LOGD_SETTINGS, "%s", error->message); + nm_log_warn (LOGD_SETTINGS, "keyfile: error getting unmanaged specs: %s", error->message); g_error_free (error); } if (key_file) @@ -539,7 +647,7 @@ plugin_get_hostname (SCPluginKeyfile *plugin) out: if (error) { - nm_log_warn (LOGD_SETTINGS, "%s", error->message); + nm_log_warn (LOGD_SETTINGS, "keyfile: error getting hostname: %s", error->message); g_error_free (error); } if (key_file) @@ -586,7 +694,7 @@ plugin_set_hostname (SCPluginKeyfile *plugin, const char *hostname) out: if (error) { - nm_log_warn (LOGD_SETTINGS, "%s", error->message); + nm_log_warn (LOGD_SETTINGS, "keyfile: error setting hostname: %s", error->message); g_error_free (error); } g_free (data); diff --git a/src/settings/plugins/keyfile/utils.h b/src/settings/plugins/keyfile/utils.h index 1a7c2502b0..d153367103 100644 --- a/src/settings/plugins/keyfile/utils.h +++ b/src/settings/plugins/keyfile/utils.h @@ -23,6 +23,13 @@ #include <glib.h> #include "common.h" +#include "NetworkManagerUtils.h" + +#define NM_KEYFILE_CONNECTION_LOG_PATH(path) str_if_set (path,"in-memory") +#define NM_KEYFILE_CONNECTION_LOG_FMT "%s (%s,\"%s\")" +#define NM_KEYFILE_CONNECTION_LOG_ARG(con) NM_KEYFILE_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_connection_get_uuid ((NMConnection *) (con)), nm_connection_get_id ((NMConnection *) (con)) +#define NM_KEYFILE_CONNECTION_LOG_FMTD "%s (%s,\"%s\",%p)" +#define NM_KEYFILE_CONNECTION_LOG_ARGD(con) NM_KEYFILE_CONNECTION_LOG_PATH (nm_settings_connection_get_filename ((NMSettingsConnection *) (con))), nm_connection_get_uuid ((NMConnection *) (con)), nm_connection_get_id ((NMConnection *) (con)), (con) gboolean nm_keyfile_plugin_utils_should_ignore_file (const char *filename); diff --git a/src/settings/plugins/keyfile/writer.c b/src/settings/plugins/keyfile/writer.c index 5b82ad0903..7aa5bfb018 100644 --- a/src/settings/plugins/keyfile/writer.c +++ b/src/settings/plugins/keyfile/writer.c @@ -816,13 +816,10 @@ _internal_write_connection (NMConnection *connection, WriteInfo info; GError *local_err = NULL; - if (out_path) - g_return_val_if_fail (*out_path == NULL, FALSE); + g_return_val_if_fail (!out_path || !*out_path, FALSE); - if (!nm_connection_verify (connection, error)) { + if (!nm_connection_verify (connection, error)) g_return_val_if_reached (FALSE); - return FALSE; - } id = nm_connection_get_id (connection); g_assert (id && *id); |