9067 Support PMBR customisation with EFI label

Review Request #843 — Created Feb. 7, 2018 and submitted

citrus
illumos-gate
master
9067, 9116
6fbc984...
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          0

fdisk:

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
  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
yuripv
  1. I really don't feel like providing a "default" file is correct solution here, could it be done in say zpool command instead, same way we added the -B option?

    1. 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 just zpool - for example format and fmthard 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 to zpool to set these bits on pool creation;
      2. update fdisk 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.

    2. 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.

    3. 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:)

    4. Thanks to FreeBSD, yes - see https://github.com/citrus-it/kayak/blob/46ef74e49abff7cfeb8c482f7d54922fe2fbf91d/hwfixes.sh

    5. 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.

    6. FWIW, FreeBSD installer does gpart set -a lenovofix /dev/... and gpart set -a active /dev/... for affected systems.

    7. 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/*).

    8. 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.

    9. Would that mean getting rid of all minors we expose now by default? :-)

    10. Leave it with me - I will rework this when I get time.

  2. 
      
citrus
citrus
sjorge
  1. 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?

  2. 
      
citrus
sjorge
  1. Successfully installed OmniOS on my LENOVO ThinkPad X1 with an install imaged based on this code provided by Andy.

  2. 
      
tsoome
  1. 
      
  2. 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.

  3. 
      
citrus
tsoome
  1. Ship It!
  2. 
      
yuripv
  1. The new BSD licensed file likely needs the THIRDPARTYLICENSE stuff?

    1. I don't know - would appreciate some assistance with this issue.
      That file didn't exactly come from FreeBSD, it's just that the information in there about what systems are affected and what fix should be applied is derived from a file in their installer - their file looks nothing like this.

      I obviously want to properly attribute the source, so what does it need?

  2. 
      
citrus
hadfl
  1. Ship It!
  2. 
      
citrus
Review request changed

Status: Closed (submitted)

Loading...