9529 libefi: efi_alloc_and_read should check for PMBR

Review Request #348 — Created Feb. 6, 2017 and submitted

tsoome
illumos-gate
9529
75b6047...
general

9529 libefi: efi_alloc_and_read should check for PMBR

Trying to read MBR only usb stick:

installboot -i /dev/rdsk/c7t0d0p1

open: No such file or directory
open: No such file or directory
read: Bad file number
Error reading bootblock from /dev/rds1

using fixed libefi:

env LD_LIBRARY_PATH=./lib/libefi/i386 installboot -i /dev/rdsk/c7t0d0p1

Extended version string: 1.1-2017.0.0.1
MD5 hash: e794b290bc596560f6980cccc78b0b4a

env LD_LIBRARY_PATH=./lib/libefi/i386 installboot -i /dev/rdsk/c3t0d0s1

Extended version string: 1.1-2017.0.0.2
MD5 hash: dfd0678f1e81dcc72cd4557881b520e8

Also tested newfs:

root@test:/home/tsoome# newfs /dev/zvol/rdsk/raid/test 
newfs: construct a new file system /dev/zvol/rdsk/raid/test: (y/n)? y
Warning: 4096 sector(s) in last cylinder unallocated
/dev/zvol/rdsk/raid/test:       2097152 sectors in 342 cylinders of 48 tracks, 128 sectors
        1024,0MB in 25 cyl groups (14 c/g, 42,00MB/g, 20160 i/g)
super-block backups (for fsck -F ufs -o b=#) at:
 32, 86176, 172320, 258464, 344608, 430752, 516896, 603040, 689184, 775328,
 1292192, 1378336, 1464480, 1550624, 1636768, 1722912, 1809056, 1895200,
 1981344, 2067488
root@test:/home/tsoome# fstyp /dev/zvol/rdsk/raid/test 
ufs
root@test:/home/tsoome#

root@test:/usr/lib# newfs /dev/zvol/rdsk/raid/swap
newfs: construct a new file system /dev/zvol/rdsk/raid/swap: (y/n)? y
/dev/zvol/dsk/raid/swap is currently used by swap. Please see swap(1M).
root@test:/usr/lib# newfs /dev/zvol/rdsk/raid/dump
newfs: construct a new file system /dev/zvol/rdsk/raid/dump: (y/n)? y
/dev/zvol/dsk/raid/dump is in use by dump. Please see dumpadm(1M).
root@test:/usr/lib#
  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
tsoome
tsoome
tsoome
yuripv
  1. 
      
  2. usr/src/lib/libefi/common/rdwr_efi.c (Diff revision 3)
     
     

    Returning VT_ERROR here and on line 244 doesn't seem to be correct -- the comment says "errno supplies specific error", and we don't have it set to anything specific in these cases.

    1. yep, we should return VT_EINVAL here and below.

  3. usr/src/lib/libefi/common/rdwr_efi.c (Diff revision 3)
     
     

    inconsistent {} :-)

  4. 
      
tsoome
yuripv
  1. If we can make sure that there are no other consumers that will be unhappy with these changes, ship it.

  2. 
      
tsoome
yuripv
  1. 
      
  2. usr/src/cmd/fs.d/ufs/mkfs/mkfs.c (Diff revision 4)
     
     

    Just a tiny nit, I'd remove the else clause as we exit above anyway (usually it helps with reducing the indentation a bit, not really required to change anything here).

    1. Yep, seems good idea.

  3. 
      
tsoome
tsoome
yuripv
  1. I'm sorry to say this, but looks like there's another issue:

    old libefi:

    # LD_PRELOAD=~/libefi.so.1 mkfs -F ufs /dev/zvol/rdsk/rpool/dump 106760192
    /dev/zvol/dsk/rpool/dump is in use by dump. Please see dumpadm(1M).
    

    updated libefi:

    # mkfs -F ufs /dev/zvol/rdsk/rpool/dump 106760192
    Warning: 4096 sector(s) in last cylinder unallocated
    /dev/zvol/rdsk/rpool/dump:      106760192 sectors in 17377 cylinders of 48 tracks, 128 sectors
            52129.0MB in 1337 cyl groups (13 c/g, 39.00MB/g, 18624 i/g)
    super-block backups (for fsck -F ufs -o b=#) at:
     32, 80032, 160032, 240032, 320032, 400032, 480032, 560032, 640032, 720032,
    Initializing cylinder groups:
    ..........................
    super-block backups for last 10 cylinder groups at:
     105996192, 106076192, 106156192, 106236192, 106316192, 106396192, 106476192,
     106556192, 106636192, 106716192
    
    1. thats only good. The reason for this issue is http://src.illumos.org/source/xref/illumos-gate/usr/src/lib/libdiskmgt/common/slice.c#352 - they drop out when there is no vtoc or EFI and the currenct code relies on fabricated EFI... that is, on partition table which does not exist.

    2. Well, it's probably good as it shows how ugly libdiskmgt is, but still introduces a regression :-)

    3. See the next change...

    4. Thank you.

  2. 
      
tsoome
tsoome
tsoome
yuripv
  1. Ship It!
  2. 
      
alhazred
  1. Ship It!
  2. 
      
tsoome
Review request changed

Status: Closed (submitted)

Loading...