After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 744711 - [RFE] in-memory connections should be handled by a new setting plugin "mem", not duplicating functionality in keyfile, ifcfg-rh
[RFE] in-memory connections should be handled by a new setting plugin "mem", ...
Status: RESOLVED FIXED
Product: NetworkManager
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: NetworkManager maintainer(s)
NetworkManager maintainer(s)
Depends on:
Blocks: nm-next 772414
 
 
Reported: 2015-02-18 14:57 UTC by Thomas Haller
Modified: 2019-07-17 11:22 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Thomas Haller 2015-02-18 14:57:21 UTC
Currently both ifcfg-rh and keyfile plugin support in-memory-only connections.

this duplicates functionality and makes the code more complicated.

It would be better to have a new setting plug "mem" that is (like keyfile) always present. And NMSettings would have special knowledge about mem-connections:
  - add_connection(save_to_disk=false), would directly store the connection 
    inside "mem" -- which always succeeds.
  - persisting an mem-connection would remove it from mem plugin, 
    and call add_connection() on one of the other setting plugins.


this also fixes the following unexpected behavior:
 - add in-memory connection that is writable by ifcfg-rh
 - modify it, so that it is no longer supported by ifcfg-rh
 - persistate
Comment 1 Dan Williams 2015-04-07 20:08:03 UTC
So the problem to solve here, and the reason it wasn't done this way originally, is that every NMConnection is an instance of a specific plugin's NMSettingConnection subclass.

So if the user wants to persist a 'mem' connection, that NMMemConnection would have to somehow be copied to an NMKeyfileConnection so that it could be saved.  But then, any device using that connection would still have a reference to the old NMMemConnection and not the new NMKeyfileConnection.  Also when the 'mem' connection was removed from the plugin (since we cannot have two connections with the same UUID) it will trigger a disconnect of the Device because the active connection has been removed.

I think it's simpler to just live with some of the code duplication, than try to work around the remove-connection-deactivates and to try to update all the existing references to the 'mem' connection.

But maybe you can come up with some ideas to work around all this...
Comment 2 Thomas Haller 2015-04-08 07:42:17 UTC
(In reply to Dan Williams from comment #1)

> But maybe you can come up with some ideas to work around all this...

I think the solution is not to use inheritance, but composition.

We shouldn't have NMKeyfileConnection, NMIfnetConnection, etc.
Instead there should only be NMSettingsConnection, that has a reference to the backend -- "NMSettingsBackend".
Comment 3 Thomas Haller 2019-07-17 11:22:43 UTC
this got now fixed by commit [1]. See also the merge request [2].

As this is a large rework, expect some fall out and there will be follow-up fixes necessary...




[1] https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=d35d3c468a304c3e0e78b4b068d105b1d753876c

[2] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/189