tmpfiles: use dir_cleanup() for R and D

... i.e. apply nested config (exclusions and such) when executing R and D.

This fixes a long-standing RFE. The existing logic seems to have been an
accident of implementation. After all, if somebody specifies a config with
'R /foo; x /tmp/bar', then probably the goal is to remove stuff from under /foo,
but keep /tmp/bar. If they just wanted to nuke everything, then would not specify
the second item.

This also makes R and D use O_NOATIME, i.e. the access times of the directories
that are accessed will not be changed by the cleanup.

Obviously, we'll have to add this to NEWS and such.
Looking at the whole tmpfiles.d config in Fedora, this change has no effect.

The test cases are adjusted as appropriate. I also added another test case for
'R'/'D' with a file, just to test this code path more.

Replaces #20641.
Fixes #1633.

diff --git a/TODO b/TODO
index 1be6f2e4b316c07d2b4867b654c3294382872b01..07be5d932ca24fc214148b851da2ff21b75d9690 100644 (file)
--- a/TODO
+++ b/TODO
@@ -2446,7 +2446,6 @@ Features:
 * support crash reporting operation modes (https://live.gnome.org/GnomeOS/Design/Whiteboards/ProblemReporting)
 
 * tmpfiles:
-  - apply "x" on "D" too (see patch from William Douglas)
   - allow time-based cleanup in r and R too
   - instead of ignoring unknown fields, reject them.
   - creating new directories/subvolumes/fifos/device nodes

diff --git a/man/tmpfiles.d.xml b/man/tmpfiles.d.xml
index f8bdffcae33f6e7fe49e62d20963ccd76cd24863..76807b92e5cf5f365b06cd93c96cdfe2aea5de3a 100644 (file)
--- a/man/tmpfiles.d.xml
+++ b/man/tmpfiles.d.xml
@@ -353,9 +353,7 @@ L     /tmp/foobar -    -    -     -   /dev/null</programlisting>
           <term><varname>x</varname></term>
           <listitem><para>Ignore a path during cleaning. Use this type
           to exclude paths from clean-up as controlled with the Age
-          parameter. Note that lines of this type do not influence the
-          effect of <varname>r</varname> or <varname>R</varname>
-          lines. Lines of this type accept shell-style globs in place
+          parameter. Lines of this type accept shell-style globs in place
           of normal path names.  </para></listitem>
         </varlistentry>
 
@@ -365,9 +363,7 @@ L     /tmp/foobar -    -    -     -   /dev/null</programlisting>
           to exclude paths from clean-up as controlled with the Age
           parameter. Unlike <varname>x</varname>, this parameter will
           not exclude the content if path is a directory, but only
-          directory itself. Note that lines of this type do not
-          influence the effect of <varname>r</varname> or
-          <varname>R</varname> lines. Lines of this type accept
+          directory itself. Lines of this type accept
           shell-style globs in place of normal path names.
           </para>
 

diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c
index e877a5117aa4782e6d980ad98290ffb65adf866e..f31235921e8e09adab990054d286c68d43ea9d6d 100644 (file)
--- a/src/tmpfiles/tmpfiles.c
+++ b/src/tmpfiles/tmpfiles.c
@@ -904,7 +904,7 @@ static int dir_cleanup(
         }
 
 finish:
-        if (deleted) {
+        if (deleted && (self_atime_nsec < NSEC_INFINITY || self_mtime_nsec < NSEC_INFINITY)) {
                 struct timespec ts[2];
 
                 log_debug("Restoring access and modification time on \"%s\": %s, %s",
@@ -2907,26 +2907,62 @@ static int create_item(Context *c, Item *i) {
         return 0;
 }
 
-static int purge_item_instance(Context *c, Item *i, const char *instance, CreationMode creation) {
+static int remove_recursive(
+                Context *c,
+                Item *i,
+                const char *instance,
+                bool remove_instance) {
+
+        _cleanup_closedir_ DIR *d = NULL;
+        STRUCT_STATX_DEFINE(sx);
+        bool mountpoint;
         int r;
 
-        /* FIXME: we probably should use dir_cleanup() here instead of rm_rf() so that 'x' is honoured. */
-        log_debug("rm -rf \"%s\"", instance);
-        r = rm_rf(instance, REMOVE_ROOT|REMOVE_SUBVOLUME|REMOVE_PHYSICAL);
-        if (r < 0 && r != -ENOENT)
-                return log_error_errno(r, "rm_rf(%s): %m", instance);
+        r = opendir_and_stat(instance, &d, &sx, &mountpoint);
+        if (r < 0)
+                return r;
+        if (r == 0) {
+                if (remove_instance) {
+                        log_debug("Removing file \"%s\".", instance);
+                        if (remove(instance) < 0 && errno != ENOENT)
+                                return log_error_errno(errno, "rm %s: %m", instance);
+                }
+                return 0;
+        }
+
+        r = dir_cleanup(c, i, instance, d,
+                        /* self_atime_nsec= */ NSEC_INFINITY,
+                        /* self_mtime_nsec= */ NSEC_INFINITY,
+                        /* cutoff_nsec= */ NSEC_INFINITY,
+                        sx.stx_dev_major, sx.stx_dev_minor,
+                        mountpoint,
+                        MAX_DEPTH,
+                        /* keep_this_level= */ false,
+                        /* age_by_file= */ 0,
+                        /* age_by_dir= */ 0);
+        if (r < 0)
+                return r;
 
+        if (remove_instance) {
+                log_debug("Removing directory \"%s\".", instance);
+                r = RET_NERRNO(rmdir(instance));
+                if (r < 0 && !IN_SET(r, -ENOENT, -ENOTEMPTY))
+                        return log_error_errno(r, "Failed to remove %s: %m", instance);
+        }
         return 0;
 }
 
-static int purge_item(Context *c, Item *i) {
+static int purge_item_instance(Context *c, Item *i, const char *instance, CreationMode creation) {
+        return remove_recursive(c, i, instance, /* remove_instance= */ true);
+}
 
+static int purge_item(Context *c, Item *i) {
         assert(i);
 
         if (!needs_purge(i->type))
                 return 0;
 
-        log_debug("Running purge owned action for entry %c %s", (char) i->type, i->path);
+        log_debug("Running purge action for entry %c %s", (char) i->type, i->path);
 
         if (needs_glob(i->type))
                 return glob_item(c, i, purge_item_instance);
@@ -2940,8 +2976,6 @@ static int remove_item_instance(
                 const char *instance,
                 CreationMode creation) {
 
-        int r;
-
         assert(c);
         assert(i);
 
@@ -2949,29 +2983,19 @@ static int remove_item_instance(
 
         case REMOVE_PATH:
                 if (remove(instance) < 0 && errno != ENOENT)
-                        return log_error_errno(errno, "rm(%s): %m", instance);
+                        return log_error_errno(errno, "rm %s: %m", instance);
 
-                break;
+                return 0;
 
         case RECURSIVE_REMOVE_PATH:
-                /* FIXME: we probably should use dir_cleanup() here instead of rm_rf() so that 'x' is honoured. */
-                log_debug("rm -rf \"%s\"", instance);
-                r = rm_rf(instance, REMOVE_ROOT|REMOVE_SUBVOLUME|REMOVE_PHYSICAL);
-                if (r < 0 && r != -ENOENT)
-                        return log_error_errno(r, "rm_rf(%s): %m", instance);
-
-                break;
+                return remove_recursive(c, i, instance, /* remove_instance= */ true);
 
         default:
                 assert_not_reached();
         }
-
-        return 0;
 }
 
 static int remove_item(Context *c, Item *i) {
-        int r;
-
         assert(c);
         assert(i);
 
@@ -2980,13 +3004,7 @@ static int remove_item(Context *c, Item *i) {
         switch (i->type) {
 
         case TRUNCATE_DIRECTORY:
-                /* FIXME: we probably should use dir_cleanup() here instead of rm_rf() so that 'x' is honoured. */
-                log_debug("rm -rf \"%s\"", i->path);
-                r = rm_rf(i->path, REMOVE_PHYSICAL);
-                if (r < 0 && r != -ENOENT)
-                        return log_error_errno(r, "rm_rf(%s): %m", i->path);
-
-                return 0;
+                return remove_recursive(c, i, i->path, /* remove_instance= */ false);
 
         case REMOVE_PATH:
         case RECURSIVE_REMOVE_PATH:

diff --git a/test/units/testsuite-22.11.sh b/test/units/testsuite-22.11.sh
index f71a95f4f394f1983a63a9a7b87977b7bca3c643..deee6fe5d1cb112a4a62532cbefbb0f2d54a35db 100755 (executable)
--- a/test/units/testsuite-22.11.sh
+++ b/test/units/testsuite-22.11.sh
@@ -126,16 +126,34 @@ mkdir -p /tmp/x/{1,2}/a
 touch /tmp/x/1/a/{x1,x2} /tmp/x/2/a/{y1,y2}
 
 systemd-tmpfiles --remove - <<EOF
-# X is not supposed to influence R
+# Check that X is honoured below R
 X /tmp/x/1/a
 X /tmp/x/2/a
 R /tmp/x/1
 EOF
 
 find /tmp/x | sort
-test ! -d /tmp/x/1
-test ! -d /tmp/x/1/a
-test ! -f /tmp/x/1/a/x1
-test ! -f /tmp/x/1/a/x2
+test -d /tmp/x/1
+test -d /tmp/x/1/a
+test -f /tmp/x/1/a/x1
+test -f /tmp/x/1/a/x2
 test -f /tmp/x/2/a/y1
 test -f /tmp/x/2/a/y2
+
+#
+# 'r/R/D' and non-directories
+#
+
+touch /tmp/x/{11,22,33}
+
+systemd-tmpfiles --remove - <<EOF
+# Check that X is honoured below R
+r /tmp/x/11
+R /tmp/x/22
+D /tmp/x/33
+EOF
+
+find /tmp/x | sort
+test ! -f /tmp/x/11
+test ! -f /tmp/x/22
+test -f /tmp/x/33