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

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, but in the future, please run "make check" and "make syntax-check" on all commits before posting them to the list,