-
-
The problem here is that the PMBR is created in
libefi::efi_write(int fd, dk_gpt_t *vtoc)
which is a committed interface used by a range of things, not justzpool
- for exampleformat
andfmthard
both use it.Options that I also considered:
1. add new flags to dk_gpt_t.efi_flags - for the known firmware problems I actually only need two bits, set partition active and put partition in slot 1 - and then add parameters/options tozpool
to set these bits on pool creation;
2. updatefdisk
with new options (or possibly extend -E) so that the PMBR can be modified (by an installer, say) following pool creation.All have pros and cons and I'd welcome any discussion around this.
-
I must admit that partitioning in solarish world is one of the most unpleasing things for me, and I'm trying to look at the related code, and even think about it, as little as possible.
What I wanted to say is that creating a boot pool should be a single time operation, done mostly in the installer, so providing a tunable in /etc/default/something does look a bit weird.
-
The default file has obvious issue, in install image you have (possibly) read only /etc/default.
The question there is, do we have list of "known" bad behaving systems and can hide all this mess and populate PMBR based on hw info from smbios? This list can not be too long:)
-
Thanks to FreeBSD, yes - see https://github.com/citrus-it/kayak/blob/46ef74e49abff7cfeb8c482f7d54922fe2fbf91d/hwfixes.sh
-
So we have a fourth option which is to link libefi with libsmbios and put the logic there with an external known-bad-hardware database to make the entire thing automatic.
I'm actually currently leaning towards adding this function to fdisk as it could be used by installers but also by people moving disks from a good to a bad system - just
fdisk -E:1,1
or something and fix-up the PMBR. -
FWIW, FreeBSD installer does
gpart set -a lenovofix /dev/...
andgpart set -a active /dev/...
for affected systems. -
Having the utility support would be very nice for manual fix; I still would like to have it sort of done automatically for user too because it is one thing we can have automated and therefore there is no reason why we should not do it...
also keep in mind that we will get disks relabeled on zpool replace (see /etc/sysevent/*).
-
yea, I have long dream of having gpart ported (the command and UI logic, not geom;) and implemented, but just do not have time. We really need properly scriptable partition management tool, but thats different topic anyhow.
-
Would that mean getting rid of all minors we expose now by default? :-)
-
Leave it with me - I will rework this when I get time.
-
-
9067 Support PMBR customisation with EFI label
Review Request #843 — Created Feb. 7, 2018 and submitted
Information | |
---|---|
citrus | |
illumos-gate | |
master | |
9067, 9116 | |
6fbc984... | |
Reviewers | |
general | |
hadfl |
9067 Support PMBR customisation with EFI label
libefi:
reaper# grep Dell /usr/share/hwdata/efi.fixes | tail -1 sys.manufacturer="Dell Inc." sys.product="PowerEdge R730" mb.version=A04 pmbr_active=1 pmbr_slot=3 reaper# zpool create test c0t13d1 reaper# fdisk -W - c0t13d1p0 | tail -5 * Id Act Bhead Bsect Bcyl Ehead Esect Ecyl Rsect Numsect 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 238 128 255 63 1023 255 63 1023 1 62499999 reaper# zpool replace test c0t{13,14}d1 reaper# fdisk -W - c0t14d1p0 | tail -5 * Id Act Bhead Bsect Bcyl Ehead Esect Ecyl Rsect Numsect 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 238 128 255 63 1023 255 63 1023 1 62499999 reaper# sed -i '/PowerEdge/s/active=1/active=0/' /usr/share/hwdata/efi.fixes reaper# zpool replace test c0t{14,13}d1 reaper# fdisk -W - c0t13d1p0 | tail -5 * Id Act Bhead Bsect Bcyl Ehead Esect Ecyl Rsect Numsect 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 238 0 255 63 1023 255 63 1023 1 62499999 reaper# sed -i '/PowerEdge/s/slot=3/slot=1/' /usr/share/hwdata/efi.fixes reaper# zpool replace test c0t{13,14}d1 reaper# fdisk -W - c0t14d1p0 | tail -5 * Id Act Bhead Bsect Bcyl Ehead Esect Ecyl Rsect Numsect 0 0 0 0 0 0 0 0 0 0 238 0 255 63 1023 255 63 1023 1 62499999 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 reaper# sed -i '/PowerEdge/s/^/#/' /usr/share/hwdata/efi.fixes reaper# zpool replace test c0t{14,13}d1 reaper# fdisk -W - c0t13d1p0 | tail -5 * Id Act Bhead Bsect Bcyl Ehead Esect Ecyl Rsect Numsect 238 0 255 63 1023 255 63 1023 1 62499999 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0fdisk:
reaper# zpool export test reaper# fdisk -E 2:1 c0t13d1p0 reaper# fdisk -W - c0t13d1p0 | tail -5 * Id Act Bhead Bsect Bcyl Ehead Esect Ecyl Rsect Numsect 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 238 128 255 63 1023 255 63 1023 1 62499999 0 0 0 0 0 0 0 0 0 0 reaper# zpool import test reaper# zpool status test pool: test state: ONLINE scan: resilvered 86K in 0h0m with 0 errors on Mon Feb 12 10:46:49 2018 config: NAME STATE READ WRITE CKSUM test ONLINE 0 0 0 c0t13d1 ONLINE 0 0 0 errors: No known data errors
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Bugs: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
People: |
|
Change Summary:
Round 2 - different implementation with two parts.
fdisk -E
extended to take arguments so that the PMBR can be modified if necessary (useful for testing, patching, or moving disks between systems).- libefi - automatically tweak the PMBR for broken systems as defined in a separate database (text-file). The benefit of doing it here is that all consumers of libefi benefit and things like replacing a disk with
zpool replace
work as expected - the replacement disk gets the modified PMBR too.I'm still unsure about where the database file should be in the filesystem but I think it is right to set
preserve=renamenew
since anyone who has modified this file has done it to support the host system and won't want the changes to be lost on an upgrade (and likely won't care about changes in that file since it's working for them).There was a bug in fdisk too that caused the EFI labels to always be cleared, even if the old and new PMBRs both contained an EFI partition of the same size and location. Please see testing notes for a summary of this working on a test system.
Lastly, the database file still needs a copyright adding. Since the data therein is derived from the FreeBSD installer, it likely needs to include theirs too but advice welcome.
Comments appreciated, thanks!
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+300 -45) |
-
Can't speak for the actually code but I do like the new design.
/system/boot/kernel/drv <== driver configs like sd.conf live there. Maybe somewhere close to that location would be a good place to store it?
Change Summary:
Made smbios string checks case-insensitive. Added some missing (void) casts.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+326 -45) |
-
Successfully installed OmniOS on my LENOVO ThinkPad X1 with an install imaged based on this code provided by Andy.
-
-
usr/src/cmd/fdisk/fdisk.c (Diff revision 3) I think this (and 2265) would need issue and explanation - you can combine multiple issues in single update.
Change Summary:
Modify maximum HEAD globally in fdisk.c and adjust, include new bug number too.
Bugs: |
|
||||
---|---|---|---|---|---|
Commit: |
|
||||
Diff: |
Revision 4 (+325 -45) |
Change Summary:
Add THIRDPARTYLICENSE for efi.fixes database. Remove duplicate MAX_ definitions from fdisk.c.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+369 -64) |