Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

journal: Set +C if we create /var/log/journal #32574

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DaanDeMeyer
Copy link
Contributor

@DaanDeMeyer DaanDeMeyer commented Apr 30, 2024

systemd-journal-flush.service runs before systemd-tmpfiles-setup.service so we can't rely on tmpfiles setting +C for us, so let's make journald set it itself if it creates /var/log/journal itself.

@github-actions github-actions bot added journal please-review PR is ready for (re-)review by a maintainer labels Apr 30, 2024
@bluca bluca added good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed please-review PR is ready for (re-)review by a maintainer labels Apr 30, 2024
systemd-journal-flush.service runs before systemd-tmpfiles-setup.service
so we can't rely on tmpfiles setting +C for us, so let's make journald
set it itself if it creates /var/log/journal itself.
@DaanDeMeyer DaanDeMeyer added please-review PR is ready for (re-)review by a maintainer and removed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Apr 30, 2024
Copy link

Important

An -rc1 tag has been created and a release is being prepared, so please note that PRs introducing new features and APIs will be held back until the new version has been released.

@poettering
Copy link
Member

hmm, this is quite problematic, since people might want to turn this off, which is via tmpfiles, but hard if encoded in C.

the fix seems rather to run the flush code after tmpfiles?

@DaanDeMeyer
Copy link
Contributor Author

@poettering You seem to have different thoughts according to 74055aa

journalctl: add new --flush command and make use of it in systemd-journal-flush.service

This new command will ask the journal daemon to flush all log data
stored in /run to /var, and wait for it to complete. This is useful, so
that in case of Storage=persistent we can order systemd-tmpfiles-setup
afterwards, to ensure any possibly newly created directory in /var/log
gets proper access mode and owners.

@poettering
Copy link
Member

hmm, i think we have a problem and can't really apply it before and after, can we?

hmpf. fucking btrfs.

it could be so easy if nocow was something one could turn off/on later.

btw, i think there's an issue somewhere about this already, we should probably link this up here.

I guess we should just bite the bullet and either add a journald.conf knob for this, or an env var that journald reads, to give people control, and then simply hack this up in C code, and drop the nocow flag handling in tmpfiles.

@DaanDeMeyer DaanDeMeyer added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks and removed please-review PR is ready for (re-)review by a maintainer labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
journal reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks
Development

Successfully merging this pull request may close these issues.

None yet

4 participants