geom(4): Kill GEOM_PART_EBR_COMPAT option
ClosedPublic
Actions

Authored by cem on May 21 2020, 5:10 AM.

Details

Reviewers

emaste
kevans
imp
ae
marcel

Commits

rS362823: geom(4): Kill GEOM_PART_EBR_COMPAT option

Summary

Take advantage of Warner's nice new real GEOM aliasing system and use it for
aliased partition names that actually work.

Our canonical EBR partition name is the weird, not-default-on-x86-prior-to-
this-revision "da1p4+00001234." However, if compatibility mode (tunable
kern.geom.part.ebr.compat_aliases) is enabled (1, default), we continue to
provide the alias names like "da1p5" in addition to the weird canonical
names.

Naming partition providers was just one aspect of the COMPAT knob; in
addition it limited mutability, in part because it did not preserve existing
EBR header content aside from that of LBA 0. This change saves the EBR
header for LBA 0, as well as for every EBR partition encountered. That way,
when we write out the EBR partition table on modification, we can restore
any bootloader or other metadata in both LBA0 (the first data-containing EBR
may start after 0) as well as every logical EBR we read from the disk, and
only update the geometry metadata and linked list pointers that describe the
actual partitioning.

(This change does not add support for the 'bootcode' verb to EBR.)

PR: 232463

Test Plan

# sysctl kern.features.geom_part_ebr_compat kern.geom.part.ebr.compat_aliases
kern.features.geom_part_ebr_compat: 1
kern.geom.part.ebr.compat_aliases: 1

# mdconfig -a -t swap -s 200
md0
# gpart create -s mbr md0
md0 created
# gpart add -t ebr -i 4 md0
md0s4 added
# gpart create -s ebr md0s4
md0s4 created
# gpart add -t freebsd md0s4
md0s4+00000001 added
# ls -l /dev/md0s*
crw-r----- 1 root operator 0, 59 May 21 21:39 /dev/md0s4
crw-r----- 1 root operator 0, 60 May 21 21:42 /dev/md0s4+00000001
lrwxr-xr-x 1 root wheel       14 May 21 21:42 /dev/md0s5 -> md0s4+00000001

Splitting a middle EBR partition:

# gpart show
...
=>  1  199  md0  MBR  (100K)
    1  199    4  ebr  (100K)

=>  0  199  md0s4  EBR  (100K)
    0    2      1  freebsd  (512B)
    2   50      3  freebsd  (25K)
   52   50     53  freebsd  (25K)
  102   50    103  freebsd  (25K)
  152   47    153  freebsd  (23K)
# ls -l /dev/md0s*
crw-r----- 1 root operator 0, 59 May 21 21:39 /dev/md0s4
crw-r----- 1 root operator 0, 60 May 21 21:58 /dev/md0s4+00000001
crw-r----- 1 root operator 0, 62 May 22 07:14 /dev/md0s4+00000003
crw-r----- 1 root operator 0, 64 May 22 07:14 /dev/md0s4+00000053
crw-r----- 1 root operator 0, 96 May 22 07:14 /dev/md0s4+00000103
crw-r----- 1 root operator 0, 99 May 22 07:14 /dev/md0s4+00000153
lrwxr-xr-x 1 root wheel       14 May 21 21:58 /dev/md0s5 -> md0s4+00000001
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s6 -> md0s4+00000003
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s7 -> md0s4+00000053
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s8 -> md0s4+00000103
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s9 -> md0s4+00000153

# gpart delete -i 3 md0s4
md0s4+00000003 deleted
# ls -l /dev/md0s*
...
lrwxr-xr-x 1 root wheel       14 May 21 21:58 /dev/md0s5 -> md0s4+00000001
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s7 -> md0s4+00000053
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s8 -> md0s4+00000103
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s9 -> md0s4+00000153

# gpart add -t freebsd -s 25 md0s4
g_dev_taste: make_dev_alias_p() failed (name=md0s9, error=17)   # <<< our alias unit number does not utilize g_part's built-in index uniqueing.
md0s4+00000003 added
# gpart add -t freebsd -s 25 md0s4
md0s4+00000028 added
# ls -l /dev/md0s*
...
lrwxr-xr-x 1 root wheel       14 May 21 21:58 /dev/md0s5 -> md0s4+00000001
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s7 -> md0s4+00000053
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s8 -> md0s4+00000103
lrwxr-xr-x 1 root wheel       14 May 22 07:14 /dev/md0s9 -> md0s4+00000153
lrwxr-xr-x 1 root wheel       14 May 22 07:19 /dev/md0s10 -> md0s4+00000028

# N.B., no symlink for EBR partition +03 and the numbering for +28 is out of order.  This is mildly ugly but I figure it is not a regression with compat mode, as compat mode never allowed editing the part table.  And the non-compat mode never had the compat symlinks.

"Reboot" by saving the md image and restoring it.

# dd if=/dev/md0 of=./md0.ebr.img
# mdconfig -d -u 0
# ls /dev/md0*
ls: cannot access '/dev/md0*': No such file or directory
# mdconfig -a -f ./md0.ebr.img
md0

# gpart show
=>  0  199  md0s4  EBR  (100K)
    0    2      1  freebsd  (512B)
    2   25      3  freebsd  (12K)
   27   25     28  freebsd  (12K)
   52   50     53  freebsd  (25K)
  102   50    103  freebsd  (25K)
  152   47    153  freebsd  (23K)
# ls -l /dev/md0*
crw-r----- 1 root operator 0,  59 May 22 12:34 /dev/md0s4
crw-r----- 1 root operator 0,  60 May 22 12:34 /dev/md0s4+00000001
crw-r----- 1 root operator 0,  62 May 22 12:34 /dev/md0s4+00000003
crw-r----- 1 root operator 0,  64 May 22 12:34 /dev/md0s4+00000028
crw-r----- 1 root operator 0,  96 May 22 12:34 /dev/md0s4+00000053
crw-r----- 1 root operator 0,  99 May 22 12:34 /dev/md0s4+00000103
crw-r----- 1 root operator 0, 101 May 22 12:34 /dev/md0s4+00000153
lrwxr-xr-x 1 root wheel        14 May 22 12:34 /dev/md0s5 -> md0s4+00000001
lrwxr-xr-x 1 root wheel        14 May 22 12:34 /dev/md0s6 -> md0s4+00000003
lrwxr-xr-x 1 root wheel        14 May 22 12:34 /dev/md0s7 -> md0s4+00000028
lrwxr-xr-x 1 root wheel        14 May 22 12:34 /dev/md0s8 -> md0s4+00000053
lrwxr-xr-x 1 root wheel        14 May 22 12:34 /dev/md0s9 -> md0s4+00000103
lrwxr-xr-x 1 root wheel        14 May 22 12:34 /dev/md0s10 -> md0s4+00000153

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 31229
Build 28879: arc lint + arc unit

Event Timeline

cem requested review of this revision.May 21 2020, 5:10 AM

cem created this revision.

Harbormaster completed remote builds in B31211: Diff 72064.May 21 2020, 5:10 AM

cem added a parent revision: D24938: geom_part: Dispatch to partitions to create providers and aliases.May 21 2020, 5:25 AM

Rebase on updated parent revision

Harbormaster completed remote builds in B31229: Diff 72104.May 22 2020, 4:52 AM

cem added reviewers: emaste, kevans, imp, ae.May 22 2020, 4:52 AM

cem edited the test plan for this revision. (Show Details)May 22 2020, 4:56 AM

ae added a reviewer: marcel.May 22 2020, 12:21 PM

I have no objection against this. Just one test case to test - create several EBR partitions (e.g. s5, s6, s7), then remove one from the middle (i.e. s6), then create two smaller in this free space (and then optionally reboot, and see what we will have after reboot).

In D24939#549478, @ae wrote:

I have no objection against this. Just one test case to test - create several EBR partitions (e.g. s5, s6, s7), then remove one from the middle (i.e. s6), then create two smaller in this free space (and then optionally reboot, and see what we will have after reboot).

Thanks! They will definitely be numbered differently after reboot. That matches existing EBR compat behavior — if you reboot into something with mutable EBR, and insert two partitions, and boot back into FreeBSD, everything afterward will be shifted. So use the "new"-style names or filesystem labels in /etc/fstab (this was always the right way to do it).

I will add output from this case to "TEST PLAN" in a minute or two, thanks!

cem edited the test plan for this revision. (Show Details)May 22 2020, 2:29 PM

cem edited the test plan for this revision. (Show Details)May 22 2020, 7:37 PM

Fix silly bug misnumbering compatibility EBR slices when reading existing disk images. Updated Testing section shows the expected, correct output now.

Harbormaster completed remote builds in B31243: Diff 72135.May 22 2020, 7:39 PM

This revision was not accepted when it landed; it landed in state Needs Review.Jul 1 2020, 2:16 AM

Closed by commit rS362823: geom(4): Kill GEOM_PART_EBR_COMPAT option (authored by cem). · Explain Why

This revision was automatically updated to reflect the committed changes.

cem added a commit: rS362823: geom(4): Kill GEOM_PART_EBR_COMPAT option.