diff options
author | Thomas Haller <thaller@redhat.com> | 2019-08-01 10:52:20 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2019-08-08 12:03:15 +0200 |
commit | 4e36521d4c82b2967628def8125a83ecd13bfd8c (patch) | |
tree | f7817027ebcbaab8f24283bbe98bae7799c9bd55 | |
parent | b216abb012b97a7e0e50b969e4a04c8b11a4cbe9 (diff) | |
download | NetworkManager-4e36521d4c82b2967628def8125a83ecd13bfd8c.tar.gz |
settings: return errno from nms_keyfile_nmmeta_write() for better loggingth/settings-improvements
I encountered a failure in the log
<trace> [1564647990.7822] keyfile: commit: deleting nmmeta file "/etc/NetworkManager/system-connections/35370b0b-e53b-42ea-9fe3-f1b1d552343b.nmmeta" failed
<trace> [1564647990.7822] keyfile: commit: deleting nmmeta file "/etc/NetworkManager/system-connections/35370b0b-e53b-42ea-9fe3-f1b1d552343b.nmmeta" simulated
I think that was due to SELinux (rh #1738010).
Let nms_keyfile_nmmeta_write() return an errno code so we can log
more information about the failure.
4 files changed, 42 insertions, 34 deletions
diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c index fbe70ef4d2..0a1902b65f 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c @@ -1090,7 +1090,7 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, gboolean hard_failure = FALSE; NMSKeyfileStorage *storage; gs_unref_object NMSKeyfileStorage *storage_result = NULL; - gboolean nmmeta_success = FALSE; + gboolean nmmeta_errno; gs_free char *nmmeta_filename = NULL; NMSKeyfileStorageType storage_type; const char *loaded_path; @@ -1116,6 +1116,7 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, simulate ? "simulate " : "", loaded_path ? "write" : "delete", uuid); + nmmeta_errno = 0; hard_failure = TRUE; goto out; } @@ -1124,29 +1125,30 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, } if (simulate) { - nmmeta_success = TRUE; + nmmeta_errno = 0; nmmeta_filename = nms_keyfile_nmmeta_filename (dirname, uuid, FALSE); } else { - nmmeta_success = nms_keyfile_nmmeta_write (dirname, - uuid, - loaded_path, - FALSE, - shadowed_storage, - &nmmeta_filename); + nmmeta_errno = nms_keyfile_nmmeta_write (dirname, + uuid, + loaded_path, + FALSE, + shadowed_storage, + &nmmeta_filename); } - _LOGT ("commit: %s nmmeta file \"%s\"%s%s%s%s%s%s %s", + _LOGT ("commit: %s nmmeta file \"%s\"%s%s%s%s%s%s %s%s%s%s", loaded_path ? "writing" : "deleting", nmmeta_filename, NM_PRINT_FMT_QUOTED (loaded_path, " (pointing to \"", loaded_path, "\")", ""), NM_PRINT_FMT_QUOTED (shadowed_storage, " (shadows \"", shadowed_storage, "\")", ""), simulate ? "simulated" - : ( nmmeta_success - ? "succeeded" - : "failed")); + : ( nmmeta_errno < 0 + ? "failed" + : "succeeded"), + NM_PRINT_FMT_QUOTED (nmmeta_errno < 0, " (", nm_strerror_native (nm_errno_native (nmmeta_errno)), ")", "")); - if (!nmmeta_success) + if (nmmeta_errno < 0) goto out; storage = nm_sett_util_storages_lookup_by_filename (&priv->storages, nmmeta_filename); @@ -1177,12 +1179,13 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self, } out: - nm_assert (!nmmeta_success || !hard_failure); - nm_assert (nmmeta_success || !storage_result); + nm_assert (nmmeta_errno <= 0); + nm_assert (nmmeta_errno < 0 || !hard_failure); + nm_assert (nmmeta_errno == 0 || !storage_result); NM_SET_OUT (out_hard_failure, hard_failure); NM_SET_OUT (out_storage, (NMSettingsStorage *) g_steal_pointer (&storage_result)); - return nmmeta_success; + return nmmeta_errno >= 0; } /*****************************************************************************/ diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c index ed07bea679..ce0984bc75 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.c +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c @@ -206,7 +206,7 @@ nms_keyfile_nmmeta_read_from_file (const char *full_filename, return TRUE; } -gboolean +int nms_keyfile_nmmeta_write (const char *dirname, const char *uuid, const char *loaded_path, @@ -216,6 +216,7 @@ nms_keyfile_nmmeta_write (const char *dirname, { gs_free char *full_filename_tmp = NULL; gs_free char *full_filename = NULL; + int errsv; nm_assert (dirname && dirname[0] == '/'); nm_assert ( nm_utils_is_uuid (uuid) @@ -231,13 +232,15 @@ nms_keyfile_nmmeta_write (const char *dirname, (void) unlink (full_filename_tmp); if (!loaded_path) { - gboolean success = TRUE; - full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0'; - if (unlink (full_filename_tmp) != 0) - success = NM_IN_SET (errno, ENOENT); + errsv = 0; + if (unlink (full_filename_tmp) != 0) { + errsv = -NM_ERRNO_NATIVE (errno); + if (errsv == -ENOENT) + errsv = 0; + } NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); - return success; + return errsv; } if (loaded_path_allow_relative) { @@ -270,30 +273,32 @@ nms_keyfile_nmmeta_write (const char *dirname, contents, length, 0600, - NULL, + &errsv, NULL)) { NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); - return FALSE; + return -NM_ERRNO_NATIVE (errsv); } } else { /* we only have the "loaded_path" to store. That is commonly used for the tombstones to * link to /dev/null. A symlink is sufficient to store that ammount of information. * No need to bother with a keyfile. */ if (symlink (loaded_path, full_filename_tmp) != 0) { + errsv = -NM_ERRNO_NATIVE (errno); full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0'; NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); - return FALSE; + return errsv; } if (rename (full_filename_tmp, full_filename) != 0) { + errsv = -NM_ERRNO_NATIVE (errno); (void) unlink (full_filename_tmp); NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename)); - return FALSE; + return errsv; } } NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename)); - return TRUE; + return 0; } /*****************************************************************************/ diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.h b/src/settings/plugins/keyfile/nms-keyfile-utils.h index 723c443625..768f95d95d 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.h +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.h @@ -66,12 +66,12 @@ gboolean nms_keyfile_nmmeta_read_from_file (const char *full_filename, char **out_loaded_path, char **out_shadowed_storage); -gboolean nms_keyfile_nmmeta_write (const char *dirname, - const char *uuid, - const char *loaded_path, - gboolean loaded_path_allow_relative, - const char *shadowed_storage, - char **out_full_filename); +int nms_keyfile_nmmeta_write (const char *dirname, + const char *uuid, + const char *loaded_path, + gboolean loaded_path_allow_relative, + const char *shadowed_storage, + char **out_full_filename); /*****************************************************************************/ diff --git a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c index f96111a2e4..32f4fc4f8d 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile-settings.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile-settings.c @@ -2543,7 +2543,7 @@ _assert_keyfile_nmmeta (const char *dirname, nm_clear_g_free (&full_filename); - g_assert (nms_keyfile_nmmeta_write (dirname, uuid, loaded_path, allow_relative, NULL, &full_filename)); + g_assert_cmpint (nms_keyfile_nmmeta_write (dirname, uuid, loaded_path, allow_relative, NULL, &full_filename), ==, 0); g_assert_cmpstr (full_filename, ==, exp_full_filename); nm_clear_g_free (&full_filename); |