Review Request: Add suspend-signal to powerdevil and make plasma time dataengine detecting clock skrews

[Sebastian Kügler]

On Monday, September 13, 2010 21:12:37 Björn Ruberg wrote:
> I would be fine reworking the patch in that manner IF it will be accepted
> then. May be I get some stubs provided so that I know where I have to go
> into powerdevil/solid.

That's up to Dario and Kevin, I don't object in principle, if done well. But 
really, not my call. (And still, it's not that I really like 
QTimer::singleShot() hackery working around HAL limitations.)

> But concerning your dbus-concerns: I'm inventing a signal
> standbyRequested(). I can rename it awaitingStandby() or whatever.
> Important thing is, I don't see a reason why such a signal should not be
> in the public API. I can imagine different usages for it, i.e.
> applications gracefully closing network connections before standby.

You don't have the time for that, as standby might be kicking in halfway, 
remember, there's no guarantee when a QSIGNAL will be delivered, it's 
essentially async. The actual suspend can happen before, or after, or in 
between, since the kernel can interrupt user space at any given point. (In 
fact your patch assume that this happens within 2 minutes, but again, this is 
also not a given -- the code path between when you call the signal and the 
actual resume of control in userspace might take more than those two minutes. 
Fortunately, in reality it doesn't happen very often, but it's well possible 
(imagine a machine needing to free enough swap space in order to resume, or 
some driver taking a long time to tear down some device).

What you are talking about here something like suspend blockers. And hey, 
http://lwn.net/Articles/390369/ ... that's where we have to rely on the kernel 
and middleware (HAL, upower) again, since it has to be a system-wide mechanism 
to be effective. These things provide a mechanism to say "don't suspend" I'm 
doing this and that (sounds orthogonal, I know, but your "awaitingStandby()" 
is in fact a "someone called suspend, some time ago", it's not awaiting since 
it's not blocking, and it should not be either.

The gist is that what you want is not a signal, but a lock on the suspending 
function, and that is quite a bit more work. A signal such as the one you're 
proposing is close to useless IMO, and not even the subject of this whole 
thread -- that is "updating the clock when the machine has resumed". 

Before adding something to public API, we need clear usecases, if we added 
functions because we can imagine someone doing it this way (even if broken), 
we'd end up with a pretty horrible pile of crap.

Let's just wing this part for now =)

> The whole polling stuff is there to work around the missing "resumed"
> signal. Surely that can be moved in the lower layer. The main benefit of
> this that there will actually be an "resumed()"-signal that other
> applications can catch. But as there will come a correct signal for this
> with the 4.6 release (I hope so :)), the patch might even stay as it is
> for the 4.5 line.

Yes, in the HAL backend, unless someone implements this in HAL.

The idea is to "fake" the resume() signal for the HAL backend, so that it'll 
work for all applications, and that the API for that stuff doesn't change when 
switching backends.
-- 
sebas

http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9