From: Zbigniew Jędrzejewski-Szmek Date: Wed, 7 Apr 2021 22:48:35 +0000 (+0200) Subject: tmpfiles: make handling of existing-but-different targets more consistent X-Git-Tag: v249-rc1~455^2~8 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=b88ba6c76116b6e03e202b1bdffd37933f748f03;p=thirdparty%2Fsystemd.git tmpfiles: make handling of existing-but-different targets more consistent create_fifo() was added in a2fc2f8dd30c17ad1e23a31fc6ff2aeba4c6fa27, and would always ignore failure. The test was trying to fail in this case, but we actually don't fail, which seems to be correct. We didn't notice before because the test was ineffective. To make things consistent, generally log at warning level, but don't propagate the error. For symlinks, log at debug level, as before. For 'e', failure is not propagated now. The test is adjusted to match. I think warning is appropriate in most cases: we do not expect a device node to be replaced by a different device node or even a non-device file. This would most likely be an error somewhere. An exception is made for symlinks, which are mismatched on purpose, for example /etc/resolv.conf. With this patch, we don't get any warnings with the any of the 74 tmpfiles.d files, which suggests that increasing the warning levels will not cause too many unexpected warnings. If it turns out that there are valid cases where people have expected mismatches for non-symlink types, we can always decrease the log levels again. --- diff --git a/man/systemd-tmpfiles.xml b/man/systemd-tmpfiles.xml index 5f97e37025f..15bc1ea889b 100644 --- a/man/systemd-tmpfiles.xml +++ b/man/systemd-tmpfiles.xml @@ -263,16 +263,18 @@ Exit status - On success, 0 is returned. If the configuration was syntactically invalid (syntax errors, - missing arguments, …), so some lines had to be ignored, but no other errors occurred, - 65 is returned (EX_DATAERR from - /usr/include/sysexits.h). If the configuration was syntactically valid, but - could not be executed (lack of permissions, creation of files in missing directories, invalid - contents when writing to /sys/ values, …), 73 is - returned (EX_CANTCREAT from /usr/include/sysexits.h). - Otherwise, 1 is returned (EXIT_FAILURE from - /usr/include/stdlib.h). - + On success, 0 is returned. If the configuration was syntactically invalid (syntax errors, missing + arguments, …), so some lines had to be ignored, but no other errors occurred, 65 is + returned (EX_DATAERR from /usr/include/sysexits.h). If the + configuration was syntactically valid, but could not be executed (lack of permissions, creation of files + in missing directories, invalid contents when writing to /sys/ values, …), + 73 is returned (EX_CANTCREAT from + /usr/include/sysexits.h). Otherwise, 1 is returned + (EXIT_FAILURE from /usr/include/stdlib.h). + + Note: when creating items, if the target already exists, but is of the wrong type or otherwise does + not match the requested state, and forced operation has not been requested with +, + a message is emitted, but the failure is otherwise ignored. diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index f11b4eed7cc..6b26dc8b9c3 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1656,10 +1656,9 @@ static int create_directory_or_subvolume(const char *path, mode_t mode, bool sub return log_error_errno(r, "%s does not exist and cannot be created as the file system is read-only.", path); if (k < 0) return log_error_errno(k, "Failed to check if %s exists: %m", path); - if (!k) { - log_warning("\"%s\" already exists and is not a directory.", path); - return -EEXIST; - } + if (!k) + return log_warning_errno(SYNTHETIC_ERRNO(EEXIST), + "\"%s\" already exists and is not a directory.", path); *creation = CREATION_EXISTING; } else @@ -1742,10 +1741,10 @@ static int empty_directory(Item *i, const char *path) { } if (r < 0) return log_error_errno(r, "is_dir() failed on path %s: %m", path); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EEXIST), - "'%s' already exists and is not a directory.", - path); + if (r == 0) { + log_warning("\"%s\" already exists and is not a directory.", path); + return 0; + } return path_set_perms(i, path); } @@ -1804,7 +1803,7 @@ static int create_device(Item *i, mode_t file_type) { return log_error_errno(r, "Failed to create device node \"%s\": %m", i->path); creation = CREATION_FORCE; } else { - log_debug("%s is not a device node.", i->path); + log_warning("\"%s\" already exists is not a device node.", i->path); return 0; } } else @@ -2575,7 +2574,9 @@ static int patch_var_run(const char *fname, unsigned line, char **path) { /* Also log about this briefly. We do so at LOG_NOTICE level, as we fixed up the situation automatically, hence * there's no immediate need for action by the user. However, in the interest of making things less confusing * to the user, let's still inform the user that these snippets should really be updated. */ - log_syntax(NULL, LOG_NOTICE, fname, line, 0, "Line references path below legacy directory /var/run/, updating %s → %s; please update the tmpfiles.d/ drop-in file accordingly.", *path, n); + log_syntax(NULL, LOG_NOTICE, fname, line, 0, + "Line references path below legacy directory /var/run/, updating %s → %s; please update the tmpfiles.d/ drop-in file accordingly.", + *path, n); free_and_replace(*path, n); diff --git a/test/units/testsuite-22.02.sh b/test/units/testsuite-22.02.sh index 54dcb405b08..c337cd6e05b 100755 --- a/test/units/testsuite-22.02.sh +++ b/test/units/testsuite-22.02.sh @@ -80,7 +80,7 @@ chmod 777 /tmp/e/3/d* touch /tmp/e/3/f1 chmod 644 /tmp/e/3/f1 -! systemd-tmpfiles --create - <