fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-06-19 08:58:28 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

settings: extend commit_changes() to update the settings after writing

Browse source 
During write, it can regularly happen that the connection gets modified.
For example, keyfile never writes blobs as-is, it always writes the
blob to an external file, and replaces the certificate property with
a path.
Other reasons could be just bugs, where the reader and writer are not doing
a proper round trip (these cases should be fixed).

Refactor commit_changes(), to return the re-read connection to
the settings-connection class, and handle replacing the settings
there.

Also, prepare for another change. Sometimes we first call replace_settings()
followed by commit_changes(). It would be better to instead call commit_changes()
first, and only on success proceed with replace_settings(). Hence, commit_changes()
gets a new argument new_connection, that can be used to write another
connection to disk.
This commit is contained in:
Thomas Haller 2017-10-19 10:37:29 +02:00
parent edc7503569
commit 5a82cad5f3
8 changed files with 129 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/nm-checkpoint.c
View file

@ -259,6 +259,7 @@ activate:
nm_connection_replace_settings_from_connection (NM_CONNECTION (connection),
dev_checkpoint->settings_connection);
nm_settings_connection_commit_changes (connection,
NULL,
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
NULL);
}

1
src/nm-manager.c
View file

@ -4035,6 +4035,7 @@ activation_add_done (NMSettings *settings,
if (_internal_activate_generic (self, active, &local)) {
nm_settings_connection_commit_changes (new_connection,
NULL,
NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION | NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED,
NULL);
g_dbus_method_invocation_return_value (

82
src/settings/nm-settings-connection.c
View file

@ -634,16 +634,21 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self,
gboolean
nm_settings_connection_commit_changes (NMSettingsConnection *self,
NMConnection *new_connection,
NMSettingsConnectionCommitReason commit_reason,
GError **error)
{
NMSettingsConnectionClass *klass;
gs_free_error GError *local = NULL;
gs_unref_object NMConnection *reread_connection = NULL;
gs_free char *logmsg_change = NULL;
g_return_val_if_fail (NM_IS_SETTINGS_CONNECTION (self), FALSE);
klass = NM_SETTINGS_CONNECTION_GET_CLASS (self);
if (!klass->commit_changes) {
_LOGW ("write: setting plugin %s does not support to write connection",
G_OBJECT_TYPE_NAME (self));
g_set_error (error,
NM_SETTINGS_ERROR,
NM_SETTINGS_ERROR_FAILED,
@ -651,14 +656,55 @@ nm_settings_connection_commit_changes (NMSettingsConnection *self,
return FALSE;
}
if (!klass->commit_changes (self,
commit_reason,
error)) {
if ( new_connection
&& !nm_settings_connection_replace_settings_prepare (self,
new_connection,
&local)) {
_LOGW ("write: failed to prepare connection for writing: %s",
local->message);
g_propagate_error (error, g_steal_pointer (&local));
return FALSE;
}
if (!klass->commit_changes (self,
new_connection,
commit_reason,
&reread_connection,
&logmsg_change,
&local)) {
_LOGW ("write: failure to write setting: %s",
local->message);
g_propagate_error (error, g_steal_pointer (&local));
return FALSE;
}
if (reread_connection || new_connection) {
if (!nm_settings_connection_replace_settings_full (self,
reread_connection ?: new_connection,
!reread_connection,
FALSE,
new_connection
? "update-during-write"
: "replace-and-commit-disk",
&local)) {
/* this can't really happen, because at this point replace-settings
* is no longer supposed to fail. It's a bug. */
_LOGE ("write: replacing setting failed: %s",
local->message);
g_propagate_error (error, g_steal_pointer (&local));
g_return_val_if_reached (FALSE);
}
}
set_unsaved (self, FALSE);
if (reread_connection)
_LOGI ("write: successfully updated (%s), connection was modified in the process", logmsg_change);
else if (new_connection)
_LOGI ("write: successfully updated (%s)", logmsg_change);
else
_LOGI ("write: successfully commited (%s)", logmsg_change);
return TRUE;
}
@ -945,8 +991,6 @@ nm_settings_connection_new_secrets (NMSettingsConnection *self,
GVariant *secrets,
GError **error)
{
gs_free_error GError *local = NULL;
if (!nm_settings_connection_has_unmodified_applied_connection (self, applied_connection,
NM_SETTING_COMPARE_FLAG_NONE)) {
g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
@ -960,11 +1004,10 @@ nm_settings_connection_new_secrets (NMSettingsConnection *self,
update_system_secrets_cache (self);
update_agent_secrets_cache (self, NULL);
if (!nm_settings_connection_commit_changes (self,
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
&local))
_LOGW ("Error saving new secrets to backing storage: %s", local->message);
nm_settings_connection_commit_changes (self,
NULL,
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
NULL);
return TRUE;
}
@ -1074,16 +1117,14 @@ get_secrets_done_cb (NMAgentManager *manager,
* nothing has changed, since agent-owned secrets don't get saved here.
*/
if (agent_had_system) {
gs_free_error GError *local2 = NULL;
_LOGD ("(%s:%p) saving new secrets to backing storage",
setting_name,
info);
if (!nm_settings_connection_commit_changes (self,
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
&local2))
_LOGW ("Error saving new secrets to backing storage: %s", local2->message);
nm_settings_connection_commit_changes (self,
NULL,
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
NULL);
} else {
_LOGD ("(%s:%p) new agent secrets processed",
setting_name,
@ -1640,6 +1681,7 @@ update_auth_cb (NMSettingsConnection *self,
/* We're just calling Save(). Just commit the existing connection. */
if (info->save_to_disk) {
nm_settings_connection_commit_changes (self,
NULL,
NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION,
&local);
}
@ -1691,7 +1733,10 @@ update_auth_cb (NMSettingsConnection *self,
nm_connection_get_id (info->new_settings)))
commit_reason |= NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED;
nm_settings_connection_commit_changes (self, commit_reason, &local);
nm_settings_connection_commit_changes (self,
NULL,
commit_reason,
&local);
out:
if (!local) {
@ -2010,6 +2055,7 @@ dbus_clear_secrets_auth_cb (NMSettingsConnection *self,
NM_CONNECTION (self));
nm_settings_connection_commit_changes (self,
NULL,
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
&local);

10
src/settings/nm-settings-connection.h
View file

@ -108,7 +108,10 @@ struct _NMSettingsConnectionClass {
GError **error);
gboolean (*commit_changes) (NMSettingsConnection *self,
NMConnection *new_connection,
NMSettingsConnectionCommitReason commit_reason,
NMConnection **out_reread_connection,
char **out_logmsg_change,
GError **error);
gboolean (*delete) (NMSettingsConnection *self,
@ -124,9 +127,10 @@ gboolean nm_settings_connection_has_unmodified_applied_connection (NMSettingsCon
NMConnection *applied_connection,
NMSettingCompareFlags compare_flage);
gboolean nm_settings_connection_commit_changes (NMSettingsConnection *self,
NMSettingsConnectionCommitReason commit_reason,
GError **error);
gboolean nm_settings_connection_commit_changes (NMSettingsConnection *self,
NMConnection *new_connection,
NMSettingsConnectionCommitReason commit_reason,
GError **error);
gboolean nm_settings_connection_replace_settings_prepare (NMSettingsConnection *self,
NMConnection *new_connection,

29
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c
View file

@ -325,16 +325,22 @@ can_commit (NMSettingsConnection *connection,
static gboolean
commit_changes (NMSettingsConnection *connection,
NMConnection *new_connection,
NMSettingsConnectionCommitReason commit_reason,
NMConnection **out_reread_connection,
char **out_logmsg_change,
GError **error)
{
const char *filename;
nm_assert (out_reread_connection && !*out_reread_connection);
nm_assert (!out_logmsg_change || !*out_logmsg_change);
filename = nm_settings_connection_get_filename (connection);
if (!filename) {
gs_free char *ifcfg_path = NULL;
if (!writer_new_connection (NM_CONNECTION (connection),
if (!writer_new_connection (new_connection ?: NM_CONNECTION (connection),
IFCFG_DIR,
&ifcfg_path,
NULL,
@ -342,15 +348,24 @@ commit_changes (NMSettingsConnection *connection,
error))
return FALSE;
nm_settings_connection_set_filename (connection, ifcfg_path);
NM_SET_OUT (out_logmsg_change,
g_strdup_printf ("ifcfg-rh: persist %s",
ifcfg_path));
return TRUE;
}
return writer_update_connection (NM_CONNECTION (connection),
IFCFG_DIR,
filename,
NULL,
NULL,
error);
if (!writer_update_connection (new_connection ?: NM_CONNECTION (connection),
IFCFG_DIR,
filename,
NULL,
NULL,
error))
return FALSE;
NM_SET_OUT (out_logmsg_change,
g_strdup_printf ("ifcfg-rh: update %s",
filename));
return TRUE;
}
static gboolean

14
src/settings/plugins/ifnet/nms-ifnet-connection.c
View file

@ -76,12 +76,19 @@ nm_ifnet_connection_get_conn_name (NMIfnetConnection *connection)
static gboolean
commit_changes (NMSettingsConnection *connection,
NMConnection *new_connection,
NMSettingsConnectionCommitReason commit_reason,
NMConnection **out_reread_connection,
char **out_logmsg_change,
GError **error)
{
NMIfnetConnectionPrivate *priv = NM_IFNET_CONNECTION_GET_PRIVATE ((NMIfnetConnection *) connection);
char *new_name = NULL;
gboolean success = FALSE;
gboolean added = FALSE;
nm_assert (out_reread_connection && !*out_reread_connection);
nm_assert (!out_logmsg_change || !*out_logmsg_change);
g_signal_emit (connection, signals[IFNET_CANCEL_MONITORS], 0);
@ -94,6 +101,7 @@ commit_changes (NMSettingsConnection *connection,
NULL,
error);
} else {
added = TRUE;
success = ifnet_add_new_connection (NM_CONNECTION (connection),
CONF_NET_FILE,
WPA_SUPPLICANT_CONF,
@ -112,6 +120,12 @@ commit_changes (NMSettingsConnection *connection,
g_signal_emit (connection, signals[IFNET_SETUP_MONITORS], 0);
if (success) {
NM_SET_OUT (out_logmsg_change,
g_strdup_printf ("ifcfg-rh: %s %s",
added ? "persist" : "updated",
new_name));
}
return success;
}

1
src/settings/plugins/ifupdown/nms-ifupdown-plugin.c
View file

@ -140,6 +140,7 @@ bind_device_to_connection (SettingsPluginIfupdown *self,
}
nm_settings_connection_commit_changes (NM_SETTINGS_CONNECTION (exported),
NULL,
NM_SETTINGS_CONNECTION_COMMIT_REASON_NONE,
NULL);
}

38
src/settings/plugins/keyfile/nms-keyfile-connection.c
View file

@ -52,14 +52,20 @@ G_DEFINE_TYPE (NMSKeyfileConnection, nms_keyfile_connection, NM_TYPE_SETTINGS_CO
static gboolean
commit_changes (NMSettingsConnection *connection,
NMConnection *new_connection,
NMSettingsConnectionCommitReason commit_reason,
NMConnection **out_reread_connection,
char **out_logmsg_change,
GError **error)
{
gs_free char *path = NULL;
gs_unref_object NMConnection *reread = NULL;
gboolean reread_same = FALSE;
if (!nms_keyfile_writer_connection (NM_CONNECTION (connection),
nm_assert (out_reread_connection && !*out_reread_connection);
nm_assert (!out_logmsg_change || !*out_logmsg_change);
if (!nms_keyfile_writer_connection (new_connection ?: NM_CONNECTION (connection),
nm_settings_connection_get_filename (connection),
NM_FLAGS_ALL (commit_reason, NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION
| NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED),
@ -76,29 +82,23 @@ commit_changes (NMSettingsConnection *connection,
nm_settings_connection_set_filename (connection, path);
if (old_path) {
nm_log_info (LOGD_SETTINGS, "keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT" and rename from \"%s\"",
NMS_KEYFILE_CONNECTION_LOG_ARG (connection),
old_path);
NM_SET_OUT (out_logmsg_change,
g_strdup_printf ("keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT" and rename from \"%s\"",
NMS_KEYFILE_CONNECTION_LOG_ARG (connection),
old_path));
} else {
nm_log_info (LOGD_SETTINGS, "keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT" and persist connection",
NMS_KEYFILE_CONNECTION_LOG_ARG (connection));
NM_SET_OUT (out_logmsg_change,
g_strdup_printf ("keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT" and persist connection",
NMS_KEYFILE_CONNECTION_LOG_ARG (connection)));
}
} else {
nm_log_info (LOGD_SETTINGS, "keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT,
NMS_KEYFILE_CONNECTION_LOG_ARG (connection));
NM_SET_OUT (out_logmsg_change,
g_strdup_printf ("keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT,
NMS_KEYFILE_CONNECTION_LOG_ARG (connection)));
}
if (reread && !reread_same) {
gs_free_error GError *local = NULL;
if (!nm_settings_connection_replace_settings (connection, reread, FALSE, "update-during-write", &local)) {
nm_log_warn (LOGD_SETTINGS, "keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT" after persisting connection failed: %s",
NMS_KEYFILE_CONNECTION_LOG_ARG (connection), local->message);
} else {
nm_log_info (LOGD_SETTINGS, "keyfile: update "NMS_KEYFILE_CONNECTION_LOG_FMT" after persisting connection",
NMS_KEYFILE_CONNECTION_LOG_ARG (connection));
}
}
if (reread && !reread_same)
*out_reread_connection = g_steal_pointer (&reread);
return TRUE;
}