[Pm-utils] [PATCH 1/6] Split hook running into two phases -- core and aux

[Dan Nicholson]

On Fri, May 16, 2008 at 4:10 PM, Victor Lowther
<victor.lowther at gmail.com> wrote:
> On Fri, 2008-05-16 at 10:47 -0700, Dan Nicholson wrote:
>> Hi Victor,
>>
>> On Sun, May 11, 2008 at 6:43 PM, Victor Lowther
>> <victor.lowther at gmail.com> wrote:
>> > Core phase is for hooks that work around software or hardware quirks
>> > that may cause the system to not suspend correctly. Hooks will automatically
>> > fall into this category if their filenames start with at least two digits.
>> >
>> > Aux phase is for programs that want to do something when the system suspends
>> > or resumes, but that do not need to run in any particular order or whose
>> > success or failure has no bearing on whether or not the suspend/resume
>> > process should fail.
>>
>> I would really prefer that we just tighten the specs on hooks instead
>> of getting creative with what hooks run when and whose errors are
>> critical. In my mind, the hooks should work like this:
>>
>> 1. Hooks are run in lexicographical order on suspend and reversed on
>> resume. If your hook depends on running early or late, name it so that
>> it sits at an appropriate place in the order. Maybe a restriction that
>> it must begin with two digits, in which case, 50$hookname if you don't
>> care when it runs.
>
> Fair enough, although we should define some critical points in the
> sequence (e.g. after ?? don't rely on modules being loaded, etc), and
> mabye shuffle the hook ordering around a bit (for example, the ordering
> of 50modules to 65alsa is obviously wrong).
>
> As a starting point, how about the following convention:
>
> 00 - 49: user and (most) package supplied hooks that can assume that all
> of the usual services and userspace infrastructure is still running.
>
> 50 - 74: service-handling hooks (mainly stopping and starting services,
> saving any state they may need, etc)
>
> 75 - 89: module and non-core hardware handling (usb, audio, network,
> etc).
>
> 90 - 99: reserved for critical pre-suspend hooks, starting with 90chvt
> and 90modules and ending with 99video
>
> At or before 50, you can assume that all services are still enabled.
>
> At or before 75, you can assume that all modules are still loaded.

I think that convention is reasonable. Udev is a far more complicated
system than pm-utils, and it's gotten away fine without enforcing any
serious restrictions on the rules files. If a rule file is in the
wrong location, well, then move it to a better one. But I think the
above specification is good.

> If we want to try and enforce this convention, we will want to ignore
> all hooks whose names do not begin with a numeric prefix.  This is also
> easuly codeable.

Enforcement is probably good, and a warning would probably be good,
too. Silently ignoring a hook that used to run would be lame.

>> 2. Failures from hooks always result in suspend or resume errors. If
>> the hook has non-critical errors, then it should return 0, full stop.
>> If someone doesn't have bluetooth or NetworkManager or ntpd isn't
>> running, that should not be considered a critical error. I don't
>> consider alsactl failing to be worth more than a warning in the logs,
>> but I think the better way to handle this is the hook not returning a
>> failure, not marking the hook as some sort of special case.
>
> I am fine with this -- hooks can (and do) signal when they are not
> applicable using the $NA return code.

Right. To me, I don't care if the NetworkManager hook didn't fire
because it's not running. If a 3rd party hook just wanted to exit 0 in
that case, I wouldn't consider it harmful. But as long as it doesn't
stop the resume, that's fine.

>> That's how I see it, anyway. I think we can just make the current
>> hooks more robust rather than re-implement the hooks structure. If
>> there are hooks we're distributing that are difficult to get right,
>> then let's not install them by default. Distros can add or remove the
>> hooks as apply to their system.
>
> Hmmm... my proposed patch series does not re-implement hook handling, it
> merely extends it -- right now in this patch series abort-on-error is
> not implemented, and if you do not remove numeric prefixes from the
> hooks they will run in exactly the same order they used to.  The only
> difference is that hooks without a numeric prefix will run in parallel
> with eachother, and we will not run the core hooks until the aux hooks
> finish (and as a bonus, the system will be up and usable without needing
> to wait for the aux hooks to finish).

You're right, it's extended, not re-implemented. I just don't see
enough justification for it. The tradeoff in complexity and confusion
isn't enough for the gain in functionality, IMO. Cause really, the
pm-utils hooks are just a handful of commands to run besides the
actually important part of suspending and resuming. For nearly all
users, running the commands in a serialized order is perfectly
adequate. But if you want to extend the hooks in a regression-free,
backwards-compatible way, drive on.

--
Dan