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

Use log instead of println!/eprintln! #79

Open
andreeaflorescu opened this issue Feb 22, 2021 · 7 comments
Open

Use log instead of println!/eprintln! #79

andreeaflorescu opened this issue Feb 22, 2021 · 7 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@andreeaflorescu
Copy link
Member

Import the log crate and use it to log needed information.

For now it should be enough to just replace the existing println! or eprintln!.

@andreeaflorescu andreeaflorescu added good first issue Good for newcomers help wanted Extra attention is needed labels Feb 22, 2021
@tumbleshack
Copy link

tumbleshack commented Feb 26, 2021

Hey!

My friend @seanmichwalsh and I are looking to learn about VMM's and Rust. Mind if we take on this issue?

It may take us a bit to learn our way around if that's okay with you.

@andreeaflorescu
Copy link
Member Author

catalindumitru pushed a commit to catalindumitru/vmm-reference that referenced this issue Sep 1, 2021
8901e77 Leftovers from rust-vmm#79
555474a Add script to run tests locally
cd3d97f Allow skipping tests when generating the pipeline
cc6ed99 Format integration tests code
ba2f45c make the test description configurable
877d1fb Update README.md
b47488a Autogenerate pipeline fron json config file

Signed-off-by: Catalin Dumitru <[email protected]>
andreeaflorescu pushed a commit that referenced this issue Sep 1, 2021
8901e77 Leftovers from #79
555474a Add script to run tests locally
cd3d97f Allow skipping tests when generating the pipeline
cc6ed99 Format integration tests code
ba2f45c make the test description configurable
877d1fb Update README.md
b47488a Autogenerate pipeline fron json config file

Signed-off-by: Catalin Dumitru <[email protected]>
@dodsonmg
Copy link
Contributor

Is this still open? I'm happy to pick up where #114 left off...

@andreeaflorescu
Copy link
Member Author

@dodsonmg yes, this is still opened. We should though reduce the scope for the beginning so that's easier to get it merged. We can do this initially for the main.rs, and the vmm and api crates.

@dodsonmg
Copy link
Contributor

Cool. Thanks @andreeaflorescu. I'll pick it up.

@dodsonmg
Copy link
Contributor

Do you want the log level and log file to be command line arguments, or do you just want default log level settings and log output to go to stderr?

If the former, the log level and file can't be established until we've processed command line arguments, and we'll miss some of the println! and eprintln! statements that come before then. This is how cloud hypervisor does it.

If the latter, then I think we can just initialise the logger first and sweep everything up.

@andreeaflorescu
Copy link
Member Author

I think how it worked in Firecracker is that we were using println and eprintln until the logger is initialized, and error info afterwards. This way you don't loose any logs because you're redirecting the output to a file anyway. I think we could start with something simple where we assume stdout and stderr because I believe these can also just directly be forwarded to files without us doing anything special in the VMM code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants