more direct call

In D35309#800313, @asomers wrote:

In D35309#800308, @imp wrote:

So long as we don't have two disks with that label, we'll be fine...
So this is better than what we had, but not quite to where we need to get to....
Though the functionality we need to get there isn't in bsdinstall...

Well, when using a mirrored setup at least bsdinstall creates a /dev/gpt/efiboot0 and a /dev/gpt/efiboot1. What functionality do you think we need?

So long as we don't have two disks with that label, we'll be fine...
So this is better than what we had, but not quite to where we need to get to....
Though the functionality we need to get there isn't in bsdinstall...

This is fine. armv6 likely has a more limited time frame than armv7 and it certainly has a much smaller audience.
Having said that, this may help unless we wanted to opt for the more intrusive bsd.cpu.mk idea. We did it for MIPS, but there were always issues with that for edge cases, users of gmake not picking it up, etc.

I also wonder if there's precedent for sharing code with the loader like that. Should it perhaps be moved to some common area, or should I just set .PATH=stand/libsa/zfs and not worry about it?

Shouldn't you reach over into the boot loader to get the shared files?
If there's some reason you can't do this, it should be documented and/or discussed.

Halting the machine is still important in 2022. A number of people want to power it off afterwards, though sometimes the automatic power is busted.

I think we're good. Thanks

Landed, but not automatically closed.

Yea, I stole the wording from one of the geom man pages.
I'll update. I accidentally pushed this too soon, but I'll update with this review.

Generally, I like this. Couple of small nits, sorry for the delay.

In D35141#796580, @rpokala wrote:

Nice! (How did I not know that kern.msgbuf_show_timestamp was a thing?!?)

Seems legit

This looks good. normally fixfp_t is 32-bit int, which suggests that there may be other place we need to widen.
And updating the comment with the new limit would be good (~2 million if I'm doing the math right)...

In D34977#795715, @mav wrote:

The only case I see AHCI to return CAM_SEL_TIMEOUT is:

 if (ch->devices == 0 ||
    (ch->pm_present == 0 &&
     ccb->ccb_h.target_id > 0 && ccb->ccb_h.target_id < 15)) {
        ccb->ccb_h.status = CAM_SEL_TIMEOUT;
        break;
}

, which means device is not detected. I don't see how it can be transient and why it should be retried.

closed with cc1572ddeb8cd82879ce0cca634bf6a8830c0f40 with updated wording from @chs but didn't close automatically since i didn't notice git arc stage didn't put a review in.

Address the libstand comment.

In D35102#795578, @tsoome wrote:

There is this nagging question however - should we check the return value from snprintf (like in this last instance). We do know those strings should fit (normally), but if there is non normal case of memory corruption...

I believe the new code is the same safety as the old if there's no bugs, and safer if there are bugs.
Discuss and/or prove me wrong.

unix-linux upstream pull request
https://github.com/util-linux/util-linux/pull/1670

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=258544 contains the exp run information.

Update comments from review suggestiosn, and hopefully some additional verbosity
to disambiguate other possible readings not flagged in earlier reviews.

pivot a little

In D35057#794539, @imp wrote:

In D35057#794531, @mav wrote:

async handlers are called only for the events explicitly specified to xpt_register_async(). As such, every periph driver specifies there AC_LOST_DEVICE, handled by cam_periph_async(). It means every driver has to know what cam_periph_async() can actually handle and how better call it. In such situation "default" case (calling always) can be useful only to not explicitly list new event types sometimes in the future, that may actually be more confusing.

So should I add AC_LOST_DEVICE as an explicit case here? I'm sorry, but I'm having trouble understanding your point...

In D35038#794530, @mav wrote:

Are you going to do the same for SCSI to not diverge them?

In D35057#794531, @mav wrote:

async handlers are called only for the events explicitly specified to xpt_register_async(). As such, every periph driver specifies there AC_LOST_DEVICE, handled by cam_periph_async(). It means every driver has to know what cam_periph_async() can actually handle and how better call it. In such situation "default" case (calling always) can be useful only to not explicitly list new event types sometimes in the future, that may actually be more confusing.

In D35040#794429, @mav wrote:

While I see no problem with this patch, I am not sure it is not an obfuscation. The same checks are done (or should be done) at every probe stage. The check seems to be missing on transition from ADA_CCB_RAHEAD to ADA_STATE_WCACHE. I think "goto skipstate" logic of dastart() could be more universal.