On 11/22/2011 11:33 PM, Eric Blake wrote:
On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote:
Add the core functions that implement the functionality of the API. Suspend is done by using an asynchronous mechanism so that we can return the status to the caller successfully before the host gets suspended. This asynchronous operation is achieved by suspending the host in a separate thread of execution.
+ [...]
+ return -1; + + setAlarmCmd = virCommandNewArgList("rtcwake", "-m", "no", "-s", NULL); + virCommandAddArgFormat(setAlarmCmd, "%lld", alarmTime);
'man rtcwake' says that not all systems support RTC wake from S4; systems that have a functioning nvram-wakeup will succeed, but that is not all systems. Do we need to be more cautious about allowing S4 suspension if we can't prove that RTC will wake up the system from S4?
On the other hand, you are using -m no to just set the wakeup time, which ought to fail if the system does not support the requested delay or the ability to wake up, so that you never actually suspend until after you know the wakeup was successfully scheduled.
Hmm, does that mean the public API should provide a way to schedule the wakeup without also scheduling a suspend?
But how would that help? The aim of having this API is to suspend and resume the system.. and hence I don't see why it has to expose a functionality to only schedule a wakeup..
+++ b/src/util/threads-pthread.c @@ -81,10 +81,25 @@ void virMutexDestroy(virMutexPtr m) pthread_mutex_destroy(&m->lock); }
-void virMutexLock(virMutexPtr m){ +void virMutexLock(virMutexPtr m) +{ pthread_mutex_lock(&m->lock); }
+/** + * virMutexTryLock:
I'm not convinced we need this. As I understand it, your code does:
thread 1: thread 2: thread 3: request suspend grab lock spawn helper sleep 10 sec return success request suspend lock not available return failure suspend resume request suspend lock not available return failure release lock
But we don't need a try-lock operation, if we do:
thread 1: thread 2: thread 3: request suspend grab lock request suspend mark flag to true release lock grab lock flag already true release lock return failure spawn helper sleep 10 sec return success suspend resume grab lock flag already true release lock return failure grab lock clear flag release lock
That is, instead of holding the lock for the entire duration of the suspend, just hold the lock long enough to mark a volatile variable; then you no longer need a non-blocking try-lock primitive, because the lock will never starve anyone else long enough to be an issue.
Yes, that would work. (In fact, that was the way I first developed the code. But then I felt trylock() was a fairly popular primitive to use in such cases and hence I thought I might as well add it to libvirt). Anyways, I am fine with going with the method you suggested above. Thanks, Srivatsa S. Bhat IBM Linux Technology Center