Page MenuHomeFreeBSD

bsdinstall: Add a new runconsoles helper binary
ClosedPublic

Authored by jrtc27 on Sep 29 2022, 9:05 PM.
Tags
None
Referenced Files
Unknown Object (File)
Nov 12 2024, 4:14 PM
Unknown Object (File)
Nov 12 2024, 3:32 PM
Unknown Object (File)
Nov 10 2024, 8:44 PM
Unknown Object (File)
Nov 9 2024, 6:01 PM
Unknown Object (File)
Nov 9 2024, 6:01 PM
Unknown Object (File)
Nov 9 2024, 6:01 PM
Unknown Object (File)
Nov 9 2024, 6:01 PM
Unknown Object (File)
Nov 6 2024, 8:26 AM
Subscribers

Details

Summary

This helper binary will run a given command on every on console, as
defined by /etc/ttys (except for ttyv*, where only ttyv0 will be used).
If one of the command processes exits, the rest will be killed. This
will be used by a future change to start the installer on multiple
consoles.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

brooks added a subscriber: brooks.

Not sure I've followed all the signal details, but I understand the basic design and it makes sense to me.

This revision is now accepted and ready to land.Sep 29 2022, 10:20 PM

This looks decent or better... I need to study to find more problems other than the one trivial one so far.

usr.sbin/bsdinstall/runconsoles/common.h
3

Nit: This is BSD-2-Clause, not BSD-2-Clause-FreeBSD

everything else looks good. I think I'm with Brooks: I didn't game out all scenarios, but the one I did were spot on.

usr.sbin/bsdinstall/runconsoles/child.c
68

Is procctl async safe? I think it is since it's a system call, but couldn't find it in the lists.

usr.sbin/bsdinstall/runconsoles/child.c
68

Yeah it's just a normal generated wrapper around the system call so there's not going to be anything unsafe about it, certainly no userspace state to have issues with. The list in sigaction(2) doesn't see much love; there are a lot of system calls that aren't in there but async-signal safe. The table lists kill(2) but not killpg(2), which is just a trivial wrapper around kill(2), as another example. According to the manage you can also read(2) but not pread(2), which is clearly also silly. The list goes on... bit sad but it is what it is.

usr.sbin/bsdinstall/runconsoles/common.h
3

I think I copied this mistake from one of bsdinstall's distfoo.c files...

% git grep -l BSD-2-Clause-FreeBSD | xargs grep -L 'The views and conclusions contained in the software' | wc -l
5480

which isn't much less than

% git grep -l BSD-2-Clause-FreeBSD | wc -l
5542

:(

BSD-2-Clause not -FreeBSD, despite ~everything in the tree getting this wrong

This revision now requires review to proceed.Sep 30 2022, 1:17 AM
This revision is now accepted and ready to land.Sep 30 2022, 3:50 PM

So we run multiple copies of the installer, and kill all of them when one finishes? I'd have a slight worry about someone "starting over" on a different tty than the one they started on, but I'd still think this is a usability improvement over the status quo

So we run multiple copies of the installer, and kill all of them when one finishes? I'd have a slight worry about someone "starting over" on a different tty than the one they started on, but I'd still think this is a usability improvement over the status quo

Correct. I guess if you really wanted to you could wrap the bsdinstall call in startbsdinstall with lockf(1), but that still wouldn't stop users shooting themselves in the foot by starting the installer on one and selecting Live CD on another which would kill the former.

For what it's worth, Debian's installer does the same thing of spawning on all consoles and not having interlocks if you try and use both to install the system (other than some of the tools it runs as part of that, i.e. apt/dpkg, during the odd step). As far as I know that's not been a problem in the many years it's done that.

My concern about starting again on a second instance of the installer is mostly theoretical; I'm very happy to see usability improvements in our installer story and I think this is an improvement over what happens today, so have no objection to the approach.