[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 1/2] Implement the asynchronous suspend and RTC wakeup



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]