[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!

Srivatsa S. Bhat
IBM Linux Technology Center

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