dasharo/systemd
0
0
Fork 0
mirror of https://github.com/Dasharo/systemd.git synced 2026-03-06 15:02:31 -08:00
Code Issues Packages Projects Releases Wiki Activity

journal-file: require MMapCache* for journal_file_open()

Browse Source 
Previously the MMapCache* was optionally NULL, which open would
handle by creating a new MMapCache* for the occasion.

This produced some slightly circuitous refcount-handling code in
the function, as well as arguably creating opportunities for
weirdness where an MMapCache* was intended to be supplied but
happened to be NULL, which this magic would then paper over.

In any case, this was basically only being utilized by tests,
apparently just to avoid having to create an MMapCache.  So
update the relevant tests to supply an MMapCache and make
journal_file_open() treat a NULL MMapCache* as fatal w/assert.
This commit is contained in:
Vito Caputo
2021-12-07 14:18:14 -08:00
parent 333d067262
commit 74fb5be62d
6 changed files with 58 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/journal/test-journal-flush.c
View File

@@ -14,6 +14,7 @@
#include "string-util.h"
int main(int argc, char *argv[]) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
_cleanup_free_ char *fn = NULL;
char dn[] = "/var/tmp/test-journal-flush.XXXXXX";
JournaldFile *new_journal = NULL;
@@ -21,12 +22,14 @@ int main(int argc, char *argv[]) {
unsigned n = 0;
int r;
m = mmap_cache_new();
assert_se(m != NULL);
assert_se(mkdtemp(dn));
(void) chattr_path(dn, FS_NOCOW_FL, FS_NOCOW_FL, NULL);
fn = path_join(dn, "test.journal");
r = journald_file_open(-1, fn, O_CREAT|O_RDWR, 0644, false, 0, false, NULL, NULL, NULL, NULL, &new_journal);
r = journald_file_open(-1, fn, O_CREAT|O_RDWR, 0644, false, 0, false, NULL, m, NULL, NULL, &new_journal);
assert_se(r >= 0);
if (argc > 1)

17
src/journal/test-journal-interleaving.c
View File

@@ -34,8 +34,13 @@ _noreturn_ static void log_assert_errno(const char *text, int error, const char
} while (false)
static JournaldFile *test_open(const char *name) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
JournaldFile *f;
assert_ret(journald_file_open(-1, name, O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &f));
m = mmap_cache_new();
assert_se(m != NULL);
assert_ret(journald_file_open(-1, name, O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f));
return f;
}
@@ -198,15 +203,19 @@ static void test_skip(void (*setup)(void)) {
static void test_sequence_numbers(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
char t[] = "/var/tmp/journal-seq-XXXXXX";
JournaldFile *one, *two;
uint64_t seqnum = 0;
sd_id128_t seqnum_id;
m = mmap_cache_new();
assert_se(m != NULL);
mkdtemp_chdir_chattr(t);
assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0644,
true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &one) == 0);
true, UINT64_MAX, false, NULL, m, NULL, NULL, &one) == 0);
append_number(one, 1, &seqnum);
printf("seqnum=%"PRIu64"\n", seqnum);
@@ -223,7 +232,7 @@ static void test_sequence_numbers(void) {
memcpy(&seqnum_id, &one->file->header->seqnum_id, sizeof(sd_id128_t));
assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0644,
true, UINT64_MAX, false, NULL, NULL, NULL, one, &two) == 0);
true, UINT64_MAX, false, NULL, m, NULL, one, &two) == 0);
assert_se(two->file->header->state == STATE_ONLINE);
assert_se(!sd_id128_equal(two->file->header->file_id, one->file->header->file_id));
@@ -254,7 +263,7 @@ static void test_sequence_numbers(void) {
seqnum = 0;
assert_se(journald_file_open(-1, "two.journal", O_RDWR, 0,
true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &two) == 0);
true, UINT64_MAX, false, NULL, m, NULL, NULL, &two) == 0);
assert_se(sd_id128_equal(two->file->header->seqnum_id, seqnum_id));

10
src/journal/test-journal-stream.c
View File

@@ -60,6 +60,7 @@ static void verify_contents(sd_journal *j, unsigned skip) {
}
static void run_test(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
JournaldFile *one, *two, *three;
char t[] = "/var/tmp/journal-stream-XXXXXX";
unsigned i;
@@ -69,13 +70,16 @@ static void run_test(void) {
size_t l;
dual_timestamp previous_ts = DUAL_TIMESTAMP_NULL;
m = mmap_cache_new();
assert_se(m != NULL);
assert_se(mkdtemp(t));
assert_se(chdir(t) >= 0);
(void) chattr_path(t, FS_NOCOW_FL, FS_NOCOW_FL, NULL);
assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &one) == 0);
assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &two) == 0);
assert_se(journald_file_open(-1, "three.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &three) == 0);
assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &one) == 0);
assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &two) == 0);
assert_se(journald_file_open(-1, "three.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &three) == 0);
for (i = 0; i < N_ENTRIES; i++) {
char *p, *q;

15
src/journal/test-journal-verify.c
View File

@@ -10,6 +10,7 @@
#include "journald-file.h"
#include "journal-verify.h"
#include "log.h"
#include "mmap-cache.h"
#include "rm-rf.h"
#include "terminal-util.h"
#include "tests.h"
@@ -38,10 +39,14 @@ static void bit_toggle(const char *fn, uint64_t p) {
}
static int raw_verify(const char *fn, const char *verification_key) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
JournalFile *f;
int r;
r = journal_file_open(-1, fn, O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, NULL, NULL, &f);
m = mmap_cache_new();
assert_se(m != NULL);
r = journal_file_open(-1, fn, O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, &f);
if (r < 0)
return r;
@@ -52,6 +57,7 @@ static int raw_verify(const char *fn, const char *verification_key) {
}
int main(int argc, char *argv[]) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
char t[] = "/var/tmp/journal-XXXXXX";
unsigned n;
JournalFile *f;
@@ -61,6 +67,9 @@ int main(int argc, char *argv[]) {
struct stat st;
uint64_t p;
m = mmap_cache_new();
assert_se(m != NULL);
/* journald_file_open requires a valid machine id */
if (access("/etc/machine-id", F_OK) != 0)
return log_tests_skipped("/etc/machine-id not found");
@@ -73,7 +82,7 @@ int main(int argc, char *argv[]) {
log_info("Generating...");
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, !!verification_key, NULL, NULL, NULL, NULL, &df) == 0);
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, NULL, &df) == 0);
for (n = 0; n < N_ENTRIES; n++) {
struct iovec iovec;
@@ -95,7 +104,7 @@ int main(int argc, char *argv[]) {
log_info("Verifying...");
assert_se(journal_file_open(-1, "test.journal", O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, NULL, NULL, &f) == 0);
assert_se(journal_file_open(-1, "test.journal", O_RDONLY, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, &f) == 0);
/* journal_file_print_header(f); */
journal_file_dump(f);

28
src/journal/test-journal.c
View File

@@ -24,6 +24,7 @@ static void mkdtemp_chdir_chattr(char *path) {
}
static void test_non_empty(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
dual_timestamp ts;
JournaldFile *f;
struct iovec iovec;
@@ -35,9 +36,12 @@ static void test_non_empty(void) {
test_setup_logging(LOG_DEBUG);
m = mmap_cache_new();
assert_se(m != NULL);
mkdtemp_chdir_chattr(t);
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, NULL, NULL, NULL, &f) == 0);
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f) == 0);
assert_se(dual_timestamp_get(&ts));
assert_se(sd_id128_randomize(&fake_boot_id) == 0);
@@ -98,8 +102,8 @@ static void test_non_empty(void) {
assert_se(journal_file_move_to_entry_by_seqnum(f->file, 10, DIRECTION_DOWN, &o, NULL) == 0);
journald_file_rotate(&f, NULL, true, UINT64_MAX, true, NULL);
journald_file_rotate(&f, NULL, true, UINT64_MAX, true, NULL);
journald_file_rotate(&f, m, true, UINT64_MAX, true, NULL);
journald_file_rotate(&f, m, true, UINT64_MAX, true, NULL);
(void) journald_file_close(f);
@@ -117,17 +121,21 @@ static void test_non_empty(void) {
}
static void test_empty(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
JournaldFile *f1, *f2, *f3, *f4;
char t[] = "/var/tmp/journal-XXXXXX";
test_setup_logging(LOG_DEBUG);
m = mmap_cache_new();
assert_se(m != NULL);
mkdtemp_chdir_chattr(t);
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, false, NULL, NULL, NULL, NULL, &f1) == 0);
assert_se(journald_file_open(-1, "test-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, NULL, NULL, NULL, &f2) == 0);
assert_se(journald_file_open(-1, "test-seal.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, true, NULL, NULL, NULL, NULL, &f3) == 0);
assert_se(journald_file_open(-1, "test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, NULL, NULL, NULL, &f4) == 0);
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, false, NULL, m, NULL, NULL, &f1) == 0);
assert_se(journald_file_open(-1, "test-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f2) == 0);
assert_se(journald_file_open(-1, "test-seal.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, true, NULL, m, NULL, NULL, &f3) == 0);
assert_se(journald_file_open(-1, "test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f4) == 0);
journal_file_print_header(f1->file);
puts("");
@@ -156,6 +164,7 @@ static void test_empty(void) {
#if HAVE_COMPRESSION
static bool check_compressed(uint64_t compress_threshold, uint64_t data_size) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
dual_timestamp ts;
JournaldFile *f;
struct iovec iovec;
@@ -170,9 +179,12 @@ static bool check_compressed(uint64_t compress_threshold, uint64_t data_size) {
test_setup_logging(LOG_DEBUG);
m = mmap_cache_new();
assert_se(m != NULL);
mkdtemp_chdir_chattr(t);
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, compress_threshold, true, NULL, NULL, NULL, NULL, &f) == 0);
assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, compress_threshold, true, NULL, m, NULL, NULL, &f) == 0);
dual_timestamp_get(&ts);

17
src/libsystemd/sd-journal/journal-file.c
View File

@@ -3250,13 +3250,13 @@ int journal_file_open(
JournalFile **ret) {
bool newly_created = false;
MMapCache *m = mmap_cache;
JournalFile *f;
void *h;
int r;
assert(ret);
assert(fd >= 0 || fname);
assert(mmap_cache);
if (!IN_SET((flags & O_ACCMODE), O_RDONLY, O_RDWR))
return -EINVAL;
@@ -3319,14 +3319,6 @@ int journal_file_open(
}
}
if (!m) {
m = mmap_cache_new();
if (!m) {
r = -ENOMEM;
goto fail;
}
}
if (fname) {
f->path = strdup(fname);
if (!f->path) {
@@ -3368,17 +3360,12 @@ int journal_file_open(
goto fail;
}
/* On success this incs refcnt on *m, which mmap_cache_fd_free() will dec. */
f->cache_fd = mmap_cache_add_fd(m, f->fd, prot_from_flags(flags));
f->cache_fd = mmap_cache_add_fd(mmap_cache, f->fd, prot_from_flags(flags));
if (!f->cache_fd) {
r = -ENOMEM;
goto fail;
}
/* If we created *m just for this file, unref *m so only f->cache_fd's ref remains */
if (!mmap_cache)
(void) mmap_cache_unref(m);
r = journal_file_fstat(f);
if (r < 0)
goto fail;