settings: return errno from nms_keyfile_nmmeta_write() for better logging

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.
This commit is contained in:
Thomas Haller
2019-08-01 10:52:20 +02:00
parent b216abb012
commit 4e36521d4c
4 changed files with 42 additions and 34 deletions

View File

@@ -1090,7 +1090,7 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self,
gboolean hard_failure = FALSE; gboolean hard_failure = FALSE;
NMSKeyfileStorage *storage; NMSKeyfileStorage *storage;
gs_unref_object NMSKeyfileStorage *storage_result = NULL; gs_unref_object NMSKeyfileStorage *storage_result = NULL;
gboolean nmmeta_success = FALSE; gboolean nmmeta_errno;
gs_free char *nmmeta_filename = NULL; gs_free char *nmmeta_filename = NULL;
NMSKeyfileStorageType storage_type; NMSKeyfileStorageType storage_type;
const char *loaded_path; const char *loaded_path;
@@ -1116,6 +1116,7 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self,
simulate ? "simulate " : "", simulate ? "simulate " : "",
loaded_path ? "write" : "delete", loaded_path ? "write" : "delete",
uuid); uuid);
nmmeta_errno = 0;
hard_failure = TRUE; hard_failure = TRUE;
goto out; goto out;
} }
@@ -1124,29 +1125,30 @@ nms_keyfile_plugin_set_nmmeta_tombstone (NMSKeyfilePlugin *self,
} }
if (simulate) { if (simulate) {
nmmeta_success = TRUE; nmmeta_errno = 0;
nmmeta_filename = nms_keyfile_nmmeta_filename (dirname, uuid, FALSE); nmmeta_filename = nms_keyfile_nmmeta_filename (dirname, uuid, FALSE);
} else { } else {
nmmeta_success = nms_keyfile_nmmeta_write (dirname, nmmeta_errno = nms_keyfile_nmmeta_write (dirname,
uuid, uuid,
loaded_path, loaded_path,
FALSE, FALSE,
shadowed_storage, shadowed_storage,
&nmmeta_filename); &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", loaded_path ? "writing" : "deleting",
nmmeta_filename, nmmeta_filename,
NM_PRINT_FMT_QUOTED (loaded_path, " (pointing to \"", loaded_path, "\")", ""), NM_PRINT_FMT_QUOTED (loaded_path, " (pointing to \"", loaded_path, "\")", ""),
NM_PRINT_FMT_QUOTED (shadowed_storage, " (shadows \"", shadowed_storage, "\")", ""), NM_PRINT_FMT_QUOTED (shadowed_storage, " (shadows \"", shadowed_storage, "\")", ""),
simulate simulate
? "simulated" ? "simulated"
: ( nmmeta_success : ( nmmeta_errno < 0
? "succeeded" ? "failed"
: "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; goto out;
storage = nm_sett_util_storages_lookup_by_filename (&priv->storages, nmmeta_filename); 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: out:
nm_assert (!nmmeta_success || !hard_failure); nm_assert (nmmeta_errno <= 0);
nm_assert (nmmeta_success || !storage_result); 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_hard_failure, hard_failure);
NM_SET_OUT (out_storage, (NMSettingsStorage *) g_steal_pointer (&storage_result)); NM_SET_OUT (out_storage, (NMSettingsStorage *) g_steal_pointer (&storage_result));
return nmmeta_success; return nmmeta_errno >= 0;
} }
/*****************************************************************************/ /*****************************************************************************/

View File

@@ -206,7 +206,7 @@ nms_keyfile_nmmeta_read_from_file (const char *full_filename,
return TRUE; return TRUE;
} }
gboolean int
nms_keyfile_nmmeta_write (const char *dirname, nms_keyfile_nmmeta_write (const char *dirname,
const char *uuid, const char *uuid,
const char *loaded_path, 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_tmp = NULL;
gs_free char *full_filename = NULL; gs_free char *full_filename = NULL;
int errsv;
nm_assert (dirname && dirname[0] == '/'); nm_assert (dirname && dirname[0] == '/');
nm_assert ( nm_utils_is_uuid (uuid) nm_assert ( nm_utils_is_uuid (uuid)
@@ -231,13 +232,15 @@ nms_keyfile_nmmeta_write (const char *dirname,
(void) unlink (full_filename_tmp); (void) unlink (full_filename_tmp);
if (!loaded_path) { if (!loaded_path) {
gboolean success = TRUE;
full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0'; full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0';
if (unlink (full_filename_tmp) != 0) errsv = 0;
success = NM_IN_SET (errno, ENOENT); 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)); NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp));
return success; return errsv;
} }
if (loaded_path_allow_relative) { if (loaded_path_allow_relative) {
@@ -270,30 +273,32 @@ nms_keyfile_nmmeta_write (const char *dirname,
contents, contents,
length, length,
0600, 0600,
NULL, &errsv,
NULL)) { NULL)) {
NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp));
return FALSE; return -NM_ERRNO_NATIVE (errsv);
} }
} else { } else {
/* we only have the "loaded_path" to store. That is commonly used for the tombstones to /* 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. * link to /dev/null. A symlink is sufficient to store that ammount of information.
* No need to bother with a keyfile. */ * No need to bother with a keyfile. */
if (symlink (loaded_path, full_filename_tmp) != 0) { if (symlink (loaded_path, full_filename_tmp) != 0) {
errsv = -NM_ERRNO_NATIVE (errno);
full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0'; full_filename_tmp[strlen (full_filename_tmp) - 1] = '\0';
NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp)); NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename_tmp));
return FALSE; return errsv;
} }
if (rename (full_filename_tmp, full_filename) != 0) { if (rename (full_filename_tmp, full_filename) != 0) {
errsv = -NM_ERRNO_NATIVE (errno);
(void) unlink (full_filename_tmp); (void) unlink (full_filename_tmp);
NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename)); 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)); NM_SET_OUT (out_full_filename, g_steal_pointer (&full_filename));
return TRUE; return 0;
} }
/*****************************************************************************/ /*****************************************************************************/

View File

@@ -66,12 +66,12 @@ gboolean nms_keyfile_nmmeta_read_from_file (const char *full_filename,
char **out_loaded_path, char **out_loaded_path,
char **out_shadowed_storage); char **out_shadowed_storage);
gboolean nms_keyfile_nmmeta_write (const char *dirname, int nms_keyfile_nmmeta_write (const char *dirname,
const char *uuid, const char *uuid,
const char *loaded_path, const char *loaded_path,
gboolean loaded_path_allow_relative, gboolean loaded_path_allow_relative,
const char *shadowed_storage, const char *shadowed_storage,
char **out_full_filename); char **out_full_filename);
/*****************************************************************************/ /*****************************************************************************/

View File

@@ -2543,7 +2543,7 @@ _assert_keyfile_nmmeta (const char *dirname,
nm_clear_g_free (&full_filename); 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); g_assert_cmpstr (full_filename, ==, exp_full_filename);
nm_clear_g_free (&full_filename); nm_clear_g_free (&full_filename);