[PATCH] halt: add option to call telinit when using external init

[Alexander Shishkin]

2009/7/27 Denys Vlasenko <vda.linux at googlemail.com>:
> On Sunday 26 July 2009 14:45, Alexander Shishkin wrote:
>> 2009/7/26 Denys Vlasenko <vda.linux at googlemail.com>:
>> > On Saturday 25 July 2009 11:07, Alexander Shishkin wrote:
>> >> Come to think of it, 9/10 of busybox functionality could be
>> >> implemented in shell,
>> >
>> > And some of it definitely should have been. ifup is a candidate #1.
>> > reboot also can be. An applet which does nothing else than starts
>> > or signals other processes and neither runs for a long time (server)
>> > nor contains complex logic, isn't really something which
>> > is best written in C.
>>
>> I completely miss the point here. If it is already implemented in C,
>> working and maintainable, why would anyone want to replace it with
>> something scripted?
>
> I don't want to replace anything.
> I do not plan to remove ifup or reboot. I already said it.


>> If I have to reboot and especially "reboot -f",
>> I'm very likely to be on a tight schedule and not have time to fork
>> off a shell and have it interpret a script. There are also context
>> switches, which are costly on some architectures.
>
> How do you think init performs reboot? It usually
> runs a shell script.
That's normal reboot, not "reboot -f".

>> Same stands for
>> ifup, which, be it a shell script will eat into my system's bootup
>> time where every millisecond is valuable (I'm not talking about
>> desktop machines).
>
> Shell is not slow. ifup forking and execing things can't be
> many times faster, since it is basically the same operations
> shell script does. But ifup is many times *less flexible* than
> a shell script, and for ifup it's a big disadvantage.
Well, yes, but for each particular system you don't need flexibility.
Chances are that you'll want to trade flexibility for execution time.
I don't know a lot about ifup, though. My understanding was that most
of the time it simply does "ifconfig $interface $options up", which in
case of busybox should be merely a function call.

>> > I do not plan to remove such applets, but will need a very good
>> > rationale why it's good to add _more_ things like these.
>>
>> Because one might want things to be small and fast when one considers
>> using busybox.
>
> ...but uses SysV init. Strange combination.
Not necessarily SysV init, but even that is not unheard of.

>> > Technically speaking, the situation is like this:
>> >
>> > bbox reboot currently is meant to work with busybox init.
>>
>> I don't know where this comes from, but _technically_ (considering
>> init/Config.in and the actual code) nothing says that.
>> And even should that be the case, I don't see the point.
>
> It is so because bbox reboot communicates with init in bbox-specific
> way (signals) rather than some socket in /dev.
Yes, but there's still a fallback situation in which it will simply
reboot() no matter what.

>> > You propose to make it possible to use it with SysV init
>> > (and any other kinds which have telinit), right?
>>
>> Right. There is little point in using it with SysV init though which,
>> as you pointed out, has its own reboot/poweroff.
>>
>> > Since you are using SysV init, doesn't it make more sense
>> > to use _corresponding_ reboot from the same package?
>> > I think SysV init provides those. Correct me if I'm wrong.
>>
>> I'm not, I'm using upstart, which in its present semiexperimental
>> state does not provide means of rebooting. And I hear there are other
>> "better" init replacements in the making.
>
> Upstart source tree is 11540 kbytes. Do you use it because
> you need your system to be small? This is not easy to believe.
It's installation size is quite small though. And the key feature is
that it can run things in parallel.

> Okay. Let's apply it.
>
> -               }
> -               if (rc) {
> -                       rc = kill(1, signals[which]);
> +
> +                       if (rc)
> +                               rc = kill(1, signals[which]);
>
> ?! this change breaks it. Now we never send the signal
> if !ENABLE_FEATURE_INITRD.
Indeed, my bad.

> +               } else if (ENABLE_FEATURE_CALL_TELINIT) {
> +                       /* runlevels:
> +                        * 0 == shutdown
> +                        * 6 == reboot */
> +                       const char *const args[] = {
>
> static?
Yep.

>
> +                               "telinit",
>
> and if ENABLE_FEATURE_CALL_TELINIT is something completely different?
I'm not very good at kbuild and friends, therefore I assumed that bool
means yes or no.

>
> +                               which == 2 ? "6" : "0",
> +                               NULL
> +                       };
> +
> +                       rc = execvp(CONFIG_TELINIT_PATH, (char **)args);
>
> Or just use execlp?
Might as well indeed.

> Please try attached patch.
Works for me. Thanks a lot!

Regards,
--
Alex