[PATCH 0/5] SERIAL_CONSOLES improvements

[PATCH 0/5] SERIAL_CONSOLES improvements

[Paul Eggleton]

Having two variables to enable essentially the same behaviour isn't
really desirable, so fix up the handling of SERIAL_CONSOLES, set it
from SERIAL_CONSOLE by default if not set and change everwhere we read
SERIAL_CONSOLE to read from SERIAL_CONSOLES instead. This should allow
us to deprecate SERIAL_CONSOLE whilst retaining compatibility with
machine configurations that still use it. 

Note: I'm a bit concerned about the default setting of SERIAL_CONSOLE in
systemd-serialgetty; I'm not sure why this is needed given that this is
almost always going to be machine-specific. I've left it alone for the
time being however.


The following changes since commit e86538db13605953465aacc3f2fb7d719dad919e:

  neard: add systemd unit file (2013-04-29 15:31:39 +0100)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib paule/serial-consoles
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=paule/serial-consoles

Paul Eggleton (5):
  sysvinit-inittab: ensure unique label for SERIAL_CONSOLES entries
  bitbake.conf: set SERIAL_CONSOLES from SERIAL_CONSOLE by default
  sysvinit-inittab: remove specific handling of SERIAL_CONSOLE
  shadow-securetty: change to handle SERIAL_CONSOLES
  systemd-serialgetty: change to handle SERIAL_CONSOLES

 meta/conf/bitbake.conf                             |  3 ++
 meta/recipes-core/systemd/systemd-serialgetty.bb   | 45 +++++++++++-----------
 .../sysvinit/sysvinit-inittab_2.88dsf.bb           | 11 ++----
 .../shadow/shadow-securetty_4.1.4.3.bb             | 20 ++++++----
 4 files changed, 42 insertions(+), 37 deletions(-)

-- 
1.8.1.2

[PATCH 1/5] sysvinit-inittab: ensure unique label for SERIAL_CONSOLES entries

[Paul Eggleton]

The label field in /etc/inittab entries needs to be unique, and the
numeric label being used for the SERIAL_CONSOLES getty entries was
clashing with the entries added for standard ttyX entries added via
SYSVINIT_ENABLED_GETTYS. Use the part after "tty" in the device name
(which is what the comment further down explicitly says should be done)
as the label rather than a simple incrementing number.

Fixes [YOCTO #4374].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb b/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
index b289853..fbac893 100644
--- a/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
+++ b/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
@@ -21,14 +21,12 @@ do_install() {
         echo "S:2345:respawn:${base_sbindir}/getty ${SERIAL_CONSOLE}" >> ${D}${sysconfdir}/inittab
     fi
 
-    idx=0
     tmp="${SERIAL_CONSOLES}"
     for i in $tmp
     do
 	j=`echo ${i} | sed s/\;/\ /g`
-	echo "${idx}:12345:respawn:${base_sbindir}/getty ${j}" >> ${D}${sysconfdir}/inittab
-
-	idx=`expr $idx + 1`
+	label=`echo ${i} | sed -e 's/^.*;tty//'`
+	echo "$label:12345:respawn:${base_sbindir}/getty ${j}" >> ${D}${sysconfdir}/inittab
     done
 
     if [ "${USE_VT}" = "1" ]; then
-- 
1.8.1.2

[PATCH 2/5] bitbake.conf: set SERIAL_CONSOLES from SERIAL_CONSOLE by default

[Paul Eggleton]

This is the first step in deprecating SERIAL_CONSOLE without affecting
machine configurations that still use it.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/conf/bitbake.conf | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 075f812..d5e923b 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -745,6 +745,9 @@ COMBINED_FEATURES = "\
     ${@base_both_contain("DISTRO_FEATURES", "MACHINE_FEATURES", "wifi", d)}"
 COMBINED_FEATURES[vardeps] += "DISTRO_FEATURES MACHINE_FEATURES"
 
+SERIAL_CONSOLE ??= ""
+SERIAL_CONSOLES ??= "${@d.getVar('SERIAL_CONSOLE', True).replace(' ', ';')}"
+
 # Make sure MACHINE isn't exported
 # (breaks binutils at least)
 MACHINE[unexport] = "1"
-- 
1.8.1.2

Re: [PATCH 2/5] bitbake.conf: set SERIAL_CONSOLES from SERIAL_CONSOLE by default

[Khem Raj]

On Apr 30, 2013, at 9:52 AM, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:

> This is the first step in deprecating SERIAL_CONSOLE without affecting
> machine configurations that still use it.

Did you mean deprecating SERIAL_CONSOLES or SERIAL_CONSOLE

Re: [PATCH 2/5] bitbake.conf: set SERIAL_CONSOLES from SERIAL_CONSOLE by default

[Paul Eggleton]

On Tuesday 30 April 2013 14:42:46 Khem Raj wrote:
> On Apr 30, 2013, at 9:52 AM, Paul Eggleton <paul.eggleton@linux.intel.com> 
wrote:
> > This is the first step in deprecating SERIAL_CONSOLE without affecting
> > machine configurations that still use it.
> 
> Did you mean deprecating SERIAL_CONSOLES or SERIAL_CONSOLE

I mean deprecating SERIAL_CONSOLE. SERIAL_CONSOLES is the newer replacement, 
supporting multiple consoles e.g.

SERIAL_CONSOLES = "115200;ttyS0 38400;ttyS1"

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH 2/5] bitbake.conf: set SERIAL_CONSOLES from SERIAL_CONSOLE by default

[Khem Raj]

On Apr 30, 2013, at 11:51 PM, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:

> On Tuesday 30 April 2013 14:42:46 Khem Raj wrote:
>> On Apr 30, 2013, at 9:52 AM, Paul Eggleton <paul.eggleton@linux.intel.com> 
> wrote:
>>> This is the first step in deprecating SERIAL_CONSOLE without affecting
>>> machine configurations that still use it.
>> 
>> Did you mean deprecating SERIAL_CONSOLES or SERIAL_CONSOLE
> 
> I mean deprecating SERIAL_CONSOLE. SERIAL_CONSOLES is the newer replacement, 
> supporting multiple consoles e.g.
> 
> SERIAL_CONSOLES = "115200;ttyS0 38400;ttyS1"
> 


yes thanks

[PATCH 3/5] sysvinit-inittab: remove specific handling of SERIAL_CONSOLE

[Paul Eggleton]

We already handle SERIAL_CONSOLES here and this is now set from
SERIAL_CONSOLE if not already set.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb b/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
index fbac893..23c284d 100644
--- a/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
+++ b/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
@@ -17,9 +17,6 @@ do_compile() {
 do_install() {
 	install -d ${D}${sysconfdir}
     install -m 0644 ${WORKDIR}/inittab ${D}${sysconfdir}/inittab
-    if [ ! -z "${SERIAL_CONSOLE}" ]; then
-        echo "S:2345:respawn:${base_sbindir}/getty ${SERIAL_CONSOLE}" >> ${D}${sysconfdir}/inittab
-    fi
 
     tmp="${SERIAL_CONSOLES}"
     for i in $tmp
@@ -71,7 +68,7 @@ else
 fi
 }
 
-# USE_VT and SERIAL_CONSOLE are generally defined by the MACHINE .conf.
+# USE_VT and SERIAL_CONSOLES are generally defined by the MACHINE .conf.
 # Set PACKAGE_ARCH appropriately.
 PACKAGE_ARCH = "${MACHINE_ARCH}"
 
-- 
1.8.1.2

[PATCH 4/5] shadow-securetty: change to handle SERIAL_CONSOLES

[Paul Eggleton]

SERIAL_CONSOLES is now set from SERIAL_CONSOLE if not already set. This
change also installs the file before modifying it in place, allowing
do_install to re-execute properly.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 .../shadow/shadow-securetty_4.1.4.3.bb               | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/meta/recipes-extended/shadow/shadow-securetty_4.1.4.3.bb b/meta/recipes-extended/shadow/shadow-securetty_4.1.4.3.bb
index 86b5c89..4a1f79f 100644
--- a/meta/recipes-extended/shadow/shadow-securetty_4.1.4.3.bb
+++ b/meta/recipes-extended/shadow/shadow-securetty_4.1.4.3.bb
@@ -10,22 +10,28 @@ PR = "r2"
 
 SRC_URI = "file://securetty"
 
-# Since we deduce our arch from ${SERIAL_CONSOLE}
+# Since SERIAL_CONSOLES is likely to be set from the machine configuration
 PACKAGE_ARCH = "${MACHINE_ARCH}"
 
 do_install () {
 	# Ensure we add a suitable securetty file to the package that has
 	# most common embedded TTYs defined.
-	if [ ! -z "${SERIAL_CONSOLE}" ]; then
-		# Our SERIAL_CONSOLE contains a baud rate and sometimes a -L
-		# option as well. The following pearl :) takes that and converts
+	install -d ${D}${sysconfdir}
+	install -m 0400 ${WORKDIR}/securetty ${D}${sysconfdir}/securetty
+	if [ ! -z "${SERIAL_CONSOLES}" ]; then
+		# Our SERIAL_CONSOLES contains a baud rate and sometimes extra
+		# options as well. The following pearl :) takes that and converts
 		# it into newline-separated tty's and appends them into
 		# securetty. So if a machine has a weird looking console device
 		# node (e.g. ttyAMA0) that securetty does not know, it will get
 		# appended to securetty and root logins will be allowed on that
 		# console.
-		echo "${SERIAL_CONSOLE}" | sed -e 's/[0-9][0-9]\|\-L//g'|tr "[ ]" "[\n]"  >> ${WORKDIR}/securetty
+		tmp="${SERIAL_CONSOLES}"
+		for entry in $tmp ; do
+			ttydev=`echo "$entry" | sed -e 's/^[0-9]*\;//' -e 's/\;.*//'`
+			if ! grep -q $ttydev ${D}${sysconfdir}/securetty; then
+				echo $ttydev >> ${D}${sysconfdir}/securetty
+			fi
+		done
 	fi
-	install -d ${D}${sysconfdir}
-	install -m 0400 ${WORKDIR}/securetty ${D}${sysconfdir}/securetty 
 }
-- 
1.8.1.2

[PATCH 5/5] systemd-serialgetty: change to handle SERIAL_CONSOLES

[Paul Eggleton]

SERIAL_CONSOLES is now set from SERIAL_CONSOLE if not already set.

This is a little bit gnarly because we have to create a separate service
file for each different baud rate; assume that the first baud rate is
the default, thus preserving the previous behaviour in the event there
is only one baud rate in use.

This change also installs the service file before modifying it in place,
allowing do_install to re-execute properly; additionally the service
file now has the correct permissions (i.e. no execute bit set).

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/recipes-core/systemd/systemd-serialgetty.bb | 45 ++++++++++++------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/meta/recipes-core/systemd/systemd-serialgetty.bb b/meta/recipes-core/systemd/systemd-serialgetty.bb
index 96cbf31..37c49b1 100644
--- a/meta/recipes-core/systemd/systemd-serialgetty.bb
+++ b/meta/recipes-core/systemd/systemd-serialgetty.bb
@@ -8,37 +8,38 @@ SERIAL_CONSOLE ?= "115200 ttyS0"
 
 SRC_URI = "file://serial-getty@.service"
 
-def get_serial_console_value(d, index):
-    c = d.getVar('SERIAL_CONSOLE', True)
-
-    if len(c):
-        return c.split()[index]
-
-    return ""
-
-def get_baudrate(d):
-    return get_serial_console_value(d, 0)
-
-def get_console(d):
-    return get_serial_console_value(d, 1)
-
 do_install() {
-	if [ ! ${@get_baudrate(d)} = "" ]; then
-		sed -i -e s/\@BAUDRATE\@/${@get_baudrate(d)}/g ${WORKDIR}/serial-getty@.service
+	if [ ! -z "${SERIAL_CONSOLES}" ] ; then
+		default_baudrate=`echo "${SERIAL_CONSOLES}" | sed 's/\;.*//'`
 		install -d ${D}${systemd_unitdir}/system/
 		install -d ${D}${sysconfdir}/systemd/system/getty.target.wants/
-		install ${WORKDIR}/serial-getty@.service ${D}${systemd_unitdir}/system/
-
-		# enable the service
-		ln -sf ${systemd_unitdir}/system/serial-getty@.service \
-			${D}${sysconfdir}/systemd/system/getty.target.wants/serial-getty@${@get_console(d)}.service
+		install -m 0644 ${WORKDIR}/serial-getty@.service ${D}${systemd_unitdir}/system/
+		sed -i -e s/\@BAUDRATE\@/$default_baudrate/g ${D}${systemd_unitdir}/system/serial-getty@.service
+
+		tmp="${SERIAL_CONSOLES}"
+		for entry in $tmp ; do
+			baudrate=`echo $entry | sed 's/\;.*//'`
+			ttydev=`echo $entry | sed -e 's/^[0-9]*\;//' -e 's/\;.*//'`
+			if [ "$baudrate" = "$default_baudrate" ] ; then
+				# enable the service
+				ln -sf ${systemd_unitdir}/system/serial-getty@.service \
+					${D}${sysconfdir}/systemd/system/getty.target.wants/serial-getty@$ttydev.service
+			else
+				# install custom service file for the non-default baudrate
+				install -m 0644 ${WORKDIR}/serial-getty@.service ${D}${systemd_unitdir}/system/serial-getty$baudrate@.service
+				sed -i -e s/\@BAUDRATE\@/$baudrate/g ${D}${systemd_unitdir}/system/serial-getty$baudrate@.service
+				# enable the service
+				ln -sf ${systemd_unitdir}/system/serial-getty$baudrate@.service \
+					${D}${sysconfdir}/systemd/system/getty.target.wants/serial-getty$baudrate@$ttydev.service
+			fi
+		done
 	fi
 }
 
 RDEPENDS_${PN} = "systemd"
 
 # This is a machine specific file
-FILES_${PN} = "${systemd_unitdir}/system/serial-getty@.service ${sysconfdir}"
+FILES_${PN} = "${systemd_unitdir}/system/*.service ${sysconfdir}"
 PACKAGE_ARCH = "${MACHINE_ARCH}"
 
 # As this package is tied to systemd, only build it when we're also building systemd.
-- 
1.8.1.2

Re: [PATCH 0/5] SERIAL_CONSOLES improvements

[Koen Kooi]

Op 30 apr. 2013, om 18:52 heeft Paul Eggleton <paul.eggleton@linux.intel.com> het volgende geschreven:

> Having two variables to enable essentially the same behaviour isn't
> really desirable, so fix up the handling of SERIAL_CONSOLES, set it
> from SERIAL_CONSOLE by default if not set and change everwhere we read
> SERIAL_CONSOLE to read from SERIAL_CONSOLES instead. This should allow
> us to deprecate SERIAL_CONSOLE whilst retaining compatibility with
> machine configurations that still use it. 
> 
> Note: I'm a bit concerned about the default setting of SERIAL_CONSOLE in
> systemd-serialgetty; I'm not sure why this is needed given that this is
> almost always going to be machine-specific. I've left it alone for the
> time being however.

It is always machine specific, but at the time of writing SERIAL_CONSOLE wasn't being set by all machines leading to parse failures.

regards,

Koen