tmpfiles: make handling of existing-but-different targets more consistent

create_fifo() was added in a2fc2f8dd3, 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.

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>

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);
 

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
 

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