colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

settings: add "version-id" argument to Update2() D-Bus call

Browse Source 
We want to guard against concurrent modifications of profiles. We cannot
lock profiles, so what we instead do is expose (and bump) a version ID.
The user can check the version ID, plan ahead what to do, and tell
NetworkManager to only make the modification if no concurrent
modification was done. The conflict can be detected via the version ID.

The Update2() D-Bus call gets a parameter to only allow the request if
the version ID still matches.

nmcli should use this, but it is quite some effort to retry upon
concurrent modification. This is still to do.

Note that the user might make a decision that is based on multiple
profiles. As the new version-id is only per-profile, we cannot guard
against such inter-profile modifications. What would be needed, is a
UpdateMany() call, where we could modify multiple profiles at once, and
the action only takes effect if all version IDs show no concurrent
modification. That's not done yet, and maybe never will be.
This commit is contained in:
Thomas Haller
2023-06-22 11:40:42 +02:00
parent cf5c576d55
commit e883ae765f
3 changed files with 31 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
introspection/org.freedesktop.NetworkManager.Settings.Connection.xml
View File

@@ -164,6 +164,10 @@
<listitem><para>The settings plugin the connection will be migrated to
such as "keyfile" or "ifcfg-rh".</para>
<para role="since">Since 1.38</para></listitem>
<term><literal>version-id</literal>:</term>
<listitem><para>If specified, the update request is rejected if the
profile's version-id does not match. This can be used to catch concurrent
modifications. Zero means no version check.</para><para role="since">Since 1.44</para></listitem>
</varlistentry>
</variablelist>

24
src/core/settings/nm-settings-connection.c
View File

@@ -1443,6 +1443,7 @@ typedef struct {
NMSettingsUpdate2Flags flags;
char *audit_args;
char *plugin_name;
guint64 version_id;
bool is_update2 : 1;
} UpdateInfo;
@@ -1492,6 +1493,16 @@ update_auth_cb(NMSettingsConnection *self,
priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self);
if (info->version_id != 0 && info->version_id != priv->version_id) {
g_set_error_literal(&local,
NM_SETTINGS_ERROR,
NM_SETTINGS_ERROR_VERSION_ID_MISMATCH,
"Update failed because profile changed in the meantime and the "
"version-id mismatches");
error = local;
goto out;
}
if (info->new_settings) {
if (!_nm_connection_aggregate(info->new_settings,
NM_CONNECTION_AGGREGATE_ANY_SECRETS,
@@ -1639,6 +1650,7 @@ settings_connection_update(NMSettingsConnection *self,
GDBusMethodInvocation *context,
GVariant *new_settings,
const char *plugin_name,
guint64 version_id,
NMSettingsUpdate2Flags flags)
{
NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self);
@@ -1695,6 +1707,7 @@ settings_connection_update(NMSettingsConnection *self,
.flags = flags,
.new_settings = tmp,
.plugin_name = g_strdup(plugin_name),
.version_id = version_id,
};
permission = get_update_modify_permission(nm_settings_connection_get_connection(self),
@@ -1729,6 +1742,7 @@ impl_settings_connection_update(NMDBusObject *obj,
invocation,
settings,
NULL,
0,
NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
}
@@ -1750,6 +1764,7 @@ impl_settings_connection_update_unsaved(NMDBusObject *obj,
invocation,
settings,
NULL,
0,
NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY);
}
@@ -1769,6 +1784,7 @@ impl_settings_connection_save(NMDBusObject *obj,
invocation,
NULL,
NULL,
0,
NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
}
@@ -1785,6 +1801,7 @@ impl_settings_connection_update2(NMDBusObject *obj,
gs_unref_variant GVariant *settings = NULL;
gs_unref_variant GVariant *args = NULL;
gs_free char *plugin_name = NULL;
guint64 version_id = 0;
guint32 flags_u;
GError *error = NULL;
GVariantIter iter;
@@ -1831,6 +1848,11 @@ impl_settings_connection_update2(NMDBusObject *obj,
plugin_name = g_variant_dup_string(args_value, NULL);
continue;
}
if (nm_streq(args_name, "version-id")
&& g_variant_is_of_type(args_value, G_VARIANT_TYPE_UINT64)) {
version_id = g_variant_get_uint64(args_value);
continue;
}
error = g_error_new(NM_SETTINGS_ERROR,
NM_SETTINGS_ERROR_INVALID_ARGUMENTS,
@@ -1840,7 +1862,7 @@ impl_settings_connection_update2(NMDBusObject *obj,
return;
}
settings_connection_update(self, TRUE, invocation, settings, plugin_name, flags);
settings_connection_update(self, TRUE, invocation, settings, plugin_name, version_id, flags);
}
static void

4
src/libnm-core-public/nm-errors.h
View File

@@ -250,6 +250,9 @@ GQuark nm_secret_agent_error_quark(void);
* @NM_SETTINGS_ERROR_UUID_EXISTS: a connection with that UUID already exists
* @NM_SETTINGS_ERROR_INVALID_HOSTNAME: attempted to set an invalid hostname
* @NM_SETTINGS_ERROR_INVALID_ARGUMENTS: invalid arguments
* @NM_SETTINGS_ERROR_VERSION_ID_MISMATCH: The profile's VersionId mismatched
* and the update is rejected. See the "version-id" argument to Update2()
* method. Since 1.44.
*
* Errors related to the settings/persistent configuration interface of
* NetworkManager.
@@ -267,6 +270,7 @@ typedef enum {
NM_SETTINGS_ERROR_UUID_EXISTS, /*< nick=UuidExists >*/
NM_SETTINGS_ERROR_INVALID_HOSTNAME, /*< nick=InvalidHostname >*/
NM_SETTINGS_ERROR_INVALID_ARGUMENTS, /*< nick=InvalidArguments >*/
NM_SETTINGS_ERROR_VERSION_ID_MISMATCH, /*< nick=VersionIdMismatch >*/
} NMSettingsError;
GQuark nm_settings_error_quark(void);