Openembedded Core Discussions
 help / color / mirror / Atom feed
* [PATCH] sysvinit-inittab: support non-busybox-getty on serial consoles
@ 2019-02-08 12:02 André Draszik
  2019-02-08 14:07 ` Richard Purdie
  0 siblings, 1 reply; 4+ messages in thread
From: André Draszik @ 2019-02-08 12:02 UTC (permalink / raw)
  To: openembedded-core

From: André Draszik <andre.draszik@jci.com>

Busybox' getty has code to try to make itself a session leader,
whereas util-linux' agetty doesn't. It expects this to happen
from outside.
When getty is not a session leader, many things don't work on
the serial console, e.g. setting the terminal process group,
job control doesn't work, etc.

Executing image tests also fails with AssertionErrors, because
    Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot get controlling tty: Operation not permitted
    Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot set process group: Inappropriate ioctl for device

Update the start_getty script to invoke getty via the setsid
utility if needed, i.e. if /sbin/getty is not busybox getty.

[YOCTO #13058]

Signed-off-by: André Draszik <andre.draszik@jci.com>
---
 meta/recipes-core/sysvinit/sysvinit-inittab/start_getty | 9 ++++++++-
 meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb  | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
index e15ae35f90..ae613f1e54 100644
--- a/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
+++ b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
@@ -13,6 +13,13 @@ active_serial=$(grep "serial" /proc/tty/drivers | cut -d/ -f1 | sed "s/ *$//")
 # Rephrase input parameter from ttyS target index (ttyS1, ttyS2, ttyAMA0, etc).
 runtime_tty=$(echo $2 | grep -oh '[0-9]')
 
+# busybox' getty does this itself, util-linux' getty needs extra help
+getty="/sbin/getty"
+if ! basename $(readlink -f "${getty}") | grep -q busybox \
+    && [ -x "/usr/bin/setsid" ] ; then
+    getty="/usr/bin/setsid ${getty}"
+fi
+
 # Backup $IFS.
 DEFAULT_IFS=$IFS
 # Customize Internal Field Separator.
@@ -31,7 +38,7 @@ for line in $active_serial; do
 			then
 				if [ -c /dev/$2 ]
 				then
-				    /sbin/getty -L $1 $2 $3
+				    eval ${getty} -L $1 $2 $3
 				fi
 				break
 			fi
diff --git a/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb b/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
index 8585a418ab..bfd890d924 100644
--- a/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
+++ b/meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb
@@ -20,6 +20,7 @@ do_install() {
     install -m 0644 ${WORKDIR}/inittab ${D}${sysconfdir}/inittab
     install -d ${D}${base_bindir}
     install -m 0755 ${WORKDIR}/start_getty ${D}${base_bindir}/start_getty
+    sed -e 's,/usr/bin,${bindir},g' -i ${D}${base_bindir}/start_getty
 
     set -x
     tmp="${SERIAL_CONSOLES}"
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] sysvinit-inittab: support non-busybox-getty on serial consoles
  2019-02-08 12:02 [PATCH] sysvinit-inittab: support non-busybox-getty on serial consoles André Draszik
@ 2019-02-08 14:07 ` Richard Purdie
  2019-02-08 16:55   ` André Draszik
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Purdie @ 2019-02-08 14:07 UTC (permalink / raw)
  To: André Draszik, openembedded-core

On Fri, 2019-02-08 at 12:02 +0000, André Draszik wrote:
> From: André Draszik <andre.draszik@jci.com>
> 
> Busybox' getty has code to try to make itself a session leader,
> whereas util-linux' agetty doesn't. It expects this to happen
> from outside.
> When getty is not a session leader, many things don't work on
> the serial console, e.g. setting the terminal process group,
> job control doesn't work, etc.
> 
> Executing image tests also fails with AssertionErrors, because
>     Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot get
> controlling tty: Operation not permitted
>     Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot set
> process group: Inappropriate ioctl for device
> 
> Update the start_getty script to invoke getty via the setsid
> utility if needed, i.e. if /sbin/getty is not busybox getty.
> 
> [YOCTO #13058]
> 
> Signed-off-by: André Draszik <andre.draszik@jci.com>
> ---
>  meta/recipes-core/sysvinit/sysvinit-inittab/start_getty | 9
> ++++++++-
>  meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb  | 1 +
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> index e15ae35f90..ae613f1e54 100644
> --- a/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> +++ b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> @@ -13,6 +13,13 @@ active_serial=$(grep "serial" /proc/tty/drivers |
> cut -d/ -f1 | sed "s/ *$//")
>  # Rephrase input parameter from ttyS target index (ttyS1, ttyS2,
> ttyAMA0, etc).
>  runtime_tty=$(echo $2 | grep -oh '[0-9]')
>  
> +# busybox' getty does this itself, util-linux' getty needs extra
> help
> +getty="/sbin/getty"
> +if ! basename $(readlink -f "${getty}") | grep -q busybox \
> +    && [ -x "/usr/bin/setsid" ] ; then
> +    getty="/usr/bin/setsid ${getty}"
> +fi

The reason I'm reluctant to take changes like this is you just added 3
fork calls above. This does have a significant effect on boot time on
slower devices.

Is there a way we can do a shell string comparison rather than using
grep? case statement maybe?

Cheers,

Richard




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sysvinit-inittab: support non-busybox-getty on serial consoles
  2019-02-08 14:07 ` Richard Purdie
@ 2019-02-08 16:55   ` André Draszik
  2019-02-08 17:47     ` Richard Purdie
  0 siblings, 1 reply; 4+ messages in thread
From: André Draszik @ 2019-02-08 16:55 UTC (permalink / raw)
  To: Richard Purdie, openembedded-core

On Fri, 2019-02-08 at 14:07 +0000, Richard Purdie wrote:
> On Fri, 2019-02-08 at 12:02 +0000, André Draszik wrote:
> > From: André Draszik <andre.draszik@jci.com>
> > 
> > Busybox' getty has code to try to make itself a session leader,
> > whereas util-linux' agetty doesn't. It expects this to happen
> > from outside.
> > When getty is not a session leader, many things don't work on
> > the serial console, e.g. setting the terminal process group,
> > job control doesn't work, etc.
> > 
> > Executing image tests also fails with AssertionErrors, because
> >     Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot get
> > controlling tty: Operation not permitted
> >     Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot set
> > process group: Inappropriate ioctl for device
> > 
> > Update the start_getty script to invoke getty via the setsid
> > utility if needed, i.e. if /sbin/getty is not busybox getty.
> > 
> > [YOCTO #13058]
> > 
> > Signed-off-by: André Draszik <andre.draszik@jci.com>
> > ---
> >  meta/recipes-core/sysvinit/sysvinit-inittab/start_getty | 9
> > ++++++++-
> >  meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb  | 1 +
> >  2 files changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> > b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> > index e15ae35f90..ae613f1e54 100644
> > --- a/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> > +++ b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> > @@ -13,6 +13,13 @@ active_serial=$(grep "serial" /proc/tty/drivers |
> > cut -d/ -f1 | sed "s/ *$//")
> >  # Rephrase input parameter from ttyS target index (ttyS1, ttyS2,
> > ttyAMA0, etc).
> >  runtime_tty=$(echo $2 | grep -oh '[0-9]')
> >  
> > +# busybox' getty does this itself, util-linux' getty needs extra
> > help
> > +getty="/sbin/getty"
> > +if ! basename $(readlink -f "${getty}") | grep -q busybox \
> > +    && [ -x "/usr/bin/setsid" ] ; then
> > +    getty="/usr/bin/setsid ${getty}"
> > +fi
> 
> The reason I'm reluctant to take changes like this is you just added 3
> fork calls above. This does have a significant effect on boot time on
> slower devices.
> 
> Is there a way we can do a shell string comparison rather than using
> grep? case statement maybe?

I've sent a v2.

Are you aware that there are at least 9 forks in this file for something as
simple as forking a getty?


Cheers,
Andre'

> 
> Cheers,
> 
> Richard
> 
> 



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sysvinit-inittab: support non-busybox-getty on serial consoles
  2019-02-08 16:55   ` André Draszik
@ 2019-02-08 17:47     ` Richard Purdie
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Purdie @ 2019-02-08 17:47 UTC (permalink / raw)
  To: André Draszik, openembedded-core

On Fri, 2019-02-08 at 16:55 +0000, André Draszik wrote:
> On Fri, 2019-02-08 at 14:07 +0000, Richard Purdie wrote:
> > On Fri, 2019-02-08 at 12:02 +0000, André Draszik wrote:
> > > From: André Draszik <andre.draszik@jci.com>
> > > 
> > > Busybox' getty has code to try to make itself a session leader,
> > > whereas util-linux' agetty doesn't. It expects this to happen
> > > from outside.
> > > When getty is not a session leader, many things don't work on
> > > the serial console, e.g. setting the terminal process group,
> > > job control doesn't work, etc.
> > > 
> > > Executing image tests also fails with AssertionErrors, because
> > >     Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot get
> > > controlling tty: Operation not permitted
> > >     Feb  5 16:12:55 qemuarm getty[590]: /dev/ttyAMA1: cannot set
> > > process group: Inappropriate ioctl for device
> > > 
> > > Update the start_getty script to invoke getty via the setsid
> > > utility if needed, i.e. if /sbin/getty is not busybox getty.
> > > 
> > > [YOCTO #13058]
> > > 
> > > Signed-off-by: André Draszik <andre.draszik@jci.com>
> > > ---
> > >  meta/recipes-core/sysvinit/sysvinit-inittab/start_getty | 9
> > > ++++++++-
> > >  meta/recipes-core/sysvinit/sysvinit-inittab_2.88dsf.bb  | 1 +
> > >  2 files changed, 9 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/meta/recipes-core/sysvinit/sysvinit-
> > > inittab/start_getty
> > > b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> > > index e15ae35f90..ae613f1e54 100644
> > > --- a/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> > > +++ b/meta/recipes-core/sysvinit/sysvinit-inittab/start_getty
> > > @@ -13,6 +13,13 @@ active_serial=$(grep "serial"
> > > /proc/tty/drivers |
> > > cut -d/ -f1 | sed "s/ *$//")
> > >  # Rephrase input parameter from ttyS target index (ttyS1, ttyS2,
> > > ttyAMA0, etc).
> > >  runtime_tty=$(echo $2 | grep -oh '[0-9]')
> > >  
> > > +# busybox' getty does this itself, util-linux' getty needs extra
> > > help
> > > +getty="/sbin/getty"
> > > +if ! basename $(readlink -f "${getty}") | grep -q busybox \
> > > +    && [ -x "/usr/bin/setsid" ] ; then
> > > +    getty="/usr/bin/setsid ${getty}"
> > > +fi
> > 
> > The reason I'm reluctant to take changes like this is you just
> > added 3
> > fork calls above. This does have a significant effect on boot time
> > on
> > slower devices.
> > 
> > Is there a way we can do a shell string comparison rather than
> > using
> > grep? case statement maybe?
> 
> I've sent a v2.
> 
> Are you aware that there are at least 9 forks in this file for
> something as
> simple as forking a getty?

I'm not familiar with this file in particular.

I know we removed a lot from general startup but a lot has crept in
since then :( 

Cheers,

Richard



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-02-08 17:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-02-08 12:02 [PATCH] sysvinit-inittab: support non-busybox-getty on serial consoles André Draszik
2019-02-08 14:07 ` Richard Purdie
2019-02-08 16:55   ` André Draszik
2019-02-08 17:47     ` Richard Purdie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox