In D40816#1164504, @jhb wrote:

In D40816#1155652, @allanjude wrote:

I am not sure how the import of OpenZFS would have changed any of this, but, for MBR we hide the 256kb /boot/zfsboot file in the slack space between the end of the ZFS uberblocks, and the start of the data area (4 MiB offset from the start of the partition).

The first 1kb, is installed in the slack at the start of the partition.

So basically:

1. mbr reads first 1kb of partition
2. first 512b of zfsboot knows where to find the rest of zfsboot
3. the rest of zfsboot is installed at an offset of 1024x512b where the first part of zfsboot knows where to find it.

1. mbr actually reads the first 512b of the partition

However, this change seems to not trust ZFS to do what you claim above. Rather than letting ZFS start at the start of the MBR slice, it is creating an explicit gap so that s1a doesn't start at the the start of a slice but leaves a 2048 sector (1MB) "hole" (between the start of the MBR slice, and the first BSD label partition ("s1a"). It then places the rest of the zfsboot payload at an offset (1024 sectors in) to the rest of that "hole" before the actual ZFS partition starts. So, clearly it seems like OpenZFS is not preserving the same layout for some reason and this change is trying to avoid interacting with ZFS entirely. If we really want ZFS + MBR to work this way, we should just change zfsboot to stop assuming the 1024 offset so we can write zfsboot contiguously to the start of the MBR slice. Then we could at least leave a smaller "hole" prior to "s1a". I'm also not sure how wise it is to leave unallocated space in the BSD label at the start of the slice. If you happen to fat finger a gpart command in the future, you could end up adding a new partition in the "hole" and end up overwriting the zfsboot contents. :(

Given all that, I wonder if we shouldn't just punt and say that ZFS + MBR isn't supported anymore. It hasn't worked in years and is pretty fragile.

I wonder if the -q ends up hiding too much useful context for the error, but better to avoid the console spam in the happy case.

Very useful functionality

The description says 'patched' (if an tagged incoming frame does not patch the port's pvid) where I think you meant 'matched'

I am not sure how the import of OpenZFS would have changed any of this, but, for MBR we hide the 256kb /boot/zfsboot file in the slack space between the end of the ZFS uberblocks, and the start of the data area (4 MiB offset from the start of the partition).

I didn't realize we had added -l to /bin/sh.

Looks good

Otherwise looks good to me.

This seems fine, is there a reason it is not moving forward?

silly GNU getopt

In D48874#1115208, @ronald_klop.ws wrote:

In D48874#1114805, @dsl wrote:

Could you add a short test plan to follow?

I hope the test plan is as expected. Otherwise please let me know.

In D10320#1111754, @glebius wrote:

https://reviews.freebsd.org/D48729

This seems like the best approach to avoid surprising users and breaking scripts.

Reviewed by: allanjude

still a WIP

In D45056#1058221, @david_crossfamilyweb.com wrote:

How should I submit this to you for the path; I have never used git format-patch before; I tried doing git format-patch main and it generated 21 files (since I have 21 commits). Manpage suggests I email this to you? But it seems rude to just blast you with 21 patches; and I don't know your email.

In D46069#1050289, @fuz wrote:

Wouldn't it be much simpler to just keep a pointer to the domain part around rather than moving it to the beginning of the domain array?

I kind of feel that '--no-follow' is the more common name, but not that fussed.

ZFS bits look good to me

The warning has been there long enough, and an error is better than doing the wrong thing, it has far fewer side effects.

In D45537#1039434, @delphij wrote:

The most important change was to add support of TLS, which is #ifdef'ed out and the rest of change is not quite worth the effort IMHO.

It would be nice to have something that implements libtls APi, e.g. libretls first.

Fix usage() alignment

Looks good, just the SPDX and the date need fixing.

Are you sure it is safe to drop the __weak_reference() bits at the bottom? They relate to the fact that there are conflicts between libmd and libcrypt (not to be confused with libcrypto)

That was committed as https://reviews.freebsd.org/D42526 / 80044c785cb040a2cf73779d23f9e1e81a00c6c3