Project

General

Profile

Bug #11182

loader: Distinguish between "no partition" and "choose best partition" with a constant.

Added by Toomas Soome 4 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
bootloader
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

The disk API in loader has received series of cleanups in FreeBSD, also we need to address issues found by smatch.

    https://svnweb.freebsd.org/base?view=revision&revision=345477

    The values of the d_slice and d_partition fields of a disk_devdesc have a
    few values with special meanings in the disk_open() routine. Through various
    evolutions of the loader code over time, a d_partition value of -1 has
    meant both "use the first ufs partition found in the bsd label" and "don't
    open a bsd partition at all, open the raw slice." 

    This defines a new special value of -2 to mean open the raw slice, and it
    gives symbolic names to all the special values used in d_slice and
    d_partition, and adjusts all existing uses of those fields to use the new
    constants.

    The phab review for this timed out without being accepted, but I'm still
    citing it below because there is useful commentary there.

    Differential Revision:  https://reviews.freebsd.org/D19262

    https://svnweb.freebsd.org/base?view=revision&revision=r346675
    Restore the ability to open a raw disk or partition in loader(8).

    The disk_open() function searches for "the best partition" when slice and
    partition information is not provided as part of the device name.  As of
    r345477 the slice and partition fields of a disk_devdesc are initialized to
    D_SLICEWILD and D_PARTWILD; in the past they were initialized to -1, which
    was sometimes interpreted as meaning 'wildcard' and sometimes as 'open the
    raw partition' depending on the context.  So as an unintended side effect of
    r345477 it became basically impossible to ever open a disk or partition
    without doing the 'best partition' search.  One visible effect of that was
    the inability to open the raw disk to read the partition table correctly in
    zfs_probe_dev(), leading to failures to find the zfs pool unless it was on
    the first partition.

    Now instead of always initializing slice and partition to wildcards, the
    disk_parsedev() function initializes them based on the presence of a
    path/file name following the device.  If there is any path or filename
    following the ':' that ends the device name, then slice and partition are
    initialized to D_SLICEWILD and D_PARTWILD.  If there is nothing after the
    ':' then it is considered to be a request to open the raw device or
    partition itself (not a file stored within it), and the fields are
    initialized to D_SLICENONE and D_PARTNONE.

    With this change in place, all the tests in src/tools/boot are succesful
    again, including the recently-added cases of booting from a zfs pool on
    a partition other than slice 1 of the device.

    https://svnweb.freebsd.org/base?view=revision&revision=r346984
    Use D_PARTISGPT rather than bare 255

    These three cases dovetail with other places in the code where we use
    or set D_PARTISGPT when we mean that the partitioning scheme is
    GPT. Use this #define to make the code easier to undertand.

    Reviewed by: tsoome@
    Differential Revision: https://reviews.freebsd.org/D20122

    use local partdev device descriptor to read disk global partition table, so we do not disturb provided dev descriptor.

    Check if we have VTOC at the start of the disk.

    In case the D_PARTISGPT was used with non-GPT table, use D_PARTWILD to allow
    the autodetection code to work - D_PARTNONE means we should open raw MBR
    partition.

    smatch findings:    
    /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: ../../common/disk.c:310 disk_open() warn: was && intended here instead of ||?
    /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: ../../common/disk.c:327 disk_open() warn: was && intended here instead of ||?

History

#1

Updated by Toomas Soome 4 months ago

Testing done:

ok lsdev
fd devices:
    fd0:   BIOS drive A (2880 X 512):
disk devices:
    disk0:   BIOS drive C (62914560 X 512):
      disk0p1: EFI
      disk0p2: illumos ZFS
      disk0p9: Reserved
    disk1:   BIOS drive D (20971520 X 512):
      disk1s1: Solaris 2
        disk1s1a: root
        disk1s1b: swap
        disk1s1g: usr
        disk1s1i: boot
    disk2:   BIOS drive E (20971520 X 512):
      disk2p1: illumos ZFS
      disk2p9: Reserved

#2

Updated by Electric Monk 4 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 90 to 100

git commit 9a34674dce796d46567833216389d6d430925bb2

commit  9a34674dce796d46567833216389d6d430925bb2
Author: Toomas Soome <tsoome@me.com>
Date:   2019-06-18T17:44:04.000Z

    11182 loader: Distinguish between "no partition" and "choose best partition" with a constant.
    Reviewed by: John Levon <john.levon@joyent.com>
    Reviewed by: Andy Fiddaman <andy@omniosce.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF