From 1fa105eaef1f95dabf508d8828ace97dc5854060 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 24 Aug 2021 13:26:08 +0200 Subject: [PATCH] ifcfg-rh: fix updating ifcfg file if file on disk is no longer present Have an ifcfg file loaded in NetworkManager, then move/remove the file and try to modify it. That will fail with: "failed to update connection: Could not read file '/etc/sysconfig/network-scripts/ifcfg-eth0': No such file or directory" That is not right. If the user didn't move/remove the file but merely modified it, NetworkManager would silently overwrite it. There is no reason why move/remove should behave differently and not just write a completely fresh file. The reason why NetworkManager first loads the file before writing, is to preserve comments and unrecognized shell variables. This is a certain effort to play nice with users editing the file. It's not essential to load the file first and a failure to do so should not result in a failure. And of course, keyfile writer doesn't behave like this either. This bug exists since 2009, but let's not add a "Fixes" comment for commit 1974b257e008 ('ifcfg-rh: begin adding write support'), because it seems not right to backport this patch to all the old releases. --- .../plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index d2358ba6c0..824016dd64 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -3218,10 +3218,11 @@ do_write_construct(NMConnection * connection, GError ** error) { NMSettingConnection * s_con; - nm_auto_shvar_file_close shvarFile *ifcfg = NULL; - gs_free char * ifcfg_name = NULL; - gs_free char * route_path = NULL; - gs_free char * route6_path = NULL; + nm_auto_shvar_file_close shvarFile *ifcfg = NULL; + const char * ifcfg_name; + gs_free char * ifcfg_name_free = NULL; + gs_free char * route_path = NULL; + gs_free char * route6_path = NULL; const char * type; gs_unref_hashtable GHashTable *blobs = NULL; gs_unref_hashtable GHashTable *secrets = NULL; @@ -3245,11 +3246,7 @@ do_write_construct(NMConnection * connection, if (filename) { /* For existing connections, 'filename' should be full path to ifcfg file */ - ifcfg = svOpenFile(filename, error); - if (!ifcfg) - return FALSE; - - ifcfg_name = g_strdup(filename); + ifcfg_name = filename; } else if (ifcfg_dir) { gs_free char *escaped = NULL; int i_path; @@ -3270,21 +3267,22 @@ do_write_construct(NMConnection * connection, if (g_file_test(path_candidate, G_FILE_TEST_EXISTS)) continue; - ifcfg_name = g_steal_pointer(&path_candidate); + ifcfg_name_free = g_steal_pointer(&path_candidate); break; } - if (!ifcfg_name) { + if (!ifcfg_name_free) { g_set_error_literal(error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED, "Failed to find usable ifcfg file name"); return FALSE; } - - ifcfg = svCreateFile(ifcfg_name); + ifcfg_name = ifcfg_name_free; } else - ifcfg = svCreateFile("/tmp/ifcfg-dummy"); + ifcfg_name = "/tmp/ifcfg-dummy"; + + ifcfg = svCreateFile(ifcfg_name); route_path = utils_get_route_path(svFileGetName(ifcfg)); if (!route_path) {