-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Suppress unimplemented MSR related warnings #1611
base: main
Are you sure you want to change the base?
Conversation
@@ -96,6 +97,7 @@ bhyve_usage(int code) | |||
#ifdef BHYVE_SNAPSHOT | |||
" -r: path to checkpoint file\n" | |||
#endif | |||
" -q: disable verbose MSR print out\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably not add a new top-level option as there are already several, and -q is pretty generic. I think having it just settable via '-o' is fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the feedback, i made it settable via o instead now, and added the corresponding entries in the man pages
@@ -63,6 +63,7 @@ bhyve_init_config(void) | |||
set_config_bool("acpi_tables_in_memory", true); | |||
set_config_value("memory.size", "256M"); | |||
set_config_bool("x86.strictmsr", true); | |||
set_config_bool("x86.verbosemsr", true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can actually default it to false, only developers probably care and for users it is just annoying spam.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this to false now
usr.sbin/bhyve/bhyve.8
Outdated
@@ -295,6 +295,13 @@ To map a 4 vCPU guest to host CPUs 12-15: | |||
.Bd -literal | |||
-p 0:12 -p 1:13 -p 2:14 -p 3:15 | |||
.Ed | |||
.It Fl q |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't do -q, we should still document this in bhyve_config.5 which describes all the config nodes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated this as well!
Tested with and without the |
The change looks good to me. Can you rebase it and collapse it down to a single commit with an updated log message? |
Thanks for the review! I rebased and squashed those commits, but it looks like the jenkins build failed with "Monthly compute limit exceeded error". Can we still merge or should I kick off the build again somehow |
When using bhyve on x86, rdmsr and wrmsr emits a lot of warnings when dealing with unimplemented MSR. An option x86.verbosemsr is created to control these warnings. By default, the MSR related warnings are suppressed to avoid spamming the console. Sponsored by: Netflix
While using bhyve on intel machines, if certain MSRs are unimplemented, the bhyve console is spammed with these messages:
This does not interfere with the normal operation of bhyve (since I can run the image), and there are already options to ignore unimplmented MSRs and continue (
-w
) But the warning messages makes it difficult to use the console. A new option-q
is created to suppress therdmsr
andwrmsr
messages.