system/systemd
system/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd synced 2024-10-14 20:17:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

coredump: remove Storage=both option

Browse source 
Back when external storage was initially added in 34c10968cb, this mode of
storage was added. This could have made some sense back when XZ compression was
used, and an uncompressed core on disk could be used as short-lived cache file
which does require costly decompression. But now fast LZ4 compression is used
(by default) both internally and externally, so we have duplicated storage,
using the same compression and same default maximum core size in both cases,
but with different expiration lifetimes. Even the uncompressed-external,
compressed-internal mode is not very useful: for small files, decompression
with LZ4 is fast enough not to matter, and for large files, decompression is
still relatively fast, but the disk-usage penalty is very big.

An additional problem with the two modes of storage is that it complicates
the code and makes it much harder to return a useful error message to the user
if we cannot find the core file, since if we cannot find the file we have to
check the internal storage first.

This patch drops "both" storage mode. Effectively this means that if somebody
configured coredump this way, they will get a warning about an unsupported
value for Storage, and the default of "external" will be used.
I'm pretty sure that this mode is very rarely used anyway.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2016-09-26 23:40:20 +02:00
parent 831d3dc8d7
commit fc6cec8613
3 changed files with 78 additions and 91 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
man/coredump.conf.xml
View file

@ -83,16 +83,13 @@
<varlistentry>
<term><varname>Storage=</varname></term>
<listitem><para>Controls where to store cores. One of
<literal>none</literal>, <literal>external</literal>,
<literal>journal</literal>, and <literal>both</literal>. When
<literal>none</literal>, the core dumps will be logged but not
stored permanently. When <literal>external</literal> (the
default), cores will be stored in <filename>/var/lib/systemd/coredump</filename>.
When <literal>journal</literal>, cores will be stored in
the journal and rotated following normal journal
rotation patterns. When <literal>both</literal>, cores
will be stored in both locations.</para>
<listitem><para>Controls where to store cores. One of <literal>none</literal>,
<literal>external</literal>, and <literal>journal</literal>. When
<literal>none</literal>, the core dumps will be logged (included the traceback if
possible), but not stored permanently. When <literal>external</literal> (the
default), cores will be stored in <filename>/var/lib/systemd/coredump/</filename>.
When <literal>journal</literal>, cores will be stored in the journal and rotated
following normal journal rotation patterns.</para>
<para>When cores are stored in the journal, they might be
compressed following journal compression settings, see

6
src/coredump/coredump.c
View file

@ -93,7 +93,6 @@ typedef enum CoredumpStorage {
COREDUMP_STORAGE_NONE,
COREDUMP_STORAGE_EXTERNAL,
COREDUMP_STORAGE_JOURNAL,
COREDUMP_STORAGE_BOTH,
_COREDUMP_STORAGE_MAX,
_COREDUMP_STORAGE_INVALID = -1
} CoredumpStorage;
@ -102,7 +101,6 @@ static const char* const coredump_storage_table[_COREDUMP_STORAGE_MAX] = {
[COREDUMP_STORAGE_NONE] = "none",
[COREDUMP_STORAGE_EXTERNAL] = "external",
[COREDUMP_STORAGE_JOURNAL] = "journal",
[COREDUMP_STORAGE_BOTH] = "both",
};
DEFINE_PRIVATE_STRING_TABLE_LOOKUP(coredump_storage, CoredumpStorage);
@ -247,7 +245,7 @@ static int maybe_remove_external_coredump(const char *filename, uint64_t size) {
/* Returns 1 if might remove, 0 if will not remove, < 0 on error. */
if (IN_SET(arg_storage, COREDUMP_STORAGE_EXTERNAL, COREDUMP_STORAGE_BOTH) &&
if (arg_storage == COREDUMP_STORAGE_EXTERNAL &&
size <= arg_external_size_max)
return 0;
@ -740,7 +738,7 @@ log:
IOVEC_SET_STRING(iovec[n_iovec++], core_message);
/* Optionally store the entire coredump in the journal */
if (IN_SET(arg_storage, COREDUMP_STORAGE_JOURNAL, COREDUMP_STORAGE_BOTH) &&
if (arg_storage == COREDUMP_STORAGE_JOURNAL &&
coredump_size <= arg_journal_size_max) {
size_t sz = 0;

146
src/coredump/coredumpctl.c
View file

@ -580,23 +580,21 @@ static int save_core(sd_journal *j, int fd, char **path, bool *unlink_temp) {
_cleanup_free_ char *filename = NULL;
size_t len;
int r;
_cleanup_close_ int fdt = -1;
char *temp = NULL;
assert((fd >= 0) != !!path);
assert(!!path == !!unlink_temp);
/* Prefer uncompressed file to journal (probably cached) to
* compressed file (probably uncached). */
/* Look for a coredump on disk first. */
r = sd_journal_get_data(j, "COREDUMP_FILENAME", (const void**) &data, &len);
if (r < 0 && r != -ENOENT)
log_warning_errno(r, "Failed to retrieve COREDUMP_FILENAME: %m");
return log_error_errno(r, "Failed to retrieve COREDUMP_FILENAME: %m");
else if (r == 0)
retrieve(data, len, "COREDUMP_FILENAME", &filename);
if (filename && access(filename, R_OK) < 0) {
log_full(errno == ENOENT ? LOG_DEBUG : LOG_WARNING,
"File %s is not readable: %m", filename);
filename = mfree(filename);
}
if (filename && access(filename, R_OK) < 0)
return log_error_errno(errno, "File \"%s\" is not readable: %m", filename);
if (filename && !endswith(filename, ".xz") && !endswith(filename, ".lz4")) {
if (path) {
@ -605,92 +603,86 @@ static int save_core(sd_journal *j, int fd, char **path, bool *unlink_temp) {
}
return 0;
} else {
_cleanup_close_ int fdt = -1;
char *temp = NULL;
}
if (fd < 0) {
const char *vt;
if (fd < 0) {
const char *vt;
r = var_tmp_dir(&vt);
if (r < 0)
return log_error_errno(r, "Failed to acquire temporary directory path: %m");
/* Create a temporary file to write the uncompressed core to. */
temp = strjoin(vt, "/coredump-XXXXXX", NULL);
if (!temp)
return log_oom();
r = var_tmp_dir(&vt);
if (r < 0)
return log_error_errno(r, "Failed to acquire temporary directory path: %m");
fdt = mkostemp_safe(temp);
if (fdt < 0)
return log_error_errno(fdt, "Failed to create temporary file: %m");
log_debug("Created temporary file %s", temp);
temp = strjoin(vt, "/coredump-XXXXXX", NULL);
if (!temp)
return log_oom();
fd = fdt;
fdt = mkostemp_safe(temp);
if (fdt < 0)
return log_error_errno(fdt, "Failed to create temporary file: %m");
log_debug("Created temporary file %s", temp);
fd = fdt;
}
if (filename) {
#if defined(HAVE_XZ) || defined(HAVE_LZ4)
_cleanup_close_ int fdf;
fdf = open(filename, O_RDONLY | O_CLOEXEC);
if (fdf < 0) {
r = log_error_errno(errno, "Failed to open %s: %m", filename);
goto error;
}
r = decompress_stream(filename, fdf, fd, -1);
if (r < 0) {
log_error_errno(r, "Failed to decompress %s: %m", filename);
goto error;
}
#else
log_error("Cannot decompress file. Compiled without compression support.");
r = -EOPNOTSUPP;
goto error;
#endif
} else {
ssize_t sz;
r = sd_journal_get_data(j, "COREDUMP", (const void**) &data, &len);
if (r == 0) {
ssize_t sz;
if (r < 0)
return log_error_errno(r,
r == -ENOENT ? "Core file was not saved for this entry." :
"Failed to retrieve COREDUMP field: %m");
assert(len >= 9);
data += 9;
len -= 9;
assert(len >= 9);
data += 9;
len -= 9;
sz = write(fdt, data, len);
if (sz < 0) {
r = log_error_errno(errno,
"Failed to write temporary file: %m");
goto error;
}
if (sz != (ssize_t) len) {
log_error("Short write to temporary file.");
r = -EIO;
goto error;
}
} else if (filename) {
#if defined(HAVE_XZ) || defined(HAVE_LZ4)
_cleanup_close_ int fdf;
fdf = open(filename, O_RDONLY | O_CLOEXEC);
if (fdf < 0) {
r = log_error_errno(errno,
"Failed to open %s: %m",
filename);
goto error;
}
r = decompress_stream(filename, fdf, fd, -1);
if (r < 0) {
log_error_errno(r, "Failed to decompress %s: %m", filename);
goto error;
}
#else
log_error("Cannot decompress file. Compiled without compression support.");
r = -EOPNOTSUPP;
goto error;
#endif
} else {
if (r == -ENOENT)
log_error("Cannot retrieve coredump from journal or disk.");
else
log_error_errno(r, "Failed to retrieve COREDUMP field: %m");
sz = write(fdt, data, len);
if (sz < 0) {
r = log_error_errno(errno, "Failed to write temporary file: %m");
goto error;
}
if (temp) {
*path = temp;
*unlink_temp = true;
if (sz != (ssize_t) len) {
log_error("Short write to temporary file.");
r = -EIO;
goto error;
}
}
return 0;
if (temp) {
*path = temp;
*unlink_temp = true;
}
return 0;
error:
if (temp) {
unlink(temp);
log_debug("Removed temporary file %s", temp);
}
return r;
if (temp) {
unlink(temp);
log_debug("Removed temporary file %s", temp);
}
return r;
}
static int dump_core(sd_journal* j) {