Hide Forgot
1) Creating a new connection with connection.autoconnect=true may immediately activate the new connection. 2) updating a connection that is connection.autoconnect=true may immediately activate the modified connection 3) updating a connection that is currently active usually does not affect any properties on the active device. Usually, the changes only take effect when the user issues one of `nmcli connection up`, `nmcli device connection` or `nmcli device reapply`. However, that is not true for the properties "connection.zone" and "connection.metered", which take effect immediately: https://cgit.freedesktop.org/NetworkManager/NetworkManager/tree/src/devices/nm-device.c?id=a96c819f6f5d1c4faa21b242bea18096049c9674#n10456 4) deleting a connection that is currently active brings down the device (which then may autoconnect again with another connection). It should be possible to manange connections without *any* change to the network. The concrete use-case is an ansible role to configure connections. While configuring connections, no actual changes should to the network should be done, similar like when dropping ifcfg files. For that, NMSettingsConnection should have a autoconnect property exposed on D-Bus. Note that NMDevice already has such a property, but also the connection should have one. It should be a tri-state, with the default being unset (meaning: choose whatever the corresponding connection.autoconnect property has). Maybe, it should be instead a "autoconnect-blocked" property(?). Maybe, manually activating a such a connection should clear the autoconnect-blocked flag. Regarding 1) and 2), there should be a D-Bus API to allow determining the autoconnect flag of the NMSettingsConnection when adding/changing the connection. Regarding 3), it should be possibly to modify a connection without propagating any changes from the NMSettingsConnnection to the applied-connection. Regarding 4), it should be possible to mark a connection for garbage-collection. If the connection is currently not active, it should be just deleted right away. Otherwise, it should become a in-memory-connection only, that gets deleted when the device goes down (and all NMActiveConnections release the NMSettingsConnection).
For a different take on this, why not just use the checkpoint feature? Any time you want to make configuration changes that don't have an immediate effect, start a checkpoint and pass a new ONLY_DO_STUFF_WHEN_I_CALL_COMMIT flag. Make your changes; none of them would have an effect yet because of the flag. Then call the Commit() action which then allows all the changes to have an effect. Then if for whatever reason that breaks networking, the rollback timeout would still be in force until the checkpoint was made permanent or something like that.
(In reply to Dan Williams from comment #2) > For a different take on this, why not just use the checkpoint feature? That seems an orthogonal solution to me > Any time you want to make configuration changes that don't have an immediate > effect, start a checkpoint and pass a new ONLY_DO_STUFF_WHEN_I_CALL_COMMIT > flag. Make your changes; none of them would have an effect yet because of > the flag. Then call the Commit() action which then allows all the changes > to have an effect. Hm, that would extend the current checkpoint/rollback semantic. Currently, checkpoint just records the current state, and rollback restores that state. Between checkpoint/rollback, all actions are performed as usual and are globally visible (I like the simplicity of that). ONLY_DO_STUFF_WHEN_I_CALL_COMMIT sounds like, allowing the user to apply multiple changes that only take effect when commit is called -- very much like a database transation. That sounds super complicated to me. If multiple users start such transations, would they see each others changes? (Isolation-level?) > Then if for whatever reason that breaks networking, the rollback timeout > would still be in force until the checkpoint was made permanent or something > like that. I see one part of NetworkManager to have a global connection settings storage and API to manage these profiles. That is, all the settings exposed as /org/freedesktop/NetworkManager/Settings/*. This RFE is to add more API to allow managing this global-storage without *any* changes to the actual networking.
th/autoconnect-rh1401515 LGTM
th/user-block-autoconnect-rh1401515: > policy: merge reset_autoconnect_*() functions + if ( only_for_failed_secrets + && nm_settings_connection_autoconnect_blocked_reason_get (connection) == NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_NO_SECRETS) + continue; Shouldn't it be if ( only_for_failed_secrets && !nm_settings_connection_autoconnect_blocked_reason_get (NO_SECRETS)) ? Ok, it's fixed by a later patch. Also, the line is very long (137 columns). Can you split it, using temporary variables if needed? > policy: minor code cleanup in activate_slave_connections() This introduces very long lines (> 130), which require scrolling for me if I don't use the editor fully maximized. The rest looks good!
> all: add new D-Bus API org.freedesktop.NetworkManager.Settings.Connection.Update2() + if ( ((guint32) ((NMSettingsUpdate2Flags) flags)) != flags I don't understand this, what does it do? <!-- + Update2: ... + Update the connection with new settings and properties (replacing all + previous settings and properties) and save the connection to disk. Secrets ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ As I understand it, it saves only when the SAVE flag is passed or the connection is already persistent, right? Can you explain it? > settings: add NM_SETTINGS_UPDATE2_FLAG_BLOCK_AUTOCONNECT flag + * has autoconnect enabled and is modified, it becomes ellegible to autoconnect ellegible --> eligible @@ -1713,64 +1713,68 @@ update_auth_cb (NMSettingsConnection *self, ... + nm_settings_connection_autoconnect_blocked_reason_set (self, + NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, + NM_FLAGS_HAS (info->flags, NM_SETTINGS_UPDATE2_FLAG_BLOCK_AUTOCONNECT)); Isn't this a change in behavior? I mean, previously an Update() would not unblock a connection blocked by user request, but now it does.
(In reply to Beniamino Galvani from comment #6) > > all: add new D-Bus API org.freedesktop.NetworkManager.Settings.Connection.Update2() > > + if ( ((guint32) ((NMSettingsUpdate2Flags) flags)) != flags > > I don't understand this, what does it do? It's there to catch a situation where sizeof(flags) < sizeof(guint32). I solved it differently. > <!-- > + Update2: > ... > + Update the connection with new settings and properties (replacing > all > + previous settings and properties) and save the connection to disk. > Secrets > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > As I understand it, it saves only when the SAVE flag is passed or the > connection is already persistent, right? Can you explain it? better? > > settings: add NM_SETTINGS_UPDATE2_FLAG_BLOCK_AUTOCONNECT flag > > + * has autoconnect enabled and is modified, it becomes ellegible to > autoconnect > > ellegible --> eligible fixed > @@ -1713,64 +1713,68 @@ update_auth_cb (NMSettingsConnection *self, > ... > + nm_settings_connection_autoconnect_blocked_reason_set (self, > + > NM_SETTINGS_AUTO_CONNECT_BLOCKED_REASON_USER_REQUEST, > + NM_FLAGS_HAS > (info->flags, NM_SETTINGS_UPDATE2_FLAG_BLOCK_AUTOCONNECT)); > > Isn't this a change in behavior? I mean, previously an Update() would > not unblock a connection blocked by user request, but now it does. changed. Thanks. Repushed.
> libnm: add nm_remote_connection_update2() +/** + * nm_remote_connection_update2: + * @self: the #NMRemoteConnection Should be @connection. > settings: support setting a connection as volatile via Update2() + NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED = (1LL << 2), + NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_ONLY = (1LL << 3), I'm not sure about the usefulness of these two flags, but I'm fine with them if you think they are needed. The rest LGTM.
merged upstream as: master: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=632e693251715526817b6fb27872df66b23595a4 nm-1-10: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=ad7f1d18a0eafb1352d305035cd138f43a47b955 This extends SettingsConnection's Update() function with a new variant of update that allows to modify the settings with less interference with the current active connections: - Update2() can atomically (and temporarily) block autoconnect - Update2() can persist the connection to disk, or make it in-memory only (with 3 variants, what should happen with a potential persisted profile on disk). - Update2() can also mark the connection as volatile. That is, it will be automatically deleted once it's no longer used. What is still missing is to extend AddConnection() API to allow blocking autoconnect (temporarily) and extend AddAndActivated() to create volatile connections. I am still marking this request as fixed. Let's open follow-up RFEs, if need be.
automated, tested, working well via gi
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2018:0778