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

Minimalistic Logging Support #112

Merged
merged 3 commits into from
Feb 25, 2023
Merged

Minimalistic Logging Support #112

merged 3 commits into from
Feb 25, 2023

Conversation

blitz
Copy link
Member

@blitz blitz commented Feb 21, 2023

This PR is an alternative to #87. The end result is the same: When everything goes well, lanzaboote gives no output (fixing #84).

The approach is different in that we don't allow any runtime modification of logging level. The release build will only print warnings and errors. I think too much configurability is not really helping and just introduces complexity which we can avoid.

Closes #84.

@dasJ
Copy link
Member

dasJ commented Feb 21, 2023

Things you could pick from the original PR (or we create a new PR for them):

@nikstur
Copy link
Collaborator

nikstur commented Feb 21, 2023

Things you could pick from the original PR (or we create a new PR for them):

I think a separate PR makes more sense.

@RaitoBezarius
Copy link
Member

While I agree that added complexity is not desirable, moving forward with lanzaboote will expose it to various users and we will definitely encounter the need for runtime log level changes IMHO because users will not have an easy way to "enable a debug build" of lzbt in their state. So I am okay with this change as-is, because it's a move in the right direction (logging rather than no logging), but I am not satisfied with "no runtime modification".

I don't know when is the right moment to introduce it, so if you have a plan @blitz or if you think we need to have a formal discussion on it and track this feature, let's do it. :)

@blitz
Copy link
Member Author

blitz commented Feb 23, 2023

we will definitely encounter the need for runtime log level changes

I'm not so sure. But time will tell. :)

So I am okay with this change as-is, because it's a move in the right direction (logging rather than no logging), but I am not satisfied with "no runtime modification".

Fair enough. Let's cross this bridge when we get there.

@blitz
Copy link
Member Author

blitz commented Feb 23, 2023

@dasJ I've cherry picked 07ebc47. The error handling change is better done in a separate PR.

@blitz blitz mentioned this pull request Feb 23, 2023
Copy link
Collaborator

@nikstur nikstur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nikstur
Copy link
Collaborator

nikstur commented Feb 23, 2023

we will definitely encounter the need for runtime log level changes IMHO because users will not have an easy way to "enable a debug build" of lzbt in their state.

Just to make sure: are you speaking about the stub or the tool?

I think the tool definetely needs configurable logging at some point but the stub does not. Does systemd-stub have configurable logging? From what I can tell it doesn't.

@RaitoBezarius
Copy link
Member

we will definitely encounter the need for runtime log level changes IMHO because users will not have an easy way to "enable a debug build" of lzbt in their state.

Just to make sure: are you speaking about the stub or the tool?

The stub.

I think the tool definetely needs configurable logging at some point but the stub does not. Does systemd-stub have configurable logging? From what I can tell it doesn't.

It does not have, but also, there are issues such as systemd/systemd#25911 where I do think configurable debugging will make easier bug reports.

Anyway, I am fine with revisiting this once we hit enough usage that it becomes useful.

@blitz blitz merged commit a5e283c into master Feb 25, 2023
@blitz blitz deleted the log branch February 25, 2023 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support flicker-free boot
4 participants