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

Re: [libvirt] [ PATCH v5 0/4] API to invoke S3/S4 on a host and also resume from within libvirt



On 11/29/2011 03:12 PM, Daniel Veillard wrote:

> On Tue, Nov 29, 2011 at 03:12:07AM +0530, Srivatsa S. Bhat wrote:
>> This patchset adds a new API to put a host to a suspended state
>> (Suspend-to-RAM, Suspend-to-Disk or Hybrid-Suspend) and setup a timed resume
>> to get the host back online, from within libvirt.
>> This uses the RTC wakeup mechanism to set up a timer alarm before suspending
>> the host, so that in-band resume is facilitated by the firing of the RTC
>> alarm, which wakes up the host.
>>
>> This patch applies on top of the Hybrid-Suspend patch posted in [1].
> [...]
>>
>> Srivatsa S. Bhat (4):
>>       Add a public API to invoke suspend/resume on the host
>>       Add the remote protocol implementation for virNodeSuspendForDuration
>>       Implement the core API to suspend/resume the host
>>       Add virsh command to initiate suspend on the host
>>
> 
>   Okay I finally pushed this series. But all patches except 4/4 failed
> to apply cleanly on git head, thay all also raised various "make
> syntax-check" error, which I fixed in the standard way I think.
>   The main issue is that patch 3/4 made "make check" fail all virshtest
> for some reason (maybe the restricted PATH) the call to
> virNodeSuspendInit() in virInitialize() was failing, an well in that
> case nothing work. So I changed virNodeSuspendInit:
>    - to not return -1 if virGetPMCapabilities() fails
>    - only log an error if virGetPMCapabilities() fails and the
>      effective user id is 0
>    - initialize hostPMFeatures to 0 before checking
> 
> This code should probably still be revisited, because IMHO:
>   - when run as non-root we should not expose suspend capabilities
>     that we may not be able to use as non-root (pm-suspend works
>     only as root for example, and pm-is-supported seems to work
>     differently as root or as non-root)
>   - the error log was getting in the way of the virshtest checks
>     and we should probably not log error as non root.
>   - I was tempted to not call virNodeSuspendInit() from virInitialize()
>     but I'm afraid the drivers code may rely on those data including
>     the lock being initialized.
> 
> So my changes certainly need reviews, but the first version is commited
> now.
>


Thanks a lot!

 
>   Thanks, but in the future, please run "make check" and "make syntax-check"
> on all commits before posting them to the list,
> 


Very sorry, it was an oversight from my side, caused by an eagerness to
quickly address all the review comments, to aid further reviews/pushing the
patches before the freeze deadline.
But in future, I will definitely remember to run all the sanity checks and
only then post patches. My sincere apologies for all the trouble I caused
because of that!

-- 
Regards,
Srivatsa S. Bhat
IBM Linux Technology Center


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