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

Force systems with kernel option "quiet" to display TTY password prompt #9731

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

ghfields
Copy link
Contributor

@ghfields ghfields commented Dec 16, 2019

Motivation and Context

On systems that utilize TTY for password entry, if the kernel option "quiet"
is set, the system would appear to freeze on a blank screen, when in fact it is waiting for
password entry from the user.

Since TTY is the fallback method, this has no effect on systemd or plymouth password prompting

Description

By temporarily setting "printk" to "7", running the command, then
resuming with the original "printk" state, the user can see the password prompt.

How Has This Been Tested?

Prompt now successfully displays on my system (Ubuntu 20.04 Daily) with "quiet" on kernel line

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Dec 17, 2019
@rlaager
Copy link
Member

rlaager commented Dec 17, 2019

@ghfields Can you retry with the changes from #9723 and see if that fixes it?

@rlaager rlaager self-assigned this Dec 17, 2019
@ghfields
Copy link
Contributor Author

@ghfields Can you retry with the changes from #9723 and see if that fixes it?

The #9723 fixes allow certain systems (Ubuntu) to avoid needing to go to TTY prompting. This PR, however fixes systems that do utilize TTY and have "quiet" has the kernel option set. With #9723, there are fewer systems, but doesn't eliminate all TTY use cases.

contrib/initramfs/scripts/zfs.in Outdated Show resolved Hide resolved

# If root dataset is encrypted...
if ! [ "${ENCRYPTIONROOT}" = "-" ]; then
KEYSTATUS="$(${ZFS} get -H -o value keystatus "${ENCRYPTIONROOT}")"
KEYSTATUS="$(get_fs_value "${ENCRYPTIONROOT}" keystatus)"
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I'd prefer these be in separate commits (one PR seems fine), since they're unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, was back and forth about if they were trivial enough with myself. Additional PR inbound shortly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Became #9736

@rlaager
Copy link
Member

rlaager commented Dec 17, 2019

Other than eliminating the unnecessary cat, this looks good to me.

Signed-off-by: Garrett Fields <ghfields@gmail.com>
@codecov
Copy link

codecov bot commented Dec 17, 2019

Codecov Report

Merging #9731 into master will increase coverage by 13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9731      +/-   ##
==========================================
+ Coverage      67%      79%     +13%     
==========================================
  Files         338      420      +82     
  Lines      106419   123654   +17235     
==========================================
+ Hits        71086    98083   +26997     
+ Misses      35333    25571    -9762
Flag Coverage Δ
#kernel 80% <ø> (?)
#user 67% <ø> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ad97643...a7adede. Read the comment docs.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Dec 17, 2019
@behlendorf behlendorf merged commit e8b199f into openzfs:master Dec 18, 2019
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 26, 2019
On systems that utilize TTY for password entry, if the kernel
option "quiet" is set, the system would appear to freeze on a
blank screen, when in fact it is waiting for password entry
from the user.

Since TTY is the fallback method, this has no effect on systemd
or plymouth password prompting.

By temporarily setting "printk" to "7", running the command,
then resuming with the original "printk" state, the user can
see the password prompt.

Reviewed-by: Richard Laager <rlaager@wiktel.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Garrett Fields <ghfields@gmail.com>
Closes openzfs#9731
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Dec 27, 2019
On systems that utilize TTY for password entry, if the kernel
option "quiet" is set, the system would appear to freeze on a
blank screen, when in fact it is waiting for password entry
from the user.

Since TTY is the fallback method, this has no effect on systemd
or plymouth password prompting.

By temporarily setting "printk" to "7", running the command,
then resuming with the original "printk" state, the user can
see the password prompt.

Reviewed-by: Richard Laager <rlaager@wiktel.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Garrett Fields <ghfields@gmail.com>
Closes openzfs#9731
tonyhutter pushed a commit that referenced this pull request Jan 23, 2020
On systems that utilize TTY for password entry, if the kernel
option "quiet" is set, the system would appear to freeze on a
blank screen, when in fact it is waiting for password entry
from the user.

Since TTY is the fallback method, this has no effect on systemd
or plymouth password prompting.

By temporarily setting "printk" to "7", running the command,
then resuming with the original "printk" state, the user can
see the password prompt.

Reviewed-by: Richard Laager <rlaager@wiktel.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Garrett Fields <ghfields@gmail.com>
Closes #9731
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants