ifcfg-rh: clear all untouched, known keys before writing ifcfg-rh file

System Internals / NetworkManager - Thomas Haller [redhat.com] - 21 December 2019 11:44 UTC

When we write a connection profile to ifcfg-rh file, we first load the possibly existing file and modify it. The purpose is to preserve
variables that we don't know about, keep comments and preserve the order of the variables.

Note that the writer sets a bunch of variables according to the profile's setting. At various places the writer would explicitly clear variables with svUnsetValue(). However, that was problematic:

- we would not unset all variables that we care about. We really should not leave previous variables if they make no sense anymore for the profile. The only thing we want to preserve are entirely unknown keys and comments. Note that when the writer omits to clear an unset variable, it usually does so assuming that the reader would anyway ignore the key, become some other key renders it irrelevant. Given the complexity of the reader and writer, that is often not the case and hard to ensure.

We might have simply forgotten a svUnsetValue(), which was an easy to make mistake and hard to find (because you'd have to test with a pre-existing profile that happens to contain that key, which leaves countless combinations for testing.

That means, a profile written by the writter might be interpreted differently by the reader depending on which pre-existing keys were set.

- it was cumbersome to explicitly call svUnsetValue(). Note that for numbered tags in particular we would iterate the keys trying to unset them. For example for addresses (like "IPADDR5") we would iterate over the first 256 IPADDR keys, trying to unset them. That is horrible. For one, it doesn't cover the case where there might be more than 256 addresses. Also, it adds a significant overhead every time. While writing a ifcfg file currently is O(n^2) because setting one key is O(l), with l being the number of keys/lines. So, if you set n keys in a file with l lines, you get O(n*l). Which is basically O(n^2), because the number of lines and the number of keys to set usually corresponds. So when setting 256 times IPADDR, the overall complexity was still O(n^2 + 256 * n) and didn't change. However, the 256 factor here can be very significant.

We should not explicitly unset variables, we should always unset all known variables that we don't explicitly set.

The svUnsetValue() calls are still there. They will be dropped next.

07262b165 ifcfg-rh: clear all untouched, known keys before writing ifcfg-rh file
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 8 ++++----
.../ifcfg-System_test-bridge-component-b.cexpected | 1 -
.../ifcfg-rh/tests/network-scripts/ifcfg-netmask-1.cexpected | 1 -
3 files changed, 4 insertions(+), 6 deletions(-)

Upstream: cgit.freedesktop.org


  • Share