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

Remove instances of KernelVersion::current().unwrap() #1024

Open
alessandrod opened this issue Sep 9, 2024 · 2 comments · May be fixed by #1042
Open

Remove instances of KernelVersion::current().unwrap() #1024

alessandrod opened this issue Sep 9, 2024 · 2 comments · May be fixed by #1042
Assignees
Labels
good first issue Good for newcomers

Comments

@alessandrod
Copy link
Collaborator

In a bunch of places we do if KernelVersion::current().unwrap() >= KernelVersion::new(....) {

In theory, we should always know the kernel version so the unwrap is understandable. However it seems that distributions really like to mess with how the kernel version is reported and we've had this unwrap fail in more than one case (some version of ubuntu and recently proxmox).

In the cases where we have if version { ... } else { ... } we should handle the case where current() fails, log a warning, and probably do the else thing which is usually whatever is supported by older kernels.

@alessandrod alessandrod added the good first issue Good for newcomers label Sep 9, 2024
@pythops
Copy link

pythops commented Sep 27, 2024

I have started working on this one.
What do you mean by log a warning ? would eprintln! with a message work or should we include tracing crate ?

@alessandrod
Copy link
Collaborator Author

I have started working on this one.

Awesome thanks!

What do you mean by log a warning ? would eprintln! with a message work or should we include tracing crate ?

We use the log crate, so you can

use log::warn;

warn!("some warning message");

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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants