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 5f97e37025f3b530de4c56efd7d3e91a96c5dcbb..15bc1ea889b830d5c029513611e82dd79899f5fa 100644 (file)
--- a/man/systemd-tmpfiles.xml
+++ b/man/systemd-tmpfiles.xml
@@ -263,16 +263,18 @@
   <refsect1>
     <title>Exit status</title>
 
-    <para>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,
-    <constant>65</constant> is returned (<constant>EX_DATAERR</constant> from
-    <filename>/usr/include/sysexits.h</filename>). 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 <filename>/sys/</filename> values, …), <constant>73</constant> is
-    returned (<constant>EX_CANTCREAT</constant> from <filename>/usr/include/sysexits.h</filename>).
-    Otherwise, <constant>1</constant> is returned (<constant>EXIT_FAILURE</constant> from
-    <filename>/usr/include/stdlib.h</filename>).
-    </para>
+    <para>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, <constant>65</constant> is
+    returned (<constant>EX_DATAERR</constant> from <filename>/usr/include/sysexits.h</filename>). 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 <filename>/sys/</filename> values, …),
+    <constant>73</constant> is returned (<constant>EX_CANTCREAT</constant> from
+    <filename>/usr/include/sysexits.h</filename>). Otherwise, <constant>1</constant> is returned
+    (<constant>EXIT_FAILURE</constant> from <filename>/usr/include/stdlib.h</filename>).</para>
+
+    <para>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 <literal>+</literal>,
+    a message is emitted, but the failure is otherwise ignored.</para>
   </refsect1>
 
   <refsect1>

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index f11b4eed7cc91135d97bc9427dccdc21790b94db..6b26dc8b9c333a65a195959b60f04235c056f8a6 100644 (file)
--- 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 54dcb405b0800b0e8d854a0f1ca5f1daff958e83..c337cd6e05b829125d595e0cfa1b137091380a86 100755 (executable)
--- 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 - <<EOF
+systemd-tmpfiles --create - <<EOF
 e     /tmp/e/3/*   0755 daemon daemon - -
 EOF
 
@@ -115,7 +115,7 @@ test $(stat -c %U:%G:%a /tmp/C/1/f1) = "daemon:daemon:755"
 test -d /tmp/C/2
 test $(stat -c %U:%G:%a /tmp/C/2/f1) = "daemon:daemon:755"
 
-! systemd-tmpfiles --create - <<EOF
+systemd-tmpfiles --create - <<EOF
 C     /tmp/C/3    0755 daemon daemon - /tmp/C/3-origin
 EOF
 

diff --git a/test/units/testsuite-22.04.sh b/test/units/testsuite-22.04.sh
index f916086b1edf47f59e82a5887cfa42282b411bc5..fc90ab4dc7fcce8464d226d12a6e16bf6afe612b 100755 (executable)
--- a/test/units/testsuite-22.04.sh
+++ b/test/units/testsuite-22.04.sh
@@ -17,8 +17,8 @@ EOF
 test -p /tmp/p/fifo1
 test $(stat -c %U:%G:%a /tmp/p/fifo1) = "root:root:666"
 
-# it should refuse to overwrite an existing file
-! systemd-tmpfiles --create - <<EOF
+# Refuse to overwrite an existing file. Error is not propagated.
+systemd-tmpfiles --create - <<EOF
 p     /tmp/p/f1    0666 - - - -
 EOF