-
Thomas Haller authored
Via Update2() D-Bus API there are three ways how a profile can be stored (or migrated) to in-memory: - NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY - NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED - NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_ONLY With the recent rework of settings I dropped NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY and it had the same meaning as NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED. However, the way NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED was implemented is problematic. The problem is that it leaves the profile on disk but creates an in-memory representation which shadows the persistent storage. Later, when storing the profile to disk again, a new filename is chosen. This allows via D-Bus API to toggle between NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED and NM_SETTINGS_UPDATE2_FLAG_TO_DISK, and thereby pilling up profiles on disk. Also, there is no D-Bus API to do anything sensible with these leaked, shadowed profiles on disk. Note that if we have a read-only profile in /usr/lib or in ifupdown plugin, then the problem is not made any worse. That is, because via D-Bus API such profiles can be made in-memory, and afterwards stored to /etc. Thereby too the profile gets duplicate on disk, but this game only works once. Afterwards, you cannot repeat it to create additional profiles on disk. It means, you can only leak profiles once, and only if they already exist in read-only storage to begin with. This problem with NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED already existed before the settings-delegate-storage rework, and is unrelated to whether in-memory profiles now happen to be persisted to /run. Note that NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_ONLY is simple and does not suffer from this problem. When you move a profile to in-memory-only, it gets deleted from persistent storage and no duplication happens. The problem is that NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED used to forget about the profile that it shadows, and that is wrong. So, first re-add proper support for NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY. This works by remembering the "shadowed-storage" path for in-memory profiles. When later saving such a profile to disk again, the shadowed-storage will be re-used. Likewise, when deleting such a profile, the shadowed storage will be deleted. Note that we keep NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED and it also remembers the shadowed storage (but without "owning" it). That means, when such a profile gets saved to disk again, the orginal storage is reused too. As such, during future updates it behaves just like NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY. The difference is when deleting such a profile. In this case, the profile is left on storage and a tombstone gets written. So, how is this better than before and why even keep this complicated flag? First, we keep this flag because we really want the ansible role to be able to do in-memory changes only. That implies being able to delete a profile from NetworkManager's view, but not from persistent storage. Without this flag there is no way to do that. You can only modify an on-disk profile by shadowing it, but you could not delete it form NetworkManager's view while keeping it on disk. The new form of NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY_DETACHED is safe and avoids the duplication problem because also for tombstones it remembers the original "shadowed-storage". That is, when the profile gets recreated later via D-Bus API AddConnection, then the re-created profile will still reference and reuse the shadowed storage that it had before deletion.
9eddf9fb