Bug 433156 - rhgb details doesn't display init script output under upstart
Summary: rhgb details doesn't display init script output under upstart
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: upstart
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Casey Dahlin
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: upstart
TreeView+ depends on / blocked
 
Reported: 2008-02-17 00:34 UTC by Casey Dahlin
Modified: 2014-06-18 08:46 UTC (History)
5 users (show)

Fixed In Version: 2.86-24
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-03-12 03:32:08 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Casey Dahlin 2008-02-17 00:34:48 UTC
Description of problem:
Clicking "Show details" in rhgb with upstart installed only shows the lines
output by rc.sysinit and not any of the ones output by the individual runlevel
scripts.

How reproducible:
Every time

Steps to Reproduce:
Boot your system and click show details in rhgb
  
Actual results:
We see about 6 lines of input that remain static throughout the boot sequence

Expected results:
We get a whole bunch of lines telling us what is being started.

Comment 1 Bill Nottingham 2008-02-18 20:32:32 UTC
So, in the 'never ask how the sausage is made' department....

The way this worked with sysvinit is that rhgb would send a message to init
(using a custom message in init's control protocol) to change init's console to
a specified device (the virtual tty in rhgb.) Hence, all the scripts would be
started with this device as their console, so that's where they'd go. On exit,
rhgb would reset init so that /dev/console is init's 'console' device.

I'm sure there is a better way to accomplish this with upstart? (Because,
really, there can't be a worse way.)

Comment 2 Casey Dahlin 2008-02-18 21:44:02 UTC
giving the "console" stanza an argument to handle this somehow might work.

Comment 3 Scott James Remnant 2008-02-19 10:47:21 UTC
@Bill: You could have warned me, I was drinking that cup of tea *reaches for the
screencleaner*

I guess that the rhgb console device changes a lot?

One way you could do this is to abuse the logd code, you could have rhgb listen
on @/com/ubuntu/upstart/logd and receive the output that way -- though you'd
lose that output if rhgb wasn't run before init

Another way would to pretty much modify upstart as you modified init, I'd add a
CONSOLE_RHGB option that set the output devices to that tty

Comment 4 Casey Dahlin 2008-02-19 18:09:34 UTC
Is there a way to run rhgb as an upstart service (where does it get started from
now?) If so, we could push the tty change into upstart with independent
started/stopped events, which is marginally nicer in that at least rhgb isn't
responsible for upstart's tty.

Comment 5 Ray Strode [halfline] 2008-03-02 07:00:40 UTC
Hi,

the VTE widget in rhgb creates a pseudoterminal.  It will always (well in most
circumstances) be pts/1 since it's run so early in the boot process.  What do
you mean by @/com/ubuntu/upstart/logd ? An abstract unix domain socket? 

rhgb isn't run before init, it's started by rc.sysinit as soon as /usr is mounted.

One thing I did for plymouth is use the TIOCCONS ioctl to redirect /dev/console
to  plymouth's pts (see
http://cgit.freedesktop.org/~halfline/plymouth/tree/src/libply/ply-terminal-session.c)

That might be an alternative, too.

Comment 6 Casey Dahlin 2008-03-02 23:55:23 UTC
upstart-0.3.9-9 Just built

In it is a patch that adds two initctl commands:

initctl push-tty
initctl pop-tty

initctl push-tty /dev/somedev will set the tty used by console output jobs to
/dev/somedev. initctl pop-tty will undo the most recent push-tty.

I'm going to try to patch rc.sysinit to make this work for us.

Comment 7 Ray Strode [halfline] 2008-03-03 14:57:19 UTC
Hi Casey,

I think we either need to fix rhgb to use these new commands or have a small 

/sbin/change_console

command/shell script that runs those commands (if -r is passed in it would
pop-tty and otherwise push-tty)



Comment 8 Casey Dahlin 2008-03-03 16:03:08 UTC
Right now I've modified my /etc/rc.sysinit[1] so that instead of running rhgb
directly, it starts an upstart job for rhgb[2]. This job simply has post-start
and post-stop scripts that change the terminal. Works well, and recovers well
when rhgb craps out.

This did require a small patch to rhgb[3], which simply adds a --no-daemon flag
to keep it from forking so upstart can keep ahold of it.

The sleep 5s in the rc.sysinit is a kludge to get around me not knowing how to
operate upstart properly :) After talking to Scott I now know what was going
wrong and how not to do that (I wasn't going to let it be shipped that way I
promise :) In other words, neither the job nor the rc.sysinit changes are final.
The rhgb patch does what its supposed to do, but code review welcome :) (Not
that there's much to it, but still it felt a bit ugly somehow).

[1] http://sadmac.fedorapeople.org/rc.sysinit.use-upstart.patch
[2] http://sadmac.fedorapeople.org/rhgb
[3] http://sadmac.fedorapeople.org/rhgb-0.17.7-nodaemon-option.patch

Comment 9 Bill Nottingham 2008-03-03 16:47:50 UTC
I am somewhat saddend by pushing the same 'change console device' hack into
upstart. That's just wrong.

Not that I have a better idea right this second.

Comment 10 Casey Dahlin 2008-03-03 17:11:23 UTC
Its a bit better this time, in that its more failure proof, and also it is only
the rc output that goes to the new console (upstart's own console remains
/dev/console

Comment 11 Bill Nottingham 2008-03-03 18:04:21 UTC
Would it be possible to fix upstart so that we can specify logd for most jobs
and have output go to *both* the console and logd, and then rhgb could just pipe
from logd?

Comment 12 Casey Dahlin 2008-03-03 18:43:36 UTC
The way to do that would be to have logd itself forward the output to the
console. With logd still disabled and problematic, not sure if we have many
options there.

Comment 13 Casey Dahlin 2008-03-03 19:16:42 UTC
Looking at this again, I think I'm just going to try and patch rhgb so that it
will symlink its terminal to /etc/rhgb/temp/rhgb-pts (why does rhgb have its own
temp folder in /etc?) and just have `console output` autodetect the presence of
that link and use it if it is there.

Comment 14 Bill Nottingham 2008-03-03 20:12:13 UTC
The reason it would be nice to fix logd is that it would solve both this and the
open 'initscripts output is not logged' bug simultaneously.

Comment 15 Ray Strode [halfline] 2008-03-03 20:17:09 UTC
rhgb creates a named pipe called /etc/rhgb/temp/rhgb-console that you can write
to which feeds into the vte widget.

Honestly, i think implementing a change_console wrapper and not touching rhgb is
probably the least invasive fix.  The plan for F9 was to put upstart in place w/
a compat layer to keep it as close to a drop in replacement as possible, right?

You're patch is probably okay instead of the change_console wrapper, except
/dev/pts/1 won't exist until rhgb is started, so there is a race.

Comment 16 Ray Strode [halfline] 2008-03-03 20:20:02 UTC
Hi Bill,

Those are two distinct issues though.  Getting feature parity with F8 should be
higher priority than introducing a new feature.

I mean if it's not hard to kill both birds, all the better, but given all the
other changes going in, it might make sense to punt on the small fish and come
up with better solutions for those types of issues once the dust has settled
from the big changes...

Comment 17 Casey Dahlin 2008-03-03 20:27:51 UTC
Patching rhgb to symlink its pty won't work, because the widget doesn't know or
doesn't tell which pty its using. The rhgb-console pipe works, but the output
gets de-terminalized and doesn't look right.

I just updated these links. Its... ugly. The post-start has... well I don't like
it one bit. But it does fix the bug.

http://sadmac.fedorapeople.org/rc.sysinit.use-upstart.patch
http://sadmac.fedorapeople.org/rhgb

Looking at this the change-console thing doesn't seem too bad. Though getting
rhgb into a job definition is nice.

Comment 18 Ray Strode [halfline] 2008-03-03 20:44:45 UTC
still probably better not to hard code /dev/pts/0 or whatever, and instead let
rhgb explicitly tell upstart which pty to use.  I mean in theory, it shouldn't
ever change, but relying on it is a bit dicey...

What advantages do we get by making it an upstart job, btw?

Comment 19 Casey Dahlin 2008-03-03 21:36:01 UTC
I agree that relying on /dev/pts/0 is bad, but rhgb has as much info as we do.
There's no way at all to get the tty out of the vte-terminal.

What advantages don't you gain by making it an upstart job? 8D

Seriously though, it means upstart stays more or less in control of its own tty,
and it gives us a foothold in rc.sysinit (which I hope will disappear in the F10
cycle).

Comment 20 Casey Dahlin 2008-03-03 21:40:49 UTC
On a further note, I now have a version that uses inotify-tools, which takes
care of the busy wait.

Comment 21 Ray Strode [halfline] 2008-03-04 02:07:27 UTC
vte_terminal_fork_command sets the child's stdin/stdout/stderr to the pts it
creates.  change_console detects which tty it's running under and does the right
thing.

In HEAD rhgb actually creates its own pty and tells vte about it, so it knows
directly which pty to use.

Anyway, I don't really care what we do.  rhgb is a big hack that only survived
F9 because the giant GDM rewrite has taken up so much of my time.  It will be
gone before F10.

Comment 22 Casey Dahlin 2008-03-04 02:11:54 UTC
Well, in that case, we do have a solution. Do you mind depending on
inotify-tools (its a small package)? I think the event should be packaged with
rhgb. If you have no objections to that or the patch, and if notting will take
the rc.sysinit patch, we can close this up.

Comment 23 Casey Dahlin 2008-03-11 03:02:20 UTC
Here's my current rhgb event:

# rhgb - Red Hat Graphical Boot
#
# Boots in a shiny X server

service

pre-start exec test -x /usr/bin/rhgb

post-start script
        if [ ! -e /dev/pts/0 ]; then
                /usr/bin/inotifywait -t 5 /dev/pts 2> /dev/null | grep -m 1
"CREATE" > /dev/null
        fi
        /sbin/initctl push-tty /dev/pts/0
end script

script
        . /etc/sysconfig/i18n
        exec /usr/bin/rhgb --no-daemon
end script

post-stop script
        /sbin/initctl pop-tty
end script


All works well with this. I do have a small binary app to replace the
conditional and thus remove the race condition if that is an issue.

Comment 24 Ray Strode [halfline] 2008-03-11 14:02:31 UTC
Okay, feel free to add it to rhgb and build it.

Comment 25 Ray Strode [halfline] 2008-03-11 14:08:50 UTC
Bill, are you okay with the rc.sysinit patch?

Comment 26 Bill Nottingham 2008-03-11 15:21:51 UTC
It's reasonable - let me know when rhgb is built and I'll poke initscripts.

Comment 27 Casey Dahlin 2008-03-11 17:19:46 UTC
Built

Comment 28 Bill Nottingham 2008-03-11 19:29:58 UTC
Added in 8.66-1.

Comment 29 Bill Nottingham 2008-03-11 19:58:01 UTC
Reopening. This appears to make rhgb not start at all.

Comment 30 Bill Nottingham 2008-03-11 20:24:58 UTC
Moreover, even when convinced to make rhgb start, it doesn't actually solve this
bug.

Comment 31 Casey Dahlin 2008-03-11 21:14:08 UTC
I've got it all working. I may have to wipe my rawhide box and reinstall so I
can make sure I didn't miss any bits.

Comment 32 Bill Nottingham 2008-03-11 21:36:51 UTC
With  more testing, simply replacing /sbin/change_console (from sysvinit-tools)
with a wrapper that calls 'initctl push-tty' makes the right thing happen,
modulo some SELinux fixes.

With that change, we wouldn't need to change rhgb or initscripts at all.

Comment 33 Casey Dahlin 2008-03-11 21:47:13 UTC
When rstrode mentioned using change_console, I wasn't aware the faculty was
already in place. That is simpler. Lets roll 'em back then.

Comment 34 Bill Nottingham 2008-03-12 03:32:08 UTC
OK, should be fixed in sysvinit-tools-2.86-24 *and* selinux-policy-3.3.1-14.fc9.


Note You need to log in before you can comment on or make changes to this bug.