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

recovery shell: ESP mount helper and tab completion #558

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

zdykstra
Copy link
Member

@zdykstra zdykstra commented Dec 17, 2023

Add a mount_esp function that mounts /dev/sdxY at /mnt/sdxY if nothing else is mounted there. If the device is already mounted, return the mountpoint. The shell completion helper only completes unmounted ESP devices.

Unmounting the device is left as an exercise to the user. I'm interested in a reasonable argument for handling this when exiting the recovery shell, like we do for ZFS datasets.

The mount operation is possibly going to be a problem. I'm trapping errors and dumping them to the console, but that's not super graceful.

Edit:

Thinking about this more, I should log all errors via zerror and then add something to the recovery shell prompt that evaluates if has_errors is set - if it is, dump the errors to the console and then clear the flag. This lets internal functions still Do The Right Thing if they're called by a program, but still have a mechanism for exposing their error text to a human operator if needed.

@zdykstra zdykstra force-pushed the extra-sensory-perception branch from a8d355f to ce3225c Compare December 17, 2023 01:38
@ahesford
Copy link
Member

ahesford commented Dec 17, 2023

I think this belongs somewhere other than -core. That file provides (or should provide) critical functions used in the primary control flow of ZBM. It seems to me this should be a standalone bin script. If there is an argument against that, maybe adding a new -helpers or -utils library that collects this and a few other user-exposed functions might be good.

At some point, it would be good to audit the ZBM library and relocate other functions as appropriate.

Having the recovery shell dump errors right to the console, maybe colored red, would be an awesome feature.

@zdykstra
Copy link
Member Author

I linked this on IRC, but I'll add it here so that it's more accessible to the discussion at hand:

Functions in zfsbootmenu-core.sh and their callers

Function: decolorize
      3 lib/zfsbootmenu-core.sh
Function: colorize
      2 bin/zbm
      6 bin/zfs-chroot
      2 bin/zfsbootmenu
     42 lib/zfsbootmenu-core.sh
      8 lib/zfsbootmenu-ui.sh
      2 libexec/zfsbootmenu-diff
      1 libexec/zfsbootmenu-help
      5 libexec/zfsbootmenu-init
      5 libexec/zfsbootmenu-preview
Function: center_string
      1 lib/zfsbootmenu-core.sh
      2 lib/zfsbootmenu-ui.sh
      1 libexec/zfsbootmenu-diff
      3 libexec/zfsbootmenu-preview
Function: write_hostid
      2 lib/zfsbootmenu-core.sh
      4 libexec/zfsbootmenu-init
Function: get_spl_hostid
      3 lib/zfsbootmenu-core.sh
Function: match_hostid
      1 lib/zfsbootmenu-core.sh
      3 libexec/zfsbootmenu-init
Function: log_unimportable
      1 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-init
Function: check_for_pools
      1 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-init
Function: mount_esp
      1 help-files/132/recovery-shell.ansi
      1 help-files/52/recovery-shell.ansi
      1 help-files/92/recovery-shell.ansi
      2 lib/zfsbootmenu-completions.sh
      1 lib/zfsbootmenu-core.sh
Function: mount_zfs
      1 bin/zfs-chroot
      1 help-files/132/recovery-shell.ansi
      1 help-files/52/recovery-shell.ansi
      1 help-files/92/recovery-shell.ansi
      3 lib/zfsbootmenu-completions.sh
      4 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-diff
Function: kexec_kernel
      2 bin/zbm
      2 bin/zfsbootmenu
      1 bin/zkexec
      1 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-init
Function: duplicate_snapshot
      1 lib/zfsbootmenu-core.sh
      1 lib/zfsbootmenu-ui.sh
Function: clone_snapshot
      1 lib/zfsbootmenu-core.sh
      2 lib/zfsbootmenu-ui.sh
Function: create_snapshot
      1 lib/zfsbootmenu-core.sh
      1 lib/zfsbootmenu-ui.sh
Function: rollback_snapshot
      1 lib/zfsbootmenu-core.sh
      1 lib/zfsbootmenu-ui.sh
Function: set_default_kernel
      2 bin/zbm
      2 bin/zfsbootmenu
      1 lib/zfsbootmenu-core.sh
Function: set_default_env
      1 bin/zbm
      1 bin/zfsbootmenu
      1 lib/zfsbootmenu-core.sh
Function: find_be_initramfs
      2 lib/zfsbootmenu-core.sh
Function: find_be_kernels
      2 lib/zfsbootmenu-core.sh
      1 lib/zfsbootmenu-ui.sh
      1 libexec/zfsbootmenu-init
Function: select_kernel
      1 bin/zbm
      1 bin/zfsbootmenu
      2 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-init
      1 libexec/zfsbootmenu-preview
Function: find_root_prefix
      2 lib/zfsbootmenu-core.sh
Function: validate_cmdline_cache
      2 lib/zfsbootmenu-core.sh
Function: load_be_cmdline
      1 bin/zbm
      1 bin/zfsbootmenu
      2 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-preview
Function: import_pool
      6 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-init
Function: export_pool
      1 bin/poweroff
      1 bin/reboot
      1 bin/shutdown
      4 lib/zfsbootmenu-core.sh
Function: rewind_checkpoint
      1 bin/zbm
      1 bin/zfsbootmenu
      1 lib/zfsbootmenu-core.sh
Function: has_resume_device
      2 lib/zfsbootmenu-core.sh
Function: timed_prompt
      3 bin/zbm
      1 bin/zfs-chroot
      3 bin/zfsbootmenu
      5 lib/zfsbootmenu-core.sh
      1 lib/zfsbootmenu-ui.sh
      3 libexec/zfsbootmenu-init
Function: resume_prompt
      2 lib/zfsbootmenu-core.sh
Function: is_snapshot
      1 bin/zbm
      2 bin/zfs-chroot
      1 bin/zfsbootmenu
      1 bin/zsnapshots
      3 lib/zfsbootmenu-core.sh
Function: is_writable
      1 bin/poweroff
      1 bin/reboot
      1 bin/shutdown
      1 bin/zbm
      2 bin/zfs-chroot
      1 bin/zfsbootmenu
      2 lib/zfsbootmenu-completions.sh
      4 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-preview
Function: set_rw_pool
      1 bin/zbm
      1 bin/zfs-chroot
      1 bin/zfsbootmenu
      1 help-files/132/recovery-shell.ansi
      1 help-files/52/recovery-shell.ansi
      1 help-files/92/recovery-shell.ansi
      2 lib/zfsbootmenu-completions.sh
      8 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-diff
Function: set_ro_pool
      1 bin/zbm
      1 bin/zfsbootmenu
      1 help-files/132/recovery-shell.ansi
      1 help-files/52/recovery-shell.ansi
      1 help-files/92/recovery-shell.ansi
      2 lib/zfsbootmenu-completions.sh
      1 lib/zfsbootmenu-core.sh
Function: be_has_encroot
      4 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-preview
Function: be_is_locked
      6 lib/zfsbootmenu-core.sh
Function: be_keysource
      3 lib/zfsbootmenu-core.sh
Function: cache_key
      2 lib/zfsbootmenu-core.sh
Function: load_key
      1 bin/zfs-chroot
     12 lib/zfsbootmenu-core.sh
      1 lib/zfsbootmenu-ui.sh
      1 libexec/zfsbootmenu-diff
      1 libexec/zfsbootmenu-init
Function: be_location
      4 lib/zfsbootmenu-core.sh
      2 lib/zfsbootmenu-ui.sh
Function: zfs_chroot
      2 bin/zbm
      2 bin/zfsbootmenu
      1 lib/zfsbootmenu-completions.sh
      1 lib/zfsbootmenu-core.sh
Function: emergency_shell
      1 bin/zbm
      1 bin/zfsbootmenu
      1 lib/fzf-defaults.sh
      2 lib/zfsbootmenu-core.sh
      5 libexec/zfsbootmenu-init
Function: zreport
      1 help-files/132/recovery-shell.ansi
      1 help-files/52/recovery-shell.ansi
      1 help-files/92/recovery-shell.ansi
      1 lib/zfsbootmenu-core.sh
      3 libexec/zfsbootmenu-help
      1 libexec/zfsbootmenu-init
Function: import_zbm_hooks
      1 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-init
Function: is_mountpoint
      3 lib/zfsbootmenu-core.sh
Function: is_mounted
      1 lib/zfsbootmenu-completions.sh
      2 lib/zfsbootmenu-core.sh
Function: is_efi_system
      2 lib/zfsbootmenu-core.sh
Function: mount_efivarfs
      3 bin/zfs-chroot
      1 help-files/132/recovery-shell.ansi
      1 help-files/52/recovery-shell.ansi
      1 help-files/92/recovery-shell.ansi
      2 lib/zfsbootmenu-completions.sh
      3 lib/zfsbootmenu-core.sh
      1 libexec/zfsbootmenu-init

@zdykstra
Copy link
Member Author

I think this belongs somewhere other than -core. That file provides (or should provide) critical functions used in the primary control flow of ZBM. It seems to me this should be a standalone bin script. If there is an argument against that, maybe adding a new -helpers or -utils library that collects this and a few other user-exposed functions might be good.

At some point, it would be good to audit the ZBM library and relocate other functions as appropriate.

Having the recovery shell dump errors right to the console, maybe colored red, would be an awesome feature.

My impetus for adding mount_esp as a function was simply because mount_zfs is also a function, so adding it groups it with the other dedicated mount helper function logically and on-disk. I don't have a stronger argument for leaving it in zfsbootmenu-core.sh. If you feel that it should be a stand-alone binary, I can move it there.

Based on the list above, zreport is realistically the only one that could be moved to be outside of zfsbootmenu-core.sh. It would do well to be a stand-alone binary - the -init reference to it is only when debug logging is enabled.

@ahesford
Copy link
Member

If zreport is the only other function that might be split up, it's probably not worth the effort to separate things at this point. We can revisit when we ponder writing some of the tools in Java.

@zdykstra zdykstra force-pushed the extra-sensory-perception branch from ce3225c to ebb17b9 Compare December 17, 2023 20:09
@zdykstra
Copy link
Member Author

I've added the relevant kernel modules for mounting an ESP to the optional modules list in install-helpers.sh. There's no point in building in a helper that can't (normally) mount the filesystem.

@ahesford
Copy link
Member

I still don't know how these modules wind up in the release or recovery images, but including the modules explicitly if they are available seems like a good idea.

@zdykstra
Copy link
Member Author

I'm not sure how they're getting in there, either. I thought we were explicitly adding them in to the release/recovery images, but that's not the case. It's probably worth it to make them a hard requirement for release/recovery images on top of the best-effort installation effort that's the default.

@zdykstra zdykstra force-pushed the extra-sensory-perception branch 2 times, most recently from deb1389 to 77be7ba Compare December 18, 2023 21:31
Add a mount_esp helper with tab completion. This is a very thin wrapper
over the new mount_block function.

Add the relevant kernel modules to the optional modules list for both
dracut and mkinitcpio images. Explicitly add the same modules to the
release/recovery shared config.
@zdykstra zdykstra force-pushed the extra-sensory-perception branch from 77be7ba to cd6d101 Compare December 19, 2023 03:02
@zdykstra zdykstra merged commit fd4d012 into master Dec 19, 2023
3 checks passed
@zdykstra zdykstra deleted the extra-sensory-perception branch December 19, 2023 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants