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 1974b257e0 ('ifcfg-rh: begin adding write support'), because
it seems not right to backport this patch to all the old releases.
This commit is contained in:
Thomas Haller 2021-08-24 13:26:08 +02:00
parent f9c096ba84
commit 1fa105eaef
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728

View file

@ -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) {