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

Make VM journal volatile by default #8832

Open
emanruse opened this issue Jan 5, 2024 · 15 comments
Open

Make VM journal volatile by default #8832

emanruse opened this issue Jan 5, 2024 · 15 comments
Labels
C: storage P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.

Comments

@emanruse
Copy link

emanruse commented Jan 5, 2024

The problem you're addressing (if any)

Considering the way Qubes OS works, journal persistence in client qubes makes no sense, as the volatile volume is volatile anyway. Tracking potential issues can still be done by observing the current boot journal, or explicitly, temporarily enabling journal persistence (in template or using bind-dirs). This is an advanced administrative task anyway, so it is not supposed to be done by users on a regular basis, i.e. won't hurt usability. Other distros (e.g. openSUSE) have volatile journal by default too.

The solution you'd like

In the template:

sed -ri 's/#Storage=.*/Storage=volatile/g' /etc/systemd/journald.conf

Reboot the template, then remove the old persistent journal from /var/log/journal.

The value to a user, and who that user might be

Moving journal to tmpfs would reduce unnecessary SSD writes.

@emanruse emanruse added P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality. labels Jan 5, 2024
@adrelanos
Copy link
Member

Implementation detail if implementing this:

  • don't use sed
  • drop appropriate snippet in /usr/lib/systemd folder somewhere

rationale:
https://www.kicksecure.com/wiki/Dev/About_Debian_Packaging#Modifying_Default_Configuration_of_Third_Party_Packages

@emanruse
Copy link
Author

emanruse commented Jan 7, 2024 via email

@adrelanos
Copy link
Member

Derivative distributions are not supposed to write into /etc if avoidable. That's for system administrators. They're even less so recommended to write directly into a configuration file owned by a different package as this can cause issues when the file is upgraded by either original distribution or derivative distribution. Also shows up under debsums -ce.

The appropriate journal configuration drop-in folder is:
/usr/lib/systemd/journald.conf.d/*.conf

https://www.freedesktop.org/software/systemd/man/latest/journald.conf.html

systemd has always been good in providing drop-in configuration folders to make customization simple.

I can almost guarantee if Qubes implements this, a drop-in will be used. That's why Qubes It makes no sense to debate that. That's what Qubes always did.

find /usr/lib/systemd | grep qubes

So if you want to send a pull request, I would suggest /usr/lib/systemd/journald.conf.d/30_qubes.conf as this seems having a good chance as it matches the style of the many other existing systemd related drop-in config snippets. Talking about the file name only. I don't know yet if Qubes wants to implement this.

@emanruse
Copy link
Author

emanruse commented Jan 8, 2024 via email

@marmarek
Copy link
Member

marmarek commented Jan 8, 2024

No, /etc is for user changes, packages should avoid modifying config files there of other packages if possible. Anyway, if this change is to be made, it's clear how it should be done (via a drop-in file in /usr). What isn't clear, is whether it's a good idea to change it at all. A better solution might be making journal survive qube restart, instead of breaking it even further.

@emanruse
Copy link
Author

emanruse commented Jan 8, 2024 via email

@marmarek
Copy link
Member

marmarek commented Jan 8, 2024

Is that what you mean?

No, see earlier comment by @adrelanos

@adrelanos
Copy link
Member

Perhaps journal should be persistent or non-persistent as per Qubes respecting upstream distribution culture, FAQ What is Qubes’ attitude toward changing guest distros?. If Debian has persistent journal by default, Qubes Debian would also have it. Respectively depending on what Fedora does etc.


minor: https://www.freedesktop.org/software/systemd/man/latest/journald.conf.html unfortunately does not seem to support ConditionPathExists which then could be used to set a different config if file /run/qubes/this-is-appvm exists. (That status file is already implemented for years.)

  • What would log persistence improve?

Log analysis in case of issues without complex instructions "1) make log persistent in Template, 2) reboot App Qube, 3) reproduce the issue, 4) check the log".

  • How many users actually look at logs?

Only advanced users and developers. I am a developer but I am also a user. By putting more obstacles into my way, the less productive I get.

For example, if a VM is slow to shut down or crashing, I need to look at the journal of the previous boot. (sudo journalctl -b -1)

But if extra steps are required then that takes extra time and extra mental load. Hence lowering productivity. Therefore, I am all for to make things comfortable for advanced users and developers to keep the ability to easily debug the system.

Will there be a way to exclude logs from backups?

I guess not. That seems complex to implement to have Qubes backup parse the contents of a VM.

  • Considering running many VMs means lots of logging, which of the 2 possibilities will be better for the default overall performance?

Performance will be negligible / non-issue / same as now because now all VMs are logging just that it gets lost after App Qube gets shut down.

@emanruse
Copy link
Author

emanruse commented Jan 9, 2024 via email

@adrelanos
Copy link
Member

Or maybe it is wiser to consider what Qube OS's specifics are and what defaults fit that.

I would also prefer if Qubes would ship hardened by default, customized forks of Debian, Fedora or similar. (#8730) But that is not the case (yet?) and might be related to:
https://www.qubes-os.org/faq/#what-is-qubes-attitude-toward-changing-guest-distros

but Qubes OS already writes custom configuration stuff in /etc.

This is probably historic reasons in case there is no alternative yet.

(I am writing "probably" to be extra careful to not speak for Qubes even though Marek already agreed with the configuration file location, I shouldn't assume to know the detailed reasoning behind it.)

Elaborated below.

No, /etc is for user changes, packages should avoid modifying config files of other packages if possible.
It is possible but the question is whether it is justified in the particular context/case.

Derivative distributions are not supposed to write into /etc if avoidable.
I don't know if Debian and Fedora templates are considered derivative distributions

They are in my view and that should be explicitly documented (elaborated in #8730) but that shouldn't matter. I can rephrase this a bit better without needing to mention "Derivative distributions".

Derivative distributions are not supposed to write into /etc if avoidable.

Better rewritten to:
There's a new push by systemd and some Linux distributions that they should use /usr whenever possible and avoid /etc if avoidable. I agree with this push and Qubes seems to agree with it through its actions, through its configuration file locations, and though Marek's comment here.

Why this push exists and references for that, documented just now:
https://www.kicksecure.com/wiki/Dev/About_Debian_Packaging#use_/usr_instead_of_/etc

Changing one line in a conffile should not be a problem.

It's a problem because this can later result in an interactive dpkg conflict resolution dialog. Why that is an issue is now documented here:
https://www.kicksecure.com/wiki/Dev/About_Debian_Packaging#Modifying_Default_Configuration_of_Third_Party_Packages

Without any actual statistical data, I can only remind that 90%+ of desktop users will generally abandon software which doesn't behave as they expect, rather than report bugs or analyze logs (or even know what a log is).

I will assume this is correct. Could easily be correct. In that case, it is important for the remaining 10% of users to make it as simple as reasonably possible to debug and report issues. Complex instructions "1) make log persistent in Template, 2) reboot App Qube, 3) reproduce the issue, 4) check the log" should be avoided.

@marmarek
Copy link
Member

FWIW the alternative (preferred?) way of persisting logs from app qubes (and others) is logvm (#830). But that, besides implementing the actual feature (which is rather simple, and there are few PoC already), requires consideration of memory usage - yet another always running qube is IMO too much with their current memory usage. We need to make them lighter first (for example #7159 ).

When logvm gets implemented and enabled (including the above lightening prerequisite), setting journal to volatile will be the obvious change to make. But before that, I don't think it's universally a good idea.

In both Fedora and Debian, the default is Storage=auto (from man page):
"auto" behaves like "persistent" if the /var/log/journal directory exists, and "volatile" otherwise (the existence of the directory controls the storage mode).

At least in Fedora, the /var/log/journal directory is owned by the systemd package. So, removing it by default isn't really an option.

Complex instructions "1) make log persistent in Template, 2) reboot App Qube, 3) reproduce the issue, 4) check the log" should be avoided.

With volatile journal, it can still be accessed until qube shutdown. Volatile means it's stored in memory only (AFAIR somewhere in /run), not discarded completely. There is likely a limited amount stored (few MBs?) but should be still good enough to get logs of something that just happened. The real behavior change (compared to the current situation) if setting volatile journal applies to the template itself and standalones only. But then, it might be useful to get logs from previous app qube startup (for example to get more details why it crashed) - which currently is not possible. Having persistent journal (via bind-dirs?) would solve this. But also probably should also be limited in size (the default is quite high IMO - 10% of the disk, which would be 200MB here in default config).

There's a new push by systemd and some Linux distributions that they should use /usr whenever possible and avoid /etc if avoidable. I agree with this push and Qubes seems to agree with it through its actions, through its configuration file locations, and though Marek's comment here.

Yes, I agree packaged configuration (modifications) should be kept out of /etc, to not conflict with user modifications. Having both package and the user modify the same configuration file is complex and error-prone. It doesn't matter if that's derivative distribution or not - it applies to upstream distributions to some extend too (some go even further - openSUSE has a goal to have fully functional system running default configuration with no packaged configuration in /etc, so anything in /etc is in fact user customization).

Sadly, not all applications support config drop-ins yet. And some do not support multiple configuration files at all (the only one is in /etc or user home dir). For those cases, we still need to handle it via editing config in place (or, sometimes preferably, avoid config change at all and consider alternatives).

@adrelanos
Copy link
Member

Complex instructions "1) make log persistent in Template, 2) reboot App Qube, 3) reproduce the issue, 4) check the log" should be avoided.

I meant to debug a VM crash which requires the log of the previous boot.

But also probably should also be limited in size (the default is quite high IMO - 10% of the disk, which would be 200MB here in default config).

The number of past reboots could be limited. 1, 2 or 3 prior boots seem reasonable numbers.

Having persistent journal (via bind-dirs?) would solve this.

Yes.

@emanruse
Copy link
Author

emanruse commented Jan 12, 2024 via email

@marmarek
Copy link
Member

How about linking it to /dev/null?

That still means overriding a file/directory owned by another package. It will cause issues on updates.

Wouldn't a crash of a qube be logged into dom0 anyway?

You will see it crashed. Usually you will not see why it crashed.

Anyway, I must admit that relatively low (as in - almost non-existent) number of cases when having such logging enabled by default would be necessary for debugging some issue is rather compelling argument to not enable journal persistence by default. It's needed only if:

  • qube fully crashed (not just some application in it)
  • issue cannot be reliably reproduced (so, cannot be reproduced after enabling more logging and/or journal persistence)

FWIW, I personally use a different method to keep qube's journal persistently - enable logging to the console (ForwardToConsole=yes in journald.conf, or systemd.journald.forward_to_console in kernelopts). Then logs can be found in /var/log/xen/console in dom0. This preserve logs also for disposable qubes, which is useful for debugging, but undesired for normal operation.

Maybe some middle ground would be:

  • disable writing journal to disk by default (at least in app and disposable qubes)
  • enable persistent journal when "debug mode" is enabled (checkbox in qube settings -> advanced tab); do it via bind-dirs, so logs can be browsed using standard journalctl tool

@emanruse
Copy link
Author

emanruse commented Jan 12, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: storage P: default Priority: default. Default priority for new issues, to be replaced given sufficient information. T: enhancement Type: enhancement. A new feature that does not yet exist or improvement of existing functionality.
Projects
None yet
Development

No branches or pull requests

4 participants