Re: Multiple console support in d-i
Hey Wookey,
Reviewing the code from your patches in sequence, I think the approach
looks good *except* I think you've missed a patch or a commit or
something.
Trying to apply debian-installer-multiple-consoles.patch and
rootskel-multiple-consoles-inittab.patch in sequence, there are patch
failures. They clearly depend in that order for the changes in
src/etc/inittab-linux, but the src/sbin/reopen-console-linux hunks
don't match up. The last thing I want to do here is miss a line in the
middle of fixing up by hand and break this... :-)
Could you post a single patch against current HEAD with all of your
changes rolled up please?
Looking through your rationale below...
On Sat, Feb 16, 2019 at 02:19:41AM +0000, Wookey wrote:
>On 2019-02-09 04:11 +0000, Wookey wrote:
>> Future work:
>>
>> All this faffage has made me realise that a better approach to all
>> this would probably be to get rid of all this 'steal-ctty' bodgery,
>> and use init to do it's job: it's designed to run multiple things on
>> multiple consoles, and deal with file handles and them failing etc.
>>
>> The catch is that to make this work we'd have to use sysinit: to run
>> the console-choosing code, then write a new inittab containing one
>> respawn:/sbin/debian-installer for each console instance, then HUP
>> init. This should do exactly the right thing (assuming that busybox
>> init isn't too thick to get HUPing right).
>
>OK. This does indeed work nicely and is a cleaner solution than what
>is currently in D-I. It probably works for hurd and bsd too assuming
>their init behaviour is similar? (I think kill -HUP 1 should do the
>same thing on all 3 kernels?) (anyone up for testing these?)
Awesome! \o/ I'm glad this worked out for you.
"kill -HUP 1" is a standard pattern for telling init to re-read the
inittab, I think.
>So now inittab looks like:
># main rc script
>::sysinit:/sbin/reopen-console /sbin/debian-installer-startup
>
># main setup program
>tty0::respawn:/sbin/debian-installer
>ttyAMA0::respawn:/sbin/debian-installer
>(these lines added for whatever consoles are found)
>
>(and the rest as before - spare shells, tty4 for logging and restart stuff)
>
>Attached is a patch implementing this. It does rely on the inittab
>comment '# main setup program' existing in order to anchor the sed
>edit, so perhaps a comment should go in there so someone doesn't
>accidentally edit it in the future and break this code? Or we could
>just use a simple append (it's not aesthetic just putting them on the
>end but it does work fine), or just 'insert at line n'. Given the
>constrained environment I don't think it matters much -
>maintainability is probably the most important thing here.
To be honest, I think you're obsessing about ordering too much
here. Ordering of things in inittab should not matter for things
spawned on separate terminals. You could simplify things here a lot by
simply appending each of your console-specific startup lines to
/etc/inittab and not trying to put them in the middle!
>Steal-ctty is still present for the initial run of the
>debian-install-startup scripts but I suspect it's not actually
>needed. Anyone know if there was ever a good reason for running
>the rc scripts through this as well as doing so for the installer itself?
Pass. *Maybe* to enable it to do one-time initialisation of the
terminal? e.g. to do UTF-8 configuration like Ben mentions in his
followup...
>This all works with netbook-gtk too (X starts on framebuffer, as
>before)
Awesome! With a clean patch to apply I'll be able to build and test
things running on x86 bare metal and another different arm64 machine
here.
>Note that my finish-install patch should be applied too if this one is used.
Please post together in the same mail!
>Issues:
>The framebuffer console came up with some UTF-8 chars as blocks (ones
>not in 8859-1?). I've seen this before once with the old code then it
>went away again, so I'm not sure it's anything to do with these
>changes but it might be. The LANG=C.UTF-8, TERM=linux,
>TERM_TYPE=virtual, TERM_FRAMEBUFFER=yes, which seems reasonable. Clues
>welcome. fonts or TERM configuration?
Ben's responded to this.
>Further work:
>There is more tidying that could be done here, but some discussion is
>in order first.
>
>With things done this way 'reopen-console' is something of a misnomer
>as it only gets run once. 'choose-console' would be better. Or
>possibly something like 'initialise-installer' as it now chooses the
>consoles, calls the init-script runner and reinits to start the real
>installer. Perhaps a more logical split is
>
>choose-consoles: (OS-specific)
> 1) parses consoles, saves in /var/run files
1a) configure consoles (if needed)
> 2) runs debian-installer-startup
>
>debian-installer-startup: (OS-independent)
> 1) modifies inittab
> 2) runs startup scripts
> 3) HUPs init
I think this makes sense to me.
>This seems a bit cleaner and better-named? Also is there any point
>having choose-console run $@ now there is only one thing it runs
>(debian-installer-startup)?
>
>The fly in this pointment is that there is one script that uses the
>existing /sbin/debian-installer-startup. That is
>debian-installer-launcher/debian-installer.sh which runs it in a live
>image chroot so moving the restart init there would be an unexpected change
>of interface. The desire to leave that interface alone results in this:
To the best of my knowledge, debian-installer-laucnher is currently
not used on our (official) live media anyway, due to longstanding bugs
that nobody ever tried to fix. I wouldn't let that block you here.
>choose-consoles: (OS-specific)
> 1) parses consoles, saves in /var/run files
> 2) modifies inittab
> 3) runs debian-installer-startup
> 4) HUPs init
>
>debian-installer-startup: (OS-independent)
> 1) runs startup scripts
>
>Which is essentially the existing patch, but renaming reopen-console -> choose-consoles
OK. Again, I think this also looks sane.
>currently we have:
>::sysinit:/sbin/reopen-console /sbin/debian-installer-startup
>
>but I suggest we just change it to:
>::sysinit:/sbin/choose-consoles
>and choose consoles can explicitly run /sbin/debian-installer-startup
>
>This just makes it a bit more obvious how things work/fit together.
>
>Have I missed anything?
>
>Does anyone care about all this? Shall I just stop now (it's working
>fine) or tidy a bit more to make the names clearer and reduce the
>cruft further?
As I said, I'm happy to test a clean rolled-up set of patches, one
per d-i "package". If that works and looks OK, I say "go for it" but
we need to be it ASAP.
--
Steve McIntyre, Cambridge, UK. steve@einval.com
"Arguing that you don't care about the right to privacy because you have
nothing to hide is no different than saying you don't care about free
speech because you have nothing to say."
-- Edward Snowden
Reply to: