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

Jump to bottom

boot.initrd: add verbose option #108294

Merged
merged 1 commit into from Jan 29, 2021
Merged

boot.initrd: add verbose option #108294

merged 1 commit into from Jan 29, 2021

Conversation

GovanifY
Copy link
Member

@GovanifY GovanifY commented Jan 3, 2021 •
edited

Motivation for this change

This fixes #32555

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

lopsided98
lopsided98 suggested changes Jan 4, 2021
View reviewed changes
Copy link
Contributor

@lopsided98 lopsided98 left a comment

Choose a reason for hiding this comment

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

Have not tested, but looks good to me except for the one minor suggestion.

nixos/modules/system/boot/stage-1.nix Outdated Show resolved Hide resolved
@GovanifY GovanifY force-pushed the silent-boot branch 3 times, most recently from 9eaf61c to 2a924c5 Compare January 4, 2021 09:21
@GovanifY
Copy link
Member Author

GovanifY commented Jan 4, 2021

There seems to be an error on our cachix instance, hence why my test has failed. The failure is meaningless and we can probably merge as is.

@GovanifY
Copy link
Member Author

@lopsided98 @oxij @Mic92 Is there any news on the review of this patch?

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/450

@GovanifY GovanifY force-pushed the silent-boot branch 3 times, most recently from 3b1899c to 86106f0 Compare January 23, 2021 16:44
@Mic92
Copy link
Member

Mic92 commented Jan 23, 2021

@GrahamcOfBorg test grub

@SuperSandro2000
Copy link
Member

@GrahamcOfBorg test grub

aarch64 failed.

@AndersonTorres
Copy link
Member

Error log:

https://logs.nix.ci/?key=nixos/nixpkgs.108294&attempt_id=0523dd99-4357-487d-a159-8ff066243862

@GovanifY
Copy link
Member Author

I don't think the aarch64 failure is my fault in this case?

@AndersonTorres
Copy link
Member

Indeed the error log says something about not being able to find the hard disk (?).

@AndersonTorres
Copy link
Member

Now LGTBorg.

@kanashimia
Copy link
Member

Can't we just redirect output to /dev/kmsg ?
Like loglevels already exist, so why there should be a separate flag for this?

@GovanifY
Copy link
Member Author

@kanashimia If you have a better implementation idea go for it, but the patch has already been aproved and is ready for merge

@AndersonTorres
Copy link
Member

@kanashimia I will merge this PR. Please make a new PR with your suggestions.

@AndersonTorres AndersonTorres merged commit 7925661 into NixOS:master Jan 29, 2021
@peterhoeg
Copy link
Member

This is great, thank you! As for @kanashimia's comment, another option would be to read the quiet flag off /proc/cmdline and act based on that as it allows you to dynamically switch things on/off.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/removing-persistent-boot-messages-for-a-silent-boot/14835/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/removing-persistent-boot-messages-for-a-silent-boot/14835/3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndersonTorres AndersonTorres AndersonTorres approved these changes

@oxij oxij Awaiting requested review from oxij

@Mic92 Mic92 Awaiting requested review from Mic92

@lopsided98 lopsided98 Awaiting requested review from lopsided98

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add option for silent boot
8 participants
@GovanifY @nixos-discourse @Mic92 @SuperSandro2000 @AndersonTorres @kanashimia @peterhoeg @lopsided98