Skip to content

Better logging #306

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

Closed
jasoncouture opened this issue Jan 3, 2023 · 5 comments · Fixed by #314
Closed

Better logging #306

jasoncouture opened this issue Jan 3, 2023 · 5 comments · Fixed by #314
Labels
enhancement New feature or request

Comments

@jasoncouture
Copy link
Contributor

The UEFI and bios loggers use only the framebuffer.

This can be an issue during headless runs (server with no display, unit tests)

I propose we refactor the logger to support multiple methods of logging output. (Serial being the first idea), with support in the kernel configuration to enable/disable sources. Or a text config file (this is likely better, so it can be configured at runtime)

This would also allow us to separate the logging and framebuffer init, as well be able to add/remove loggers at will. Rather than using writeln for early logging, we will be able to initialize the logger before anything else, and configure it with the optional config.txt or a default config that matches the current behavior.

@phil-opp
Copy link
Member

phil-opp commented Jan 3, 2023

Good idea! I fully agree that optional serial logging would be useful.

Or a text config file (this is likely better, so it can be configured at runtime)

You mean a separate text file that is placed on the FAT partition? I guess we could create such a file in the create_disk_image function. The question is how we split the options into kernel and runtime options. Candidates for runtime options could be the recently merged #303 and also the framebuffer size requirements.

@phil-opp phil-opp added the enhancement New feature or request label Jan 3, 2023
@jasoncouture
Copy link
Contributor Author

Would it be good to define runtime options, and have the config file on disk override the kernel? That way, the kernel could specify defaults (For things like frambuffer, logging, etc), and the end user could override via config text file.

I'm thinking something along the lines of an INI style file, where missing configuration options would defer to the kernel config.

@phil-opp
Copy link
Member

phil-opp commented Jan 4, 2023

Sounds reasonable to me! How about we start with a kernel config option for configuring the logger and then do the runtime config file separately?

@asensio-project
Copy link
Contributor

I am working in this and I think that we should move the code of the framebuffer to common/src/framebuffer.rs and the code related to the logger to common/src/logger.rs.

This will be alright for then adding the serial being and modifying the code of the logger for including this method.

Do you like this aproach or do you have another idea?

@asensio-project asensio-project mentioned this issue Jan 5, 2023
3 tasks
@phil-opp
Copy link
Member

phil-opp commented Jan 5, 2023

Thanks for tackling this!

I think that we should move the code of the framebuffer to common/src/framebuffer.rs and the code related to the logger to common/src/logger.rs.

This will be alright for then adding the serial being and modifying the code of the logger for including this method.

Sounds good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants