Merge lp://staging/~clint-fewbar/ubuntu/natty/upstart/add-serial-console into lp://staging/ubuntu/natty/upstart

Hi Clint,

This merge request only consists of a changelog entry?

Do you think it reasonable that we parse the console= line in Upstart
itself, and make the value of that available to jobs that want it.
What kind of consoles do we want to support? Do we want to support
multiple console= values? How does the kernel interpret this field
already?

I think this is a discussion you should start on the upstart mailing
list, because it seems like something we need "smart" handling for
built-in.

Scott

On Fri, 2011-01-14 at 10:04 +0000, Scott James Remnant wrote:
> Hi Clint,
>
> This merge request only consists of a changelog entry?
>

I had forgotten to bzr add. The file is now listed in the MP.

>
> Do you think it reasonable that we parse the console= line in Upstart
> itself, and make the value of that available to jobs that want it.
> What kind of consoles do we want to support? Do we want to support
> multiple console= values? How does the kernel interpret this field
> already?
>

I love that idea.

> I think this is a discussion you should start on the upstart mailing
> list, because it seems like something we need "smart" handling for
> built-in.
>

Sounds good, I'll take it up there.

Hi Guys,

From reviewing the following...

http://www.kernel.org/doc/Documentation/kernel-parameters.txt
http://www.kernel.org/doc/Documentation/serial-console.txt

...and playing with my new USB setup, my comments on "debian/conf/ttyS.conf" are below:

SPECIFICS
=========

- Line 43: I think we should be checking for "console=tty*" to avoid matching when
  user specifies "console=uart...".

- Line 45: Might be simpler to write this as:

  [ ! -z `echo $arg | egrep "tty([0-9]+)"` ] && exit 0

- Line 52: Again, simpler to write as (same logic applies for lines 62,63,65):

  [ -f /etc/init/$PORT.conf ] && exit 0

- Line 54: Assumption that parity always specified as 'n'.

- Line 63: If flow control specified, BITS could be set to something like "8r" which causes
  this test to fail.

- Line 68: Missing ";;" denoting end of 'console=*)' case statement (appears to be non-fatal).

GENERICS
========

- Is it a reasonable assumption that we only consider a single serial console device?

- As mentioned in comment on Line 54 above, we are not parsing the options ("bbbbpnf")
  correctly. If I boot with the following, the script will fail:

    console=ttyUSB0,115200n8r

- I wonder if we should name the conf file "tty-serial.conf" rather than "ttyS.conf"
  to recognize that users might not be using serial device /dev/ttyS* but also /dev/ttyUSB*?

On Wed, Jan 26, 2011 at 10:06:14AM -0000, James Hunt wrote:
> - Line 45: Might be simpler to write this as:
>
> [ ! -z `echo $arg | egrep "tty([0-9]+)"` ] && exit 0

Drive-by shell style nitpicking - I'd say this instead:

  echo "$arg" | egrep -q 'tty([0-9]+)' && exit 0

(i.e. use exit status rather than testing contents.)

> - I wonder if we should name the conf file "tty-serial.conf" rather than "ttyS.conf"
> to recognize that users might not be using serial device /dev/ttyS* but also /dev/ttyUSB*?

One thing to bear in mind with all of this is that debian-installer
already dynamically creates /etc/init/tty*.conf when you install on a
serial console, and the file name it uses comes straight from the device
name. Using /etc/init/ttyS.conf would certainly conflict with that in
an interesting way, which indeed suggests to me that something like
tty-serial.conf would be better, but we also ought to think about how
the whole thing plays together; maybe the installer code should be
obsoleted, but there's a fair bit of intelligence in there particularly
for some of the more obscure systems. In particular there are some
systems where the installer automatically sets up a serial console even
if you don't say console=.

Have a look at finish-install.d/90console in lp:finish-install.

>
> - Line 45: Might be simpler to write this as:
>
> [ ! -z `echo $arg | egrep "tty([0-9]+)"` ] && exit 0
>
this will definitely not work for things like ttyAMR0 or ttyO2 (as imx5x or omap nowadays use them for example)