Patchwork [2/3] busybox: Add setsid and cttyhack for tiny DISTRO_FEATURE

login
register
mail settings
Submitter Darren Hart
Date June 14, 2012, 5:19 a.m.
Message ID <3b44305edae36265295087fb5f31d22d660599b8.1339650859.git.dvhart@linux.intel.com>
Download mbox | patch
Permalink /patch/29813/
State New
Headers show

Comments

Darren Hart - June 14, 2012, 5:19 a.m.
When building very small systems, it can be useful to spawn
a shell from a simple init script, rather than a full System V Init
process. This requires the shell be the session leader and be able to
open the controlling terminal if it is to have job control.

Enable CONFIG_CTTYHACK and CONFIG_SETSID to enable this for distros
defining the "tiny" DISTRO_FEATURE.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: OE Core <openembedded-core@lists.openembedded.org>
---
 meta/recipes-core/busybox/busybox.inc |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Darren Hart - June 14, 2012, 5:21 a.m.
Note that this is a part of a larger series. The other patches are
against meta-yocto, but the busybox change seemed more appropriate here
than in a bbappend in meta-yocto.

At some point poky-tiny may need to come into oe-core, until then...

On 06/13/2012 10:19 PM, Darren Hart wrote:
> When building very small systems, it can be useful to spawn
> a shell from a simple init script, rather than a full System V Init
> process. This requires the shell be the session leader and be able to
> open the controlling terminal if it is to have job control.
> 
> Enable CONFIG_CTTYHACK and CONFIG_SETSID to enable this for distros
> defining the "tiny" DISTRO_FEATURE.
> 
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> CC: OE Core <openembedded-core@lists.openembedded.org>
> ---
>  meta/recipes-core/busybox/busybox.inc |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
> index 5b83d32..d07ba7e 100644
> --- a/meta/recipes-core/busybox/busybox.inc
> +++ b/meta/recipes-core/busybox/busybox.inc
> @@ -57,6 +57,8 @@ def features_to_busybox_settings(d):
>  	busybox_cfg('nls',  distro_features, 'CONFIG_LOCALE_SUPPORT', cnf, rem)
>  	busybox_cfg('ipv4', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem)
>  	busybox_cfg('ipv6', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem)
> +	busybox_cfg('tiny', distro_features, 'CONFIG_SETSID', cnf, rem)
> +	busybox_cfg('tiny', distro_features, 'CONFIG_CTTYHACK', cnf, rem)
>  	return "\n".join(cnf), "\n".join(rem)
>  
>  # X, Y = ${@features_to_uclibc_settings(d)}
>
Khem Raj - June 14, 2012, 7:05 a.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 6/13/2012 10:19 PM, Darren Hart wrote:
> When building very small systems, it can be useful to spawn a shell
> from a simple init script, rather than a full System V Init 
> process. This requires the shell be the session leader and be able
> to open the controlling terminal if it is to have job control.
> 
> Enable CONFIG_CTTYHACK and CONFIG_SETSID to enable this for
> distros defining the "tiny" DISTRO_FEATURE.
> 
> Signed-off-by: Darren Hart <dvhart@linux.intel.com> CC: OE Core
> <openembedded-core@lists.openembedded.org> --- 
> meta/recipes-core/busybox/busybox.inc |    2 ++ 1 files changed, 2
> insertions(+), 0 deletions(-)
> 
> diff --git a/meta/recipes-core/busybox/busybox.inc
> b/meta/recipes-core/busybox/busybox.inc index 5b83d32..d07ba7e
> 100644 --- a/meta/recipes-core/busybox/busybox.inc +++
> b/meta/recipes-core/busybox/busybox.inc @@ -57,6 +57,8 @@ def
> features_to_busybox_settings(d): busybox_cfg('nls',
> distro_features, 'CONFIG_LOCALE_SUPPORT', cnf, rem) 
> busybox_cfg('ipv4', distro_features,
> 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem) busybox_cfg('ipv6',
> distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem) +
> busybox_cfg('tiny', distro_features, 'CONFIG_SETSID', cnf, rem) +
> busybox_cfg('tiny', distro_features, 'CONFIG_CTTYHACK', cnf, rem) 
> return "\n".join(cnf), "\n".join(rem)
> 
> # X, Y = ${@features_to_uclibc_settings(d)}

Enabling cttyhack and setsid could be a good thing in general
(thinking of initramfs kind of images) so I would propose to enable it
by default
and not introduce another distro knob here. How much does busybox code
size grow which these two features turned on.?

otherwise second best thing is to keep it in poky-tiny distro layer
which wants to set the policy
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk/ZjTsACgkQuwUzVZGdMxSXwACfX44Ffs5G4hhW9q9toubwxZYn
Ag4AnijTe1TbzrGZfHaHw+qZY/IQAzlA
=mTVi
-----END PGP SIGNATURE-----
Phil Blundell - June 14, 2012, 9:41 a.m.
On Wed, 2012-06-13 at 22:19 -0700, Darren Hart wrote:
> diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
> index 5b83d32..d07ba7e 100644
> --- a/meta/recipes-core/busybox/busybox.inc
> +++ b/meta/recipes-core/busybox/busybox.inc
> @@ -57,6 +57,8 @@ def features_to_busybox_settings(d):
>  	busybox_cfg('nls',  distro_features, 'CONFIG_LOCALE_SUPPORT', cnf, rem)
>  	busybox_cfg('ipv4', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem)
>  	busybox_cfg('ipv6', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem)
> +	busybox_cfg('tiny', distro_features, 'CONFIG_SETSID', cnf, rem)
> +	busybox_cfg('tiny', distro_features, 'CONFIG_CTTYHACK', cnf, rem)
>  	return "\n".join(cnf), "\n".join(rem)
>  
>  # X, Y = ${@features_to_uclibc_settings(d)}

What exactly is the mission of the "tiny" DISTRO_FEATURE?  It doesn't
seem very wholesome for it to be enabling a random grab-bag of bits in
busybox (or anywhere else).  

If poky-tiny wants those features enabled then it can, and should, ship
its own configuration for busybox which turns them on.  I think that
would be better than further proliferation of switches in oe-core.

p.
Darren Hart - June 14, 2012, 1:55 p.m.
On 06/14/2012 02:41 AM, Phil Blundell wrote:
> On Wed, 2012-06-13 at 22:19 -0700, Darren Hart wrote:
>> diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
>> index 5b83d32..d07ba7e 100644
>> --- a/meta/recipes-core/busybox/busybox.inc
>> +++ b/meta/recipes-core/busybox/busybox.inc
>> @@ -57,6 +57,8 @@ def features_to_busybox_settings(d):
>>  	busybox_cfg('nls',  distro_features, 'CONFIG_LOCALE_SUPPORT', cnf, rem)
>>  	busybox_cfg('ipv4', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem)
>>  	busybox_cfg('ipv6', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem)
>> +	busybox_cfg('tiny', distro_features, 'CONFIG_SETSID', cnf, rem)
>> +	busybox_cfg('tiny', distro_features, 'CONFIG_CTTYHACK', cnf, rem)
>>  	return "\n".join(cnf), "\n".join(rem)
>>  
>>  # X, Y = ${@features_to_uclibc_settings(d)}
> 
> What exactly is the mission of the "tiny" DISTRO_FEATURE?  It doesn't
> seem very wholesome for it to be enabling a random grab-bag of bits in
> busybox (or anywhere else).

The idea is to avoid having to bbappend busybox and other recipes
basically. I can see how "tiny" is different than "ipv4" as it isn't
explicit it what it enables, making it a "grab-bag" as you put it.

I could come up with a term that describes systems without complex init
systems that need to be able to setup their own shells easily.

Another approach would be to just consider these two features and decide
if they shouldn't just be part of the oe-core busybox defconfig anyway.
I don't see why setsid shouldn't be. I can see arguments against
cttyhack (as it is a hack), but I wouldn't think either should be a huge
deal to just include. I thought the DISTRO_FEATURE was a reasonable
compromise between that and having to maintain a bbappend outside of
oe-core.

> 
> If poky-tiny wants those features enabled then it can, and should, ship
> its own configuration for busybox which turns them on.  I think that
> would be better than further proliferation of switches in oe-core.
> 

How would you feel about just including these two features in the
defconfig then?
Phil Blundell - June 14, 2012, 2:26 p.m.
On Thu, 2012-06-14 at 06:55 -0700, Darren Hart wrote:
> Another approach would be to just consider these two features and decide
> if they shouldn't just be part of the oe-core busybox defconfig anyway.
> I don't see why setsid shouldn't be. I can see arguments against
> cttyhack (as it is a hack), but I wouldn't think either should be a huge
> deal to just include. I thought the DISTRO_FEATURE was a reasonable
> compromise between that and having to maintain a bbappend outside of
> oe-core.

Well, you don't need to maintain a .bbappend as such: you can just
provide your own, completely custom config file for busybox.  That's
what micro does, what shr does (I think), and what I would suggest that
most DISTROs with non-trivial configuration requirements should probably
do.

I think most folks do actually use a .bbappend to fiddle FILESPATH in
order to get the configuration file picked up, but I'm fairly sure you
could do this with a bit of suitable magic in your distro config file
and avoid the .bbappend altogether.

> How would you feel about just including these two features in the
> defconfig then?

Not massively enthusiastic, simply because the logical conclusion of
that approach is that we end up with basically everything in busybox
turned on.  If we're going to go along that path then I think I would
prefer us to be up-front about it and just make the oe-core default
configuration for busybox be a "maximum features enabled" one. 

p.
Darren Hart - June 15, 2012, 10:22 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 06/14/2012 12:05 AM, Khem Raj wrote:
> On 6/13/2012 10:19 PM, Darren Hart wrote:
>> When building very small systems, it can be useful to spawn a
>> shell from a simple init script, rather than a full System V Init
>>  process. This requires the shell be the session leader and be
>> able to open the controlling terminal if it is to have job
>> control.
> 
>> Enable CONFIG_CTTYHACK and CONFIG_SETSID to enable this for 
>> distros defining the "tiny" DISTRO_FEATURE.
> 
>> Signed-off-by: Darren Hart <dvhart@linux.intel.com> CC: OE Core 
>> <openembedded-core@lists.openembedded.org> --- 
>> meta/recipes-core/busybox/busybox.inc |    2 ++ 1 files changed,
>> 2 insertions(+), 0 deletions(-)
> 
>> diff --git a/meta/recipes-core/busybox/busybox.inc 
>> b/meta/recipes-core/busybox/busybox.inc index 5b83d32..d07ba7e 
>> 100644 --- a/meta/recipes-core/busybox/busybox.inc +++ 
>> b/meta/recipes-core/busybox/busybox.inc @@ -57,6 +57,8 @@ def 
>> features_to_busybox_settings(d): busybox_cfg('nls', 
>> distro_features, 'CONFIG_LOCALE_SUPPORT', cnf, rem) 
>> busybox_cfg('ipv4', distro_features, 
>> 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem) busybox_cfg('ipv6', 
>> distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem) + 
>> busybox_cfg('tiny', distro_features, 'CONFIG_SETSID', cnf, rem)
>> + busybox_cfg('tiny', distro_features, 'CONFIG_CTTYHACK', cnf,
>> rem) return "\n".join(cnf), "\n".join(rem)
> 
>> # X, Y = ${@features_to_uclibc_settings(d)}
> 
> Enabling cttyhack and setsid could be a good thing in general 
> (thinking of initramfs kind of images) so I would propose to enable
> it by default and not introduce another distro knob here. How much
> does busybox code size grow which these two features turned on.?

So this struck me as strange, but I did it twice to make sure I didn't
mess something up:

BEFORE
- -rwxr-xr-x 1 dvhart dvhart 2799417 2012-06-15 15:06 busybox
- -rwxr-xr-x 1 dvhart dvhart 547872 2012-06-15 15:06 busybox_stripped

AFTER (w/ SETSID and CTTYHACK)
- -rwxr-xr-x 1 dvhart dvhart 2805195 2012-06-15 14:45 busybox
- -rwxr-xr-x 1 dvhart dvhart 547872 2012-06-15 15:05 busybox_stripped

DELTA
         busybox: 5778
busybox_stripped: 0

The difference is <6k for the unstripped binary, and 0 bytes for the
stripped binary. It must have something to do with the way busybox
builds and enables features. The above was using my host strip
command. I also tried building the rootfs with and without these
features and comparing the cross-stripped binaries in the rootfs,
delta is also 0 bytes.

So the cost is "negligible" and there is a clear advantage to having
these two options enabled. I'll resend with a two line change to the
defconfig to enable them.

- --
Darren

> 
> otherwise second best thing is to keep it in poky-tiny distro
> layer which wants to set the policy
> 
> _______________________________________________ Openembedded-core
> mailing list Openembedded-core@lists.openembedded.org 
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
> 
- -- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJP27WMAAoJEKbMaAwKp364Hf0IAKpHOMWy63ZPYwN+Ay7wXPf1
KaUe4fWGBIMNJqS5FGJyhxBnWMdXJceAKMP6Wo2nIa/kvjat//IME8v41xwAb+mD
5sjZpkcBE02CFq9rZmHfiTTPbYC7iS8a6Dz5n/fTLb79b189Jv3Rx/G3GCN4ioeC
xqwVU8zmdtkrHfU+UonR5G4HdNMCjOCFETLWcWW6DNTsj5PmazJV2cMIZTYJEr43
G/EyvO2fz/n+y53dsI04zwNEXuyr0pjurCK7dXt9oxlOnDvE276yS1b0Xihwm3cW
G/h0udSeQ3fcF91vCxdYMAlQNJyMqiCOCeSW5gOVe6NKs+kuxzRqYFSEfD0gtmY=
=aZeL
-----END PGP SIGNATURE-----
Darren Hart - June 15, 2012, 10:57 p.m.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 06/15/2012 03:22 PM, Darren Hart wrote:
> 
> 
> On 06/14/2012 12:05 AM, Khem Raj wrote:
>> On 6/13/2012 10:19 PM, Darren Hart wrote:
>>> When building very small systems, it can be useful to spawn a 
>>> shell from a simple init script, rather than a full System V
>>> Init process. This requires the shell be the session leader and
>>> be able to open the controlling terminal if it is to have job 
>>> control.
> 
>>> Enable CONFIG_CTTYHACK and CONFIG_SETSID to enable this for 
>>> distros defining the "tiny" DISTRO_FEATURE.
> 
>>> Signed-off-by: Darren Hart <dvhart@linux.intel.com> CC: OE Core
>>>  <openembedded-core@lists.openembedded.org> --- 
>>> meta/recipes-core/busybox/busybox.inc |    2 ++ 1 files
>>> changed, 2 insertions(+), 0 deletions(-)
> 
>>> diff --git a/meta/recipes-core/busybox/busybox.inc 
>>> b/meta/recipes-core/busybox/busybox.inc index 5b83d32..d07ba7e
>>>  100644 --- a/meta/recipes-core/busybox/busybox.inc +++ 
>>> b/meta/recipes-core/busybox/busybox.inc @@ -57,6 +57,8 @@ def 
>>> features_to_busybox_settings(d): busybox_cfg('nls', 
>>> distro_features, 'CONFIG_LOCALE_SUPPORT', cnf, rem) 
>>> busybox_cfg('ipv4', distro_features, 
>>> 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem) busybox_cfg('ipv6', 
>>> distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem) + 
>>> busybox_cfg('tiny', distro_features, 'CONFIG_SETSID', cnf,
>>> rem) + busybox_cfg('tiny', distro_features, 'CONFIG_CTTYHACK',
>>> cnf, rem) return "\n".join(cnf), "\n".join(rem)
> 
>>> # X, Y = ${@features_to_uclibc_settings(d)}
> 
>> Enabling cttyhack and setsid could be a good thing in general 
>> (thinking of initramfs kind of images) so I would propose to
>> enable it by default and not introduce another distro knob here.
>> How much does busybox code size grow which these two features
>> turned on.?
> 
> So this struck me as strange, but I did it twice to make sure I
> didn't mess something up:
> 
> BEFORE -rwxr-xr-x 1 dvhart dvhart 2799417 2012-06-15 15:06 busybox 
> -rwxr-xr-x 1 dvhart dvhart 547872 2012-06-15 15:06
> busybox_stripped
> 
> AFTER (w/ SETSID and CTTYHACK) -rwxr-xr-x 1 dvhart dvhart 2805195
> 2012-06-15 14:45 busybox -rwxr-xr-x 1 dvhart dvhart 547872
> 2012-06-15 15:05 busybox_stripped
> 
> DELTA busybox: 5778 busybox_stripped: 0
> 
> The difference is <6k for the unstripped binary, and 0 bytes for
> the stripped binary. It must have something to do with the way
> busybox builds and enables features. The above was using my host
> strip command. I also tried building the rootfs with and without
> these features and comparing the cross-stripped binaries in the
> rootfs, delta is also 0 bytes.
> 
> So the cost is "negligible" and there is a clear advantage to
> having these two options enabled. I'll resend with a two line
> change to the defconfig to enable them.

Sorry, I hit the menuconfig/sstate bug. There is a 2560 byte
difference for including these two features. Seems reasonable to me. A
new patch has been sent to the list.

- --
Darren

> 
> -- Darren
> 
> 
>> otherwise second best thing is to keep it in poky-tiny distro 
>> layer which wants to set the policy
> 
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org 
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>> 
> 
> 
> _______________________________________________ Openembedded-core
> mailing list Openembedded-core@lists.openembedded.org 
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
> 
- -- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJP273OAAoJEKbMaAwKp364D7EH/2RsItQ6k5y610GbII1Emz/n
hX85K/T9f0Pooj9axNtM9E+W3X3AoRlDCRkhlKsZo8YqYnnTCcohYhVpoH0mrnN6
CtnW3e7usrAkoRjw/1COfRUuzGnxZRhE1BY7zW2+EKEYbFOeHFTw35erxOaTK8Cc
huemQLgmtfSYnMJjig8WEDETmlKy1dBHpRscRHUdJkkivKMJdv4M3ASZ9fAG21HB
fCXO6D6pRy2B60NJJ/7VKAuvB0o64ZOlK4rjuUPL+goGayQTwt85giB5Vk2K3zkT
n1ekvtBezgYg/3Av9B16kwoiKEWZzZ2qDvSkpCh4u6bzTcXeJAaWaaYUyhQGojU=
=Yn5h
-----END PGP SIGNATURE-----

Patch

diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
index 5b83d32..d07ba7e 100644
--- a/meta/recipes-core/busybox/busybox.inc
+++ b/meta/recipes-core/busybox/busybox.inc
@@ -57,6 +57,8 @@  def features_to_busybox_settings(d):
 	busybox_cfg('nls',  distro_features, 'CONFIG_LOCALE_SUPPORT', cnf, rem)
 	busybox_cfg('ipv4', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV4', cnf, rem)
 	busybox_cfg('ipv6', distro_features, 'CONFIG_FEATURE_IFUPDOWN_IPV6', cnf, rem)
+	busybox_cfg('tiny', distro_features, 'CONFIG_SETSID', cnf, rem)
+	busybox_cfg('tiny', distro_features, 'CONFIG_CTTYHACK', cnf, rem)
 	return "\n".join(cnf), "\n".join(rem)
 
 # X, Y = ${@features_to_uclibc_settings(d)}