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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list