[systemd-commits] 9 commits - .gitignore Makefile.am TODO man/coredump.conf.xml src/core src/journal

Lennart Poettering lennart at kemper.freedesktop.org
Fri Jun 27 10:36:07 PDT 2014


 .gitignore                         |    1 
 Makefile.am                        |   16 ++
 TODO                               |    2 
 man/coredump.conf.xml              |   35 ++--
 src/core/main.c                    |    7 
 src/journal/coredump-vacuum.c      |  272 +++++++++++++++++++++++++++++++++++++
 src/journal/coredump-vacuum.h      |   26 +++
 src/journal/coredump.c             |  220 +++++++++++++++--------------
 src/journal/coredump.conf          |    6 
 src/journal/journal-vacuum.c       |    7 
 src/journal/test-coredump-vacuum.c |   32 ++++
 11 files changed, 493 insertions(+), 131 deletions(-)

New commits:
commit b59233e6a3b30c0529d1649c8f7e69790733787d
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 19:32:14 2014 +0200

    coredump: simplify compression logic a bit
    
    This also make sure we remove the original coredump temporary file if we
    successfully managed to compress the coredump.

diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 0dae0af..78e89d3 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -139,6 +139,8 @@ static int fix_acl(int fd, uid_t uid) {
         acl_entry_t entry;
         acl_permset_t permset;
 
+        assert(fd >= 0);
+
         if (uid <= SYSTEM_UID_MAX)
                 return 0;
 
@@ -189,6 +191,8 @@ static int fix_xattr(int fd, const char *info[_INFO_LEN]) {
         int r = 0;
         unsigned i;
 
+        assert(fd >= 0);
+
         /* Attach some metadata to coredumps via extended
          * attributes. Just because we can. */
 
@@ -208,8 +212,17 @@ static int fix_xattr(int fd, const char *info[_INFO_LEN]) {
 
 #define filename_escape(s) xescape((s), "./ ")
 
-static int fix_permissions(int fd, const char *filename, const char *target,
-                           const char *info[_INFO_LEN], uid_t uid) {
+static int fix_permissions(
+                int fd,
+                const char *filename,
+                const char *target,
+                const char *info[_INFO_LEN],
+                uid_t uid) {
+
+        assert(fd >= 0);
+        assert(filename);
+        assert(target);
+        assert(info);
 
         /* Ignore errors on these */
         fchmod(fd, 0640);
@@ -231,7 +244,7 @@ static int fix_permissions(int fd, const char *filename, const char *target,
 
 static int maybe_remove_external_coredump(const char *filename, off_t size) {
 
-        /* Returns 1 if might remove, 0 if will not remove, <0 on error. */
+        /* Returns 1 if might remove, 0 if will not remove, < 0 on error. */
 
         if (IN_SET(arg_storage, COREDUMP_STORAGE_EXTERNAL, COREDUMP_STORAGE_BOTH) &&
             size <= arg_external_size_max)
@@ -248,55 +261,67 @@ static int maybe_remove_external_coredump(const char *filename, off_t size) {
         return 1;
 }
 
-static int save_external_coredump(
-                const char *info[_INFO_LEN],
-                uid_t uid,
-                char **ret_filename,
-                int *ret_fd,
-                off_t *ret_size) {
-
-        _cleanup_free_ char *p = NULL, *t = NULL, *c = NULL, *fn = NULL, *tmp = NULL, *u = NULL;
-        _cleanup_close_ int fd = -1;
+static int make_filename(const char *info[_INFO_LEN], char **ret) {
+        _cleanup_free_ char *c = NULL, *u = NULL, *p = NULL, *t = NULL;
         sd_id128_t boot;
-        struct stat st;
         int r;
 
         assert(info);
-        assert(ret_filename);
-        assert(ret_fd);
-        assert(ret_size);
 
         c = filename_escape(info[INFO_COMM]);
         if (!c)
-                return log_oom();
-
-        p = filename_escape(info[INFO_PID]);
-        if (!p)
-                return log_oom();
+                return -ENOMEM;
 
         u = filename_escape(info[INFO_UID]);
         if (!u)
-                return log_oom();
-
-        t = filename_escape(info[INFO_TIMESTAMP]);
-        if (!t)
-                return log_oom();
+                return -ENOMEM;
 
         r = sd_id128_get_boot(&boot);
-        if (r < 0) {
-                log_error("Failed to determine boot ID: %s", strerror(-r));
+        if (r < 0)
                 return r;
-        }
 
-        r = asprintf(&fn,
+        p = filename_escape(info[INFO_PID]);
+        if (!p)
+                return -ENOMEM;
+
+        t = filename_escape(info[INFO_TIMESTAMP]);
+        if (!t)
+                return -ENOMEM;
+
+        if (asprintf(ret,
                      "/var/lib/systemd/coredump/core.%s.%s." SD_ID128_FORMAT_STR ".%s.%s000000",
                      c,
                      u,
                      SD_ID128_FORMAT_VAL(boot),
                      p,
-                     t);
-        if (r < 0)
-                return log_oom();
+                     t) < 0)
+                return -ENOMEM;
+
+        return 0;
+}
+
+static int save_external_coredump(
+                const char *info[_INFO_LEN],
+                uid_t uid,
+                char **ret_filename,
+                int *ret_fd,
+                off_t *ret_size) {
+
+        _cleanup_free_ char *fn = NULL, *tmp = NULL;
+        _cleanup_close_ int fd = -1;
+        struct stat st;
+        int r;
+
+        assert(info);
+        assert(ret_filename);
+        assert(ret_fd);
+        assert(ret_size);
+
+        r = make_filename(info, &fn);
+        if (r < 0) {
+                log_error("Failed to determine coredump file name: %s", strerror(-r));
+                return r;
+        }
 
         tmp = tempfn_random(fn);
         if (!tmp)
@@ -329,7 +354,7 @@ static int save_external_coredump(
 
         if (lseek(fd, 0, SEEK_SET) == (off_t) -1) {
                 log_error("Failed to seek on %s: %m", tmp);
-                goto uncompressed;
+                goto fail;
         }
 
 #ifdef HAVE_XZ
@@ -337,45 +362,51 @@ static int save_external_coredump(
         if (maybe_remove_external_coredump(NULL, st.st_size) == 0
             && arg_compress) {
 
-                _cleanup_free_ char *fn2 = NULL;
-                char *tmp2;
-                _cleanup_close_ int fd2 = -1;
+                _cleanup_free_ char *fn_compressed = NULL, *tmp_compressed = NULL;
+                _cleanup_close_ int fd_compressed = -1;
 
-                tmp2 = strappenda(tmp, ".xz");
-                fd2 = open(tmp2, O_CREAT|O_EXCL|O_RDWR|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0640);
-                if (fd2 < 0) {
-                        log_error("Failed to create file %s: %m", tmp2);
+                fn_compressed = strappend(fn, ".xz");
+                if (!fn_compressed) {
+                        r = log_oom();
                         goto uncompressed;
                 }
 
-                r = compress_stream(fd, fd2, LZMA_PRESET_DEFAULT, -1);
-                if (r < 0) {
-                        log_error("Failed to compress %s: %s", tmp2, strerror(-r));
-                        unlink_noerrno(tmp2);
-                        goto fail2;
+                tmp_compressed = tempfn_random(fn_compressed);
+                if (!tmp_compressed) {
+                        r = log_oom();
+                        goto uncompressed;
                 }
 
-                fn2 = strappend(fn, ".xz");
-                if (!fn2) {
-                        log_oom();
-                        goto fail2;
+                fd_compressed = open(tmp_compressed, O_CREAT|O_EXCL|O_RDWR|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0640);
+                if (fd_compressed < 0) {
+                        log_error("Failed to create file %s: %m", tmp_compressed);
+                        goto uncompressed;
                 }
 
-                r = fix_permissions(fd2, tmp2, fn2, info, uid);
+                r = compress_stream(fd, fd_compressed, LZMA_PRESET_DEFAULT, -1);
+                if (r < 0) {
+                        log_error("Failed to compress %s: %s", tmp_compressed, strerror(-r));
+                        goto fail_compressed;
+                }
+
+                r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, info, uid);
                 if (r < 0)
-                        goto fail2;
+                        goto fail_compressed;
+
+                /* OK, this worked, we can get rid of the uncompressed version now */
+                unlink_noerrno(tmp);
 
-                *ret_filename = fn2;    /* compressed */
-                *ret_fd = fd;           /* uncompressed */
-                *ret_size = st.st_size; /* uncompressed */
+                *ret_filename = fn_compressed;    /* compressed */
+                *ret_fd = fd;                     /* uncompressed */
+                *ret_size = st.st_size;           /* uncompressed */
 
-                fn2 = NULL;
+                fn_compressed = NULL;
                 fd = -1;
 
                 return 0;
 
-        fail2:
-                unlink_noerrno(tmp2);
+        fail_compressed:
+                unlink_noerrno(tmp_compressed);
         }
 #endif
 

commit 8c9571d0ae50656f730a5e37378d5c3dcf3b9789
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 19:09:22 2014 +0200

    coredump: replace Compression= setting by simpler Compress= boolean setting
    
    Let's move things closer to journald's configuration settings, which
    knows Compress= already, as a boolean. This makes things more uniform,
    but also gives us more freedom to possibly swap out the used compression
    algorithm one day.

diff --git a/man/coredump.conf.xml b/man/coredump.conf.xml
index cc54466..c87cf68 100644
--- a/man/coredump.conf.xml
+++ b/man/coredump.conf.xml
@@ -88,11 +88,11 @@
       </varlistentry>
 
       <varlistentry>
-        <term><varname>Compression=</varname></term>
+        <term><varname>Compress=</varname></term>
 
         <listitem><para>Controls the type of compression for external
-        storage. One of <literal>xz</literal> or
-        <literal>none</literal>.</para>
+        storage. Takes a boolean argument, defaults to
+        <literal>yes</literal>.</para>
         </listitem>
       </varlistentry>
 
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 28cde66..0dae0af 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -98,29 +98,10 @@ static const char* const coredump_storage_table[_COREDUMP_STORAGE_MAX] = {
 };
 
 DEFINE_PRIVATE_STRING_TABLE_LOOKUP(coredump_storage, CoredumpStorage);
-static DEFINE_CONFIG_PARSE_ENUM(config_parse_coredump_storage, coredump_storage,
-                                CoredumpStorage,
-                                "Failed to parse storage setting");
-
-typedef enum CoredumpCompression {
-        COREDUMP_COMPRESSION_NONE,
-        COREDUMP_COMPRESSION_XZ,
-        _COREDUMP_COMPRESSION_MAX,
-        _COREDUMP_COMPRESSION_INVALID = -1
-} CoredumpCompression;
-
-static const char* const coredump_compression_table[_COREDUMP_COMPRESSION_MAX] = {
-        [COREDUMP_COMPRESSION_NONE] = "none",
-        [COREDUMP_COMPRESSION_XZ] = "xz",
-};
-
-DEFINE_PRIVATE_STRING_TABLE_LOOKUP(coredump_compression, CoredumpCompression);
-static DEFINE_CONFIG_PARSE_ENUM(config_parse_coredump_compression, coredump_compression,
-                                CoredumpCompression,
-                                "Failed to parse compression setting");
+static DEFINE_CONFIG_PARSE_ENUM(config_parse_coredump_storage, coredump_storage, CoredumpStorage, "Failed to parse storage setting");
 
 static CoredumpStorage arg_storage = COREDUMP_STORAGE_EXTERNAL;
-static CoredumpCompression arg_compression = COREDUMP_COMPRESSION_XZ;
+static bool arg_compress = true;
 static off_t arg_process_size_max = PROCESS_SIZE_MAX;
 static off_t arg_external_size_max = EXTERNAL_SIZE_MAX;
 static size_t arg_journal_size_max = JOURNAL_SIZE_MAX;
@@ -129,13 +110,13 @@ static off_t arg_max_use = (off_t) -1;
 
 static int parse_config(void) {
         static const ConfigTableItem items[] = {
-                { "Coredump", "Storage",          config_parse_coredump_storage,     0, &arg_storage           },
-                { "Coredump", "Compression",      config_parse_coredump_compression, 0, &arg_compression       },
-                { "Coredump", "ProcessSizeMax",   config_parse_iec_off,              0, &arg_process_size_max  },
-                { "Coredump", "ExternalSizeMax",  config_parse_iec_off,              0, &arg_external_size_max },
-                { "Coredump", "JournalSizeMax",   config_parse_iec_size,             0, &arg_journal_size_max  },
-                { "Coredump", "KeepFree",         config_parse_iec_off,              0, &arg_keep_free         },
-                { "Coredump", "MaxUse",           config_parse_iec_off,              0, &arg_max_use           },
+                { "Coredump", "Storage",          config_parse_coredump_storage,  0, &arg_storage           },
+                { "Coredump", "Compress",         config_parse_bool,              0, &arg_compress          },
+                { "Coredump", "ProcessSizeMax",   config_parse_iec_off,           0, &arg_process_size_max  },
+                { "Coredump", "ExternalSizeMax",  config_parse_iec_off,           0, &arg_external_size_max },
+                { "Coredump", "JournalSizeMax",   config_parse_iec_size,          0, &arg_journal_size_max  },
+                { "Coredump", "KeepFree",         config_parse_iec_off,           0, &arg_keep_free         },
+                { "Coredump", "MaxUse",           config_parse_iec_off,           0, &arg_max_use           },
                 {}
         };
 
@@ -354,7 +335,7 @@ static int save_external_coredump(
 #ifdef HAVE_XZ
         /* If we will remove the coredump anyway, do not compress. */
         if (maybe_remove_external_coredump(NULL, st.st_size) == 0
-            && arg_compression == COREDUMP_COMPRESSION_XZ) {
+            && arg_compress) {
 
                 _cleanup_free_ char *fn2 = NULL;
                 char *tmp2;
@@ -493,10 +474,9 @@ int main(int argc, char* argv[]) {
 
         /* Ignore all parse errors */
         parse_config();
-        log_debug("Selected storage '%s'.",
-                  coredump_storage_to_string(arg_storage));
-        log_debug("Selected compression %s.",
-                  coredump_compression_to_string(arg_compression));
+
+        log_debug("Selected storage '%s'.", coredump_storage_to_string(arg_storage));
+        log_debug("Selected compression %s.", yes_no(arg_compress));
 
         r = parse_uid(argv[INFO_UID + 1], &uid);
         if (r < 0) {
diff --git a/src/journal/coredump.conf b/src/journal/coredump.conf
index 20424f2..0cc328f 100644
--- a/src/journal/coredump.conf
+++ b/src/journal/coredump.conf
@@ -9,7 +9,7 @@
 
 [Coredump]
 #Storage=external
-#Compression=xz
+#Compress=yes
 #ProcessSizeMax=2G
 #ExternalSizeMax=2G
 #JournalSizeMax=767M

commit cf677ac1b7ae1e46c593d055df27f36528be548a
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 19:04:31 2014 +0200

    coredump: don't expose the compression level as configuration option
    
    This sounds overly low-level and implementation-detaily. Let's just
    use the default level XZ suggests. This gives us more room to possibly
    swap out the compression algorithm used, as the compression level range
    will not leak into user configuration.

diff --git a/man/coredump.conf.xml b/man/coredump.conf.xml
index e24d958..cc54466 100644
--- a/man/coredump.conf.xml
+++ b/man/coredump.conf.xml
@@ -97,15 +97,6 @@
       </varlistentry>
 
       <varlistentry>
-        <term><varname>CompressionLevel=</varname></term>
-
-        <listitem><para>Controls the level of compression for external
-        storage. An integer between 0 and 9. See
-        <citerefentry><refentrytitle>xz</refentrytitle><manvolnum>1</manvolnum></citerefentry>
-        for more details.</para></listitem>
-      </varlistentry>
-
-      <varlistentry>
         <term><varname>ProcessSizeMax=</varname></term>
 
         <listitem><para>The maximum size in bytes of a core
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 6499fbd..28cde66 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -121,7 +121,6 @@ static DEFINE_CONFIG_PARSE_ENUM(config_parse_coredump_compression, coredump_comp
 
 static CoredumpStorage arg_storage = COREDUMP_STORAGE_EXTERNAL;
 static CoredumpCompression arg_compression = COREDUMP_COMPRESSION_XZ;
-static unsigned arg_compression_level = LZMA_PRESET_DEFAULT;
 static off_t arg_process_size_max = PROCESS_SIZE_MAX;
 static off_t arg_external_size_max = EXTERNAL_SIZE_MAX;
 static size_t arg_journal_size_max = JOURNAL_SIZE_MAX;
@@ -129,12 +128,9 @@ static off_t arg_keep_free = (off_t) -1;
 static off_t arg_max_use = (off_t) -1;
 
 static int parse_config(void) {
-        int r;
-
         static const ConfigTableItem items[] = {
                 { "Coredump", "Storage",          config_parse_coredump_storage,     0, &arg_storage           },
                 { "Coredump", "Compression",      config_parse_coredump_compression, 0, &arg_compression       },
-                { "Coredump", "CompressionLevel", config_parse_unsigned,             0, &arg_compression_level },
                 { "Coredump", "ProcessSizeMax",   config_parse_iec_off,              0, &arg_process_size_max  },
                 { "Coredump", "ExternalSizeMax",  config_parse_iec_off,              0, &arg_external_size_max },
                 { "Coredump", "JournalSizeMax",   config_parse_iec_size,             0, &arg_journal_size_max  },
@@ -143,7 +139,7 @@ static int parse_config(void) {
                 {}
         };
 
-        r = config_parse(
+        return config_parse(
                         NULL,
                         "/etc/systemd/coredump.conf",
                         NULL,
@@ -153,15 +149,6 @@ static int parse_config(void) {
                         false,
                         false,
                         NULL);
-
-#ifdef HAVE_XZ
-        if (arg_compression_level > 9) {
-                log_warning("Invalid CompressionLevel %u, ignoring.", arg_compression_level);
-                arg_compression_level = LZMA_PRESET_DEFAULT;
-        }
-#endif
-
-        return r;
 }
 
 static int fix_acl(int fd, uid_t uid) {
@@ -380,7 +367,7 @@ static int save_external_coredump(
                         goto uncompressed;
                 }
 
-                r = compress_stream(fd, fd2, arg_compression_level, -1);
+                r = compress_stream(fd, fd2, LZMA_PRESET_DEFAULT, -1);
                 if (r < 0) {
                         log_error("Failed to compress %s: %s", tmp2, strerror(-r));
                         unlink_noerrno(tmp2);
@@ -508,9 +495,8 @@ int main(int argc, char* argv[]) {
         parse_config();
         log_debug("Selected storage '%s'.",
                   coredump_storage_to_string(arg_storage));
-        log_debug("Selected compression %s:%u.",
-                  coredump_compression_to_string(arg_compression),
-                  arg_compression_level);
+        log_debug("Selected compression %s.",
+                  coredump_compression_to_string(arg_compression));
 
         r = parse_uid(argv[INFO_UID + 1], &uid);
         if (r < 0) {
diff --git a/src/journal/coredump.conf b/src/journal/coredump.conf
index 050bde6..20424f2 100644
--- a/src/journal/coredump.conf
+++ b/src/journal/coredump.conf
@@ -10,8 +10,6 @@
 [Coredump]
 #Storage=external
 #Compression=xz
-#CompressionLevel=6
-
 #ProcessSizeMax=2G
 #ExternalSizeMax=2G
 #JournalSizeMax=767M

commit 168562d7ed8fcfc4612e51d221e7a5fa1e1297b5
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 18:59:30 2014 +0200

    journald: invoking fstatvfs() is now redundant in the vacuuming code

diff --git a/src/journal/journal-vacuum.c b/src/journal/journal-vacuum.c
index 086b40f..891b6bb 100644
--- a/src/journal/journal-vacuum.c
+++ b/src/journal/journal-vacuum.c
@@ -292,13 +292,6 @@ int journal_directory_vacuum(
         qsort_safe(list, n_list, sizeof(struct vacuum_info), vacuum_compare);
 
         for (i = 0; i < n_list; i++) {
-                struct statvfs ss;
-
-                if (fstatvfs(dirfd(d), &ss) < 0) {
-                        r = -errno;
-                        goto finish;
-                }
-
                 if ((max_retention_usec <= 0 || list[i].realtime >= retention_limit) &&
                     (max_use <= 0 || sum <= max_use))
                         break;

commit 9d951bf491a2b6d32b050e8bdc301617560813c8
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 18:59:12 2014 +0200

    coredump: don't be annoyed if another coredump hook removes our coredump while we work on it

diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 7e11cea..6499fbd 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -272,7 +272,7 @@ static int maybe_remove_external_coredump(const char *filename, off_t size) {
         if (!filename)
                 return 1;
 
-        if (unlink(filename) < 0) {
+        if (unlink(filename) < 0 && errno != ENOENT) {
                 log_error("Failed to unlink %s: %m", filename);
                 return -errno;
         }

commit cfc194575baaed980e4d4e947ec9d456ebf07838
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 18:58:44 2014 +0200

    coredump: fix how the compression level is verified

diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index 22e1932..7e11cea 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -129,12 +129,12 @@ static off_t arg_keep_free = (off_t) -1;
 static off_t arg_max_use = (off_t) -1;
 
 static int parse_config(void) {
+        int r;
 
         static const ConfigTableItem items[] = {
                 { "Coredump", "Storage",          config_parse_coredump_storage,     0, &arg_storage           },
                 { "Coredump", "Compression",      config_parse_coredump_compression, 0, &arg_compression       },
                 { "Coredump", "CompressionLevel", config_parse_unsigned,             0, &arg_compression_level },
-
                 { "Coredump", "ProcessSizeMax",   config_parse_iec_off,              0, &arg_process_size_max  },
                 { "Coredump", "ExternalSizeMax",  config_parse_iec_off,              0, &arg_external_size_max },
                 { "Coredump", "JournalSizeMax",   config_parse_iec_size,             0, &arg_journal_size_max  },
@@ -143,7 +143,7 @@ static int parse_config(void) {
                 {}
         };
 
-        return config_parse(
+        r = config_parse(
                         NULL,
                         "/etc/systemd/coredump.conf",
                         NULL,
@@ -160,6 +160,8 @@ static int parse_config(void) {
                 arg_compression_level = LZMA_PRESET_DEFAULT;
         }
 #endif
+
+        return r;
 }
 
 static int fix_acl(int fd, uid_t uid) {

commit 229811628584b370e3fa7e8524d66be46c5a4661
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 18:58:23 2014 +0200

    update TODO

diff --git a/TODO b/TODO
index 8d8d694..8a01fc2 100644
--- a/TODO
+++ b/TODO
@@ -25,6 +25,8 @@ External:
 
 Features:
 
+* ship sleep.conf by default
+
 * coredump: add a size-based cleanup logic to coredump helper, so that we don't flood /var/lib/systemd/coredump unrestricted
 
 * support empty /etc boots nicely:

commit 0dc5d23c85db85f96b141d4d32deee8018e56a6a
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 18:57:24 2014 +0200

    coredump: add simple coredump vacuuming
    
    When disk space taken up by coredumps grows beyond a configured limit
    start removing the oldest coredump of the user with the most coredumps,
    until we get below the limit again.

diff --git a/.gitignore b/.gitignore
index d2e81a2..fe8c32d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -142,6 +142,7 @@
 /test-cgroup-util
 /test-compress
 /test-conf-files
+/test-coredump-vacuum
 /test-daemon
 /test-date
 /test-device-nodes
diff --git a/Makefile.am b/Makefile.am
index e02dede..5d86f3e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3771,7 +3771,9 @@ systemd_socket_proxyd_LDADD = \
 # ------------------------------------------------------------------------------
 if ENABLE_COREDUMP
 systemd_coredump_SOURCES = \
-	src/journal/coredump.c
+	src/journal/coredump.c \
+	src/journal/coredump-vacuum.c \
+	src/journal/coredump-vacuum.h
 
 systemd_coredump_LDADD = \
 	libsystemd-journal-internal.la \
@@ -3810,6 +3812,18 @@ coredumpctl_LDADD = \
 bin_PROGRAMS += \
 	coredumpctl
 
+manual_tests += \
+	test-coredump-vacuum
+
+test_coredump_vacuum_SOURCES = \
+	src/journal/test-coredump-vacuum.c  \
+	src/journal/coredump-vacuum.c \
+	src/journal/coredump-vacuum.h
+
+test_coredump_vacuum_LDADD = \
+	libsystemd-internal.la \
+	libsystemd-shared.la
+
 dist_bashcompletion_DATA += \
 	shell-completion/bash/coredumpctl
 
diff --git a/man/coredump.conf.xml b/man/coredump.conf.xml
index 9e4adff..e24d958 100644
--- a/man/coredump.conf.xml
+++ b/man/coredump.conf.xml
@@ -121,6 +121,23 @@
         <listitem><para>The maximum (uncompressed) size in bytes of a
         core to be saved.</para></listitem>
       </varlistentry>
+
+      <varlistentry>
+        <term><varname>MaxUse=</varname></term>
+        <term><varname>KeepFree=</varname></term>
+
+        <listitem><para>Enforce limits on the disk space taken up by
+        externally stored coredumps. <option>MaxUse=</option> makes
+        sure that old coredumps are removed as soon as the total disk
+        space taken up by coredumps grows beyond this limit (defaults
+        to 10% of the total disk size). <option>KeepFree=</option>
+        controls how much disk space to keep free at least (defaults
+        to 15% of the total disk size). Note that the disk space used
+        by coredumps might temporarily exceed these limits while
+        coredumps are processed. Note that old coredumps are also
+        removed based on on time via
+        <citerefentry><refentrytitle>systemd-tmpfiles</refentrytitle><manvolnum>8</manvolnum></citerefentry>.</para></listitem>
+      </varlistentry>
     </variablelist>
 
   </refsect1>
@@ -129,7 +146,8 @@
     <title>See Also</title>
     <para>
       <citerefentry><refentrytitle>systemd-journald.service</refentrytitle><manvolnum>8</manvolnum></citerefentry>,
-      <citerefentry><refentrytitle>coredumpctl</refentrytitle><manvolnum>1</manvolnum></citerefentry>
+      <citerefentry><refentrytitle>coredumpctl</refentrytitle><manvolnum>1</manvolnum></citerefentry>,
+      <citerefentry><refentrytitle>systemd-tmpfiles</refentrytitle><manvolnum>8</manvolnum></citerefentry>
     </para>
   </refsect1>
 
diff --git a/src/journal/coredump-vacuum.c b/src/journal/coredump-vacuum.c
new file mode 100644
index 0000000..ad2e2fa
--- /dev/null
+++ b/src/journal/coredump-vacuum.c
@@ -0,0 +1,272 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <sys/statvfs.h>
+
+#include "util.h"
+#include "time-util.h"
+#include "hashmap.h"
+#include "macro.h"
+
+#include "coredump-vacuum.h"
+
+#define DEFAULT_MAX_USE_LOWER (off_t) (1ULL*1024ULL*1024ULL)           /* 1 MiB */
+#define DEFAULT_MAX_USE_UPPER (off_t) (4ULL*1024ULL*1024ULL*1024ULL)   /* 4 GiB */
+#define DEFAULT_KEEP_FREE_UPPER (off_t) (4ULL*1024ULL*1024ULL*1024ULL) /* 4 GiB */
+#define DEFAULT_KEEP_FREE (off_t) (1024ULL*1024ULL)                    /* 1 MB */
+
+struct vacuum_candidate {
+        unsigned n_files;
+        char *oldest_file;
+        usec_t oldest_mtime;
+};
+
+static void vacuum_candidate_free(struct vacuum_candidate *c) {
+        if (!c)
+                return;
+
+        free(c->oldest_file);
+        free(c);
+}
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct vacuum_candidate*, vacuum_candidate_free);
+
+static void vacuum_candidate_hasmap_free(Hashmap *h) {
+        struct vacuum_candidate *c;
+
+        while ((c = hashmap_steal_first(h)))
+                vacuum_candidate_free(c);
+
+        hashmap_free(h);
+}
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Hashmap*, vacuum_candidate_hasmap_free);
+
+static int uid_from_file_name(const char *filename, uid_t *uid) {
+        const char *p, *e, *u;
+
+        p = startswith(filename, "core.");
+        if (!p)
+                return -EINVAL;
+
+        /* Skip the comm field */
+        p = strchr(p, '.');
+        if (!p)
+                return -EINVAL;
+        p++;
+
+        /* Find end up UID */
+        e = strchr(p, '.');
+        if (!e)
+                return -EINVAL;
+
+        u = strndupa(p, e-p);
+        return parse_uid(u, uid);
+}
+
+static bool vacuum_necessary(int fd, off_t sum, off_t keep_free, off_t max_use) {
+        off_t fs_size = 0, fs_free = (off_t) -1;
+        struct statvfs sv;
+
+        assert(fd >= 0);
+
+        if (fstatvfs(fd, &sv) >= 0) {
+                fs_size = sv.f_frsize * sv.f_blocks;
+                fs_free = sv.f_frsize * sv.f_bfree;
+        }
+
+        if (max_use == (off_t) -1) {
+
+                if (fs_size > 0) {
+                        max_use = PAGE_ALIGN(fs_size / 10); /* 10% */
+
+                        if (max_use > DEFAULT_MAX_USE_UPPER)
+                                max_use = DEFAULT_MAX_USE_UPPER;
+
+                        if (max_use < DEFAULT_MAX_USE_LOWER)
+                                max_use = DEFAULT_MAX_USE_LOWER;
+                }
+
+                max_use = DEFAULT_MAX_USE_LOWER;
+        } else
+                max_use = PAGE_ALIGN(max_use);
+
+        if (max_use > 0 && sum > max_use)
+                return true;
+
+        if (keep_free == (off_t) -1) {
+
+                if (fs_size > 0) {
+                        keep_free = PAGE_ALIGN((fs_size * 3) / 20); /* 15% */
+
+                        if (keep_free > DEFAULT_KEEP_FREE_UPPER)
+                                keep_free = DEFAULT_KEEP_FREE_UPPER;
+                } else
+                        keep_free = DEFAULT_KEEP_FREE;
+        } else
+                keep_free = PAGE_ALIGN(keep_free);
+
+        if (keep_free > 0 && fs_free < keep_free)
+                return true;
+
+        return false;
+}
+
+int coredump_vacuum(int exclude_fd, off_t keep_free, off_t max_use) {
+        _cleanup_closedir_ DIR *d = NULL;
+        struct stat exclude_st;
+        int r;
+
+        if (keep_free <= 0 && max_use <= 0)
+                return 0;
+
+        if (exclude_fd >= 0) {
+                if (fstat(exclude_fd, &exclude_st) < 0) {
+                        log_error("Failed to fstat(): %m");
+                        return -errno;
+                }
+        }
+
+        /* This algorithm will keep deleting the oldest file of the
+         * user with the most coredumps until we are back in the size
+         * limits. Note that vacuuming for journal files is different,
+         * because we rely on rate-limiting of the messages there,
+         * to avoid being flooded. */
+
+        d = opendir("/var/lib/systemd/coredump");
+        if (!d) {
+                if (errno == ENOENT)
+                        return 0;
+
+                log_error("Can't open coredump directory: %m");
+                return -errno;
+        }
+
+        for (;;) {
+                _cleanup_(vacuum_candidate_hasmap_freep) Hashmap *h = NULL;
+                struct vacuum_candidate *worst = NULL;
+                struct dirent *de;
+                off_t sum = 0;
+
+                rewinddir(d);
+
+                FOREACH_DIRENT(de, d, goto fail) {
+                        struct vacuum_candidate *c;
+                        struct stat st;
+                        uid_t uid;
+                        usec_t t;
+
+                        r = uid_from_file_name(de->d_name, &uid);
+                        if (r < 0)
+                                continue;
+
+                        if (fstatat(dirfd(d), de->d_name, &st, AT_NO_AUTOMOUNT|AT_SYMLINK_NOFOLLOW) < 0) {
+                                if (errno == ENOENT)
+                                        continue;
+
+                                log_warning("Failed to stat /var/lib/systemd/coredump/%s", de->d_name);
+                                continue;
+                        }
+
+                        if (!S_ISREG(st.st_mode))
+                                continue;
+
+                        if (exclude_fd >= 0 &&
+                            exclude_st.st_dev == st.st_dev &&
+                            exclude_st.st_ino == st.st_ino)
+                                continue;
+
+                        r = hashmap_ensure_allocated(&h, NULL, NULL);
+                        if (r < 0)
+                                return log_oom();
+
+                        t = timespec_load(&st.st_mtim);
+
+                        c = hashmap_get(h, UINT32_TO_PTR(uid));
+                        if (c) {
+
+                                if (t < c->oldest_mtime) {
+                                        char *n;
+
+                                        n = strdup(de->d_name);
+                                        if (!n)
+                                                return log_oom();
+
+                                        free(c->oldest_file);
+                                        c->oldest_file = n;
+                                        c->oldest_mtime = t;
+                                }
+
+                        } else {
+                                _cleanup_(vacuum_candidate_freep) struct vacuum_candidate *n = NULL;
+
+                                n = new0(struct vacuum_candidate, 1);
+                                if (!n)
+                                        return log_oom();
+
+                                n->oldest_file = strdup(de->d_name);
+                                if (!n->oldest_file)
+                                        return log_oom();
+
+                                n->oldest_mtime = t;
+
+                                r = hashmap_put(h, UINT32_TO_PTR(uid), n);
+                                if (r < 0)
+                                        return log_oom();
+
+                                c = n;
+                                n = NULL;
+                        }
+
+                        c->n_files++;
+
+                        if (!worst ||
+                            worst->n_files < c->n_files ||
+                            (worst->n_files == c->n_files && c->oldest_mtime < worst->oldest_mtime))
+                                worst = c;
+
+                        sum += st.st_blocks * 512;
+                }
+
+                if (!worst)
+                        break;
+
+                r = vacuum_necessary(dirfd(d), sum, keep_free, max_use);
+                if (r <= 0)
+                        return r;
+
+                if (unlinkat(dirfd(d), worst->oldest_file, 0) < 0) {
+
+                        if (errno == ENOENT)
+                                continue;
+
+                        log_error("Failed to remove file %s: %m", worst->oldest_file);
+                        return -errno;
+                } else
+                        log_info("Removed old coredump %s.", worst->oldest_file);
+        }
+
+        return 0;
+
+fail:
+        log_error("Failed to read directory: %m");
+        return -errno;
+}
diff --git a/src/journal/coredump-vacuum.h b/src/journal/coredump-vacuum.h
new file mode 100644
index 0000000..7ad4399
--- /dev/null
+++ b/src/journal/coredump-vacuum.h
@@ -0,0 +1,26 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+  This file is part of systemd.
+
+  Copyright 2014 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <sys/types.h>
+
+int coredump_vacuum(int exclude_fd, off_t keep_free, off_t max_use);
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
index ab8fd2c..22e1932 100644
--- a/src/journal/coredump.c
+++ b/src/journal/coredump.c
@@ -42,6 +42,7 @@
 #include "stacktrace.h"
 #include "path-util.h"
 #include "compress.h"
+#include "coredump-vacuum.h"
 
 #ifdef HAVE_ACL
 #  include <sys/acl.h>
@@ -121,10 +122,11 @@ static DEFINE_CONFIG_PARSE_ENUM(config_parse_coredump_compression, coredump_comp
 static CoredumpStorage arg_storage = COREDUMP_STORAGE_EXTERNAL;
 static CoredumpCompression arg_compression = COREDUMP_COMPRESSION_XZ;
 static unsigned arg_compression_level = LZMA_PRESET_DEFAULT;
-
 static off_t arg_process_size_max = PROCESS_SIZE_MAX;
 static off_t arg_external_size_max = EXTERNAL_SIZE_MAX;
 static size_t arg_journal_size_max = JOURNAL_SIZE_MAX;
+static off_t arg_keep_free = (off_t) -1;
+static off_t arg_max_use = (off_t) -1;
 
 static int parse_config(void) {
 
@@ -136,6 +138,8 @@ static int parse_config(void) {
                 { "Coredump", "ProcessSizeMax",   config_parse_iec_off,              0, &arg_process_size_max  },
                 { "Coredump", "ExternalSizeMax",  config_parse_iec_off,              0, &arg_external_size_max },
                 { "Coredump", "JournalSizeMax",   config_parse_iec_size,             0, &arg_journal_size_max  },
+                { "Coredump", "KeepFree",         config_parse_iec_off,              0, &arg_keep_free         },
+                { "Coredump", "MaxUse",           config_parse_iec_off,              0, &arg_max_use           },
                 {}
         };
 
@@ -274,14 +278,14 @@ static int maybe_remove_external_coredump(const char *filename, off_t size) {
         return 1;
 }
 
+static int save_external_coredump(
+                const char *info[_INFO_LEN],
+                uid_t uid,
+                char **ret_filename,
+                int *ret_fd,
+                off_t *ret_size) {
 
-static int save_external_coredump(const char *info[_INFO_LEN],
-                                  uid_t uid,
-                                  char **ret_filename,
-                                  int *ret_fd,
-                                  off_t *ret_size) {
-
-        _cleanup_free_ char *p = NULL, *t = NULL, *c = NULL, *fn = NULL, *tmp = NULL;
+        _cleanup_free_ char *p = NULL, *t = NULL, *c = NULL, *fn = NULL, *tmp = NULL, *u = NULL;
         _cleanup_close_ int fd = -1;
         sd_id128_t boot;
         struct stat st;
@@ -300,6 +304,10 @@ static int save_external_coredump(const char *info[_INFO_LEN],
         if (!p)
                 return log_oom();
 
+        u = filename_escape(info[INFO_UID]);
+        if (!u)
+                return log_oom();
+
         t = filename_escape(info[INFO_TIMESTAMP]);
         if (!t)
                 return log_oom();
@@ -311,8 +319,9 @@ static int save_external_coredump(const char *info[_INFO_LEN],
         }
 
         r = asprintf(&fn,
-                     "/var/lib/systemd/coredump/core.%s." SD_ID128_FORMAT_STR ".%s.%s000000",
+                     "/var/lib/systemd/coredump/core.%s.%s." SD_ID128_FORMAT_STR ".%s.%s000000",
                      c,
+                     u,
                      SD_ID128_FORMAT_VAL(boot),
                      p,
                      t);
@@ -333,12 +342,10 @@ static int save_external_coredump(const char *info[_INFO_LEN],
 
         r = copy_bytes(STDIN_FILENO, fd, arg_process_size_max);
         if (r == -E2BIG) {
-                log_error("Coredump of %s (%s) is larger than configured processing limit, refusing.",
-                          info[INFO_PID], info[INFO_COMM]);
+                log_error("Coredump of %s (%s) is larger than configured processing limit, refusing.", info[INFO_PID], info[INFO_COMM]);
                 goto fail;
         } else if (IN_SET(r, -EDQUOT, -ENOSPC)) {
-                log_error("Not enough disk space for coredump of %s (%s), refusing.",
-                          info[INFO_PID], info[INFO_COMM]);
+                log_error("Not enough disk space for coredump of %s (%s), refusing.", info[INFO_PID], info[INFO_COMM]);
                 goto fail;
         } else if (r < 0) {
                 log_error("Failed to dump coredump to file: %s", strerror(-r));
@@ -646,6 +653,9 @@ int main(int argc, char* argv[]) {
         IOVEC_SET_STRING(iovec[j++], "MESSAGE_ID=fc2e22bc6ee647b6b90729ab34a250b1");
         IOVEC_SET_STRING(iovec[j++], "PRIORITY=2");
 
+        /* Vacuum before we write anything again */
+        coredump_vacuum(-1, arg_keep_free, arg_max_use);
+
         /* Always stream the coredump to disk, if that's possible */
         r = save_external_coredump(info, uid, &filename, &coredump_fd, &coredump_size);
         if (r < 0)
@@ -666,6 +676,9 @@ int main(int argc, char* argv[]) {
                 IOVEC_SET_STRING(iovec[j++], coredump_filename);
         }
 
+        /* Vacuum again, but exclude the coredump we just created */
+        coredump_vacuum(coredump_fd, arg_keep_free, arg_max_use);
+
         /* Now, let's drop privileges to become the user who owns the
          * segfaulted process and allocate the coredump memory under
          * his uid. This also ensures that the credentials journald
diff --git a/src/journal/coredump.conf b/src/journal/coredump.conf
index 9f4edd3..050bde6 100644
--- a/src/journal/coredump.conf
+++ b/src/journal/coredump.conf
@@ -15,3 +15,5 @@
 #ProcessSizeMax=2G
 #ExternalSizeMax=2G
 #JournalSizeMax=767M
+#MaxUse=
+#KeepFree=
diff --git a/src/journal/test-coredump-vacuum.c b/src/journal/test-coredump-vacuum.c
new file mode 100644
index 0000000..a4dd001
--- /dev/null
+++ b/src/journal/test-coredump-vacuum.c
@@ -0,0 +1,32 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2012 Zbigniew Jędrzejewski-Szmek
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <stdlib.h>
+
+#include "coredump-vacuum.h"
+
+int main(int argc, char *argv[]) {
+
+        if (coredump_vacuum(-1, (off_t) -1, 70 * 1024) < 0)
+                return EXIT_FAILURE;
+
+        return EXIT_SUCCESS;
+}

commit 1f97091d3cb0887c264176b47b0a86c269acf0b5
Author: Lennart Poettering <lennart at poettering.net>
Date:   Fri Jun 27 18:34:37 2014 +0200

    main: uid_to_name() might fail due to OOM, protect against that

diff --git a/src/core/main.c b/src/core/main.c
index 8ee12ef..38835fc 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1547,9 +1547,10 @@ int main(int argc, char *argv[]) {
                 if (empty_etc)
                         log_info("Running with unpopulated /etc.");
         } else {
-                _cleanup_free_ char *t = uid_to_name(getuid());
-                log_debug(PACKAGE_STRING " running in user mode for user "UID_FMT"/%s. (" SYSTEMD_FEATURES ")",
-                          getuid(), t);
+                _cleanup_free_ char *t;
+
+                t = uid_to_name(getuid());
+                log_debug(PACKAGE_STRING " running in user mode for user "UID_FMT"/%s. (" SYSTEMD_FEATURES ")", getuid(), strna(t));
         }
 
         if (arg_running_as == SYSTEMD_SYSTEM && !skip_setup) {



More information about the systemd-commits mailing list