Feat/multi console v2 enhanced by walterchris · Pull Request #3312 · u-root/u-root

walterchris · 2025-02-28T08:34:40Z

All good things are three.

This PR brings in Multi-console output and input support.

walterchris · 2025-02-28T08:35:08Z

#3300 and #3311 should be dropped in favor of this if we are happy with it.

rminnich

overall: seems the best one yet.

rminnich · 2025-02-28T20:35:02Z

+	return func(c *exec.Cmd) {
+		if mtty {
+			ww, err := openFn(ttyNames)
+			if err != nil {


you'll need to remove the print, it's a package. You can return an error however.

It return a function - if we would want to return an error we would need to modify the mod function as well to be able to return an error. I did not wanted to change the flow here - so I'd just return.

so how about this. You want to use all the available consoles, which will fail if you return early if just one fails.

You might use errors.Join accumulate a list of errors, and accumulate your list of consoles too.

Is the problem that the caller of the modifier will quit with a non-null error?

It would be nice if you could return with
zero or more consoles, error
the caller can print the error, if it is non-nil, and use whatever consoles are returned.

This is similar to what io readers do: they can return data AND an error and let you pick what to do.

This is one of those cases where you want to know if something happened, but you want to try all the things.

I hate to see packages just print stuff to stderr because it makes it so hard to read the GitHub CI output when all these random messages appear.

I agree that it makes generally sense to catch potential errors here but that would require a larger rework as we return a CommandModifier which currently is defined as type CommandModifier func(c *exec.Cmd). We would need to modify that type to also return errors which means changing every command modifier that currently exist as well. That's why Chris didn't want to change a lot here as the scope of the PR would be much broader then.

rminnich · 2025-02-28T20:36:13Z

 	}
 }

+func Raw(r *os.File) error {


did you have to break all this out? There is a Raw function in one of the packages?
If there is not a raw function, can you consider adding this to a package, so others can use it? It's a shame to have this only in libinit.

I agree to move this out. Is was copied into the libinit package for the initial PoC but for this PR it should be moved to an appropriate place. Will do so.

rminnich · 2025-02-28T20:43:35Z

+						_, err := io.Copy(m, t)
+						if err != nil {
+							log.Printf("Error copying output from command to PTM: %v", err)
+						}


you might consider passing in an io.Writer for the package to log errors to, I can see where this kind of logging might be useful, but just directly printing like this can really mess up output.

rminnich · 2025-03-10T20:26:45Z

+	return func(c *exec.Cmd) {
+		if mtty {
+			ww, err := openFn(ttyNames)
+			if err != nil {


so how about this. You want to use all the available consoles, which will fail if you return early if just one fails.

You might use errors.Join accumulate a list of errors, and accumulate your list of consoles too.

Is the problem that the caller of the modifier will quit with a non-null error?

It would be nice if you could return with
zero or more consoles, error
the caller can print the error, if it is non-nil, and use whatever consoles are returned.

This is similar to what io readers do: they can return data AND an error and let you pick what to do.

This is one of those cases where you want to know if something happened, but you want to try all the things.

I hate to see packages just print stuff to stderr because it makes it so hard to read the GitHub CI output when all these random messages appear.

MDr164 · 2025-03-13T15:55:17Z

#3300 and #3311 should be dropped in favor of this if we are happy with it.

I'd say the first one could go in as a stepping stone as it currently implements the multi output correctly. Jens also removed the kerneltty flag there to closer match what we planned on doing in the followup PR.

This commit enables to write from multiple consoles to the shell. This is a follow-up of the multi-writer support that enables output on all shells defined in the kernel cmdline. Signed-off-by: Christian Walter <christian.walter@9elements.com>

Signed-off-by: Christian Walter <christian.walter@9elements.com>

Signed-off-by: Marvin Drees <marvin.drees@9elements.com>

MDr164 · 2025-06-03T17:52:25Z

Looks like the PTY based console runs into the same issue you mentioned earlier today with the gosh console decorators. It sometimes renders the prompt in the middle of the screen/serial and causes odd placements. I will create another PR changing the defaults as discussed.

Signed-off-by: Marvin Drees <marvin.drees@9elements.com>

rminnich · 2025-08-15T16:48:05Z

not sure this is helpful but .... got into a good talk with brho@ re this console stuff. It came up as an issue on something else.

There is code that basically returns the equivalent of socketpair for a pty pair:

func ptsopen() (controlPTY, processTTY *os.File, ttyname string, err error) {
        p, err := os.OpenFile("/dev/ptmx", os.O_RDWR, 0)
        if err != nil {
                return
        }

        ttyname, err = ptsname(p)
        if err != nil {
                return
        }

        err = ptsunlock(p)
        if err != nil {
                return
        }

        v("OpenFile %v %x\n", ttyname, os.O_RDWR|syscall.O_NOCTTY)
        t, err := os.OpenFile(ttyname, os.O_RDWR|syscall.O_NOCTTY, 0)
        if err != nil {
                return
        }
        return p, t, ttyname, nil
}

To use it:

        c.SysProcAttr = &syscall.SysProcAttr{
                ...
                Setctty: true,
                Setsid:  true,
        }
        c.Stdout = processTTY
        c.Stdin = processTTY
        c.Stderr = c.Stdout // or if you want, I guess, processTTY
        c.SysProcAttr.Setctty = true
        c.SysProcAttr.Setsid = true
        ... anything else
        err = c.Start()

and for IO:

        go func() {
                io.Copy(os.Stdout, controlPTY)
                ... cleanup if EOF on controlPTY
        }()
        io.Copy(controlPTY, os.Stdin)

This is from cmds/exp/pflask, it works, the shell thinks it's on a tty, etc.

This is what I think you need to correctly set up the multi console environment.

MDr164 · 2026-01-13T17:22:54Z

Currently cleaning this up here: https://github.com/9elements/u-root/tree/linuxboot-fix and updating this PR once it works under multiple different environments.

walterchris force-pushed the feat/multi-console-v2-enhanced branch 2 times, most recently from bdb21c2 to 2f37961 Compare February 28, 2025 09:26

rminnich reviewed Feb 28, 2025

View reviewed changes

rminnich reviewed Mar 10, 2025

View reviewed changes

walterchris added 2 commits June 3, 2025 12:39

feat: enable multi-reader support

fa03064

This commit enables to write from multiple consoles to the shell. This is a follow-up of the multi-writer support that enables output on all shells defined in the kernel cmdline. Signed-off-by: Christian Walter <christian.walter@9elements.com>

fix: clean up libinit code

bea1c46

Signed-off-by: Christian Walter <christian.walter@9elements.com>

MDr164 force-pushed the feat/multi-console-v2-enhanced branch from 50f4305 to ffebd3f Compare June 3, 2025 14:36

MDr164 marked this pull request as ready for review June 3, 2025 14:39

MDr164 mentioned this pull request Jun 3, 2025

pkg/libinit: skip Multiwriter with only 1 console #3394

Closed

libinit: do not use multiwriter with single tty

f55983b

Signed-off-by: Marvin Drees <marvin.drees@9elements.com>

MDr164 force-pushed the feat/multi-console-v2-enhanced branch from ffebd3f to f55983b Compare June 3, 2025 17:50

multi-console: move code into correct packages

091bb90

Signed-off-by: Marvin Drees <marvin.drees@9elements.com>

Conversation

walterchris commented Feb 28, 2025

Uh oh!

walterchris commented Feb 28, 2025

Uh oh!

rminnich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MDr164 commented Mar 13, 2025

Uh oh!

MDr164 commented Jun 3, 2025

Uh oh!

rminnich commented Aug 15, 2025

Uh oh!

MDr164 commented Jan 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants