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

libefi: remove efi_type() #12969

Merged
merged 1 commit into from
Jan 18, 2022
Merged

Conversation

nabijaczleweli
Copy link
Contributor

@nabijaczleweli nabijaczleweli commented Jan 13, 2022

Motivation and Context

As noted in #12844:

All it is right now is some #if 0ed Solaris code that returns ENOSYS, and is only applicable for the Solaris blockdev layer. In the Illumos gate, there's a single user: rmformat(1); I recommend a read of the manual as a blast from the past, but, well

It never was anything but: imported in 5c36312 (Fri Oct 9 15:37:29 2009 -0700), commented out in d603ed6 (Thu Aug 26 11:56:53 2010 -0700), which says

    This topic branch contains all the changes needed to integrate the user
    side zfs tools with Linux style devices.  Primarily this includes fixing
    up the Solaris libefi library to be Linux friendly, and integrating with
    the libblkid library which is provided by e2fsprogs.

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)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

All it is right now is some #if 0ed Solaris code that returns ENOSYS,
and is only applicable for the Solaris blockdev layer.
In the Illumos gate, there's a single user: rmformat(1);
I recommend a read of the manual as a blast from the past, but, well

Ref: openzfs#12844
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

Yes, this was always dead code which never got removed. Happy to see it go.

@behlendorf behlendorf added the Status: Code Review Needed Ready for review and testing label Jan 13, 2022
@sempervictus
Copy link
Contributor

This reminds me of #11408 - what is the plan for native handling of (bootable) EFI-legal volumes via ZFS?

@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Jan 14, 2022

How'd you mean? I'm booting a ZFS root off EFI (GPT[ESP, root pool]) on my laptop and on CSM (GPT[GRUB trampoline, /boot, root pool]; 10th-gen Dell EFIs are Not There) on my flat computer. There's no reason you can't chainload a ZFS rootfs off your platform's favourite bootloader (be it GRUB, the platform firmware, kexec, &c.) for pretty much any disk configuration, is there?

e: oops, i seem to've misread the linked issue; that sounds nicer for the common case, yeah. Probably wasn't prioritised for the reason above.

@sempervictus
Copy link
Contributor

sempervictus commented Jan 16, 2022

@nabijaczleweli: thats how everyone's doing it really, because ZFS doesn't support the EFI requirements in its zpool format. The suggestion there is that any pool which is to be bootable must have the relevant space allocated for EFI data and presented as a fat32 slab of contiguous blocks. I pulled in a few commits a while back which help with MBR booting by embedding a boot block into the zpool's starting sector and such, but for EFI its an actual zpool format specification change since you need that slab of other-filesystem. Internally at Semper Victus we have a CI-built Arch image which we use to boot... everything pretty much, and since its on removable media the EFI partition needs to go with it. Leveraging the internal EFI space on the mainboard is also not an option on a ton of systems as their BIOS lock-in the secure boot section to only be allowed to be written by Windows using an MS signing key (like every clevo laptop, some of the major ones, a bunch of supermicro boxes, etc) and don't accept custom keys correctly.
EDIT: by no means suggesting this PR is flawed, simply clarifying context regarding problems facing various boot paradigms w/ EFI.

@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 18, 2022
@behlendorf behlendorf merged commit e1c720d into openzfs:master Jan 18, 2022
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
All it is right now is some #if 0ed Solaris code that returns ENOSYS,
and is only applicable for the Solaris blockdev layer.
In the Illumos gate, there's a single user: rmformat(1);
I recommend a read of the manual as a blast from the past, but, well

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Issue openzfs#12844
Closes openzfs#12969
nicman23 pushed a commit to nicman23/zfs that referenced this pull request Aug 22, 2022
All it is right now is some #if 0ed Solaris code that returns ENOSYS,
and is only applicable for the Solaris blockdev layer.
In the Illumos gate, there's a single user: rmformat(1);
I recommend a read of the manual as a blast from the past, but, well

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Issue openzfs#12844
Closes openzfs#12969
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