[libvirt] [PATCH v3 2/2] Make the API public

[Srivatsa S. Bhat]

On 11/23/2011 04:10 PM, Srivatsa S. Bhat wrote:
> On 11/22/2011 11:56 PM, Eric Blake wrote:
>> On 11/09/2011 05:05 AM, Srivatsa S. Bhat wrote:
>>> Define the required interfaces to export the API.
>>>
>>> Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat at linux.vnet.ibm.com>
>>> ---
>>>
>>>  include/libvirt/libvirt.h.in |    4 ++++
>>>  src/driver.h                 |    5 ++++
>>>  src/libvirt.c                |   48 ++++++++++++++++++++++++++++++++++++++++++
>>>  src/libvirt_public.syms      |    1 +
>>
>> These changes are good for the first patch, in introducing the API.
>> Plus see my comment in 1/2 about the hunk defining VIR_S3 (or whatever
>> name we settle on) being part of this patch.
>>
>>>  src/qemu/qemu_driver.c       |    1 +
>>
>> This change should be shuffled into the qemu driver implementation of
>> the driver backend.
>>
>>>  src/remote/remote_driver.c   |    1 +
>>>  src/remote/remote_protocol.x |   12 ++++++++++-
>>
>> These changes belong to a second patch, implementing the RPC driver
>> backend.  And don't forget a patch to virsh.
>>
>>> +++ b/include/libvirt/libvirt.h.in
>>> @@ -1055,6 +1055,10 @@ unsigned long long      virNodeGetFreeMemory    (virConnectPtr conn);
>>>  int                     virNodeGetSecurityModel (virConnectPtr conn,
>>>                                                   virSecurityModelPtr secmodel);
>>>  
>>> +int                     virNodeSuspendForDuration (virConnectPtr conn,
>>> +                                                   int state,
>>> +                                                   unsigned long long duration);
>>
>> Needs an 'unsigned int flags' argument, even if we always pass 0 for now.
>>
>>> +++ b/src/libvirt.c
>>> @@ -6303,6 +6303,54 @@ error:
>>>  }
>>>  
>>>  /**
>>> + * virNodeSuspendForDuration:
>>> + * @conn: pointer to the hypervisor connection
>>> + * @state: the state to which the host must be suspended to,
>>> + *         such as: VIR_S3 (Suspend-to-RAM)
>>> + *                  VIR_S4 (Suspend-to-Disk)
>>
>> Is this a bitmask or a linear list?  I guess a list makes more sense.
>> Also, some machines support a hybrid between S3 and S4 (pm-is-supported
>> --suspend-hybrid), which saves state to disk like S4 but then drops to
>> S3 for faster resume as long as power is present.  Perhaps a hybrid
>> state is deserving of a third value in the enum?
>>
> 
> That would need a corresponding change in virGetPMCapabilities etc (which
> was introduced in the other patch which Daniel Veillard pushed already).
> http://www.redhat.com/archives/libvir-list/2011-November/msg01155.html
> 
> Perhaps, I'll send a companion patch to add the hybrid-suspend discovery
> to that, and then base the next version of this patchset on that.
> 

Actually thinking about it, I don't see how that would be useful for a
libvirt managed host. The hybrid suspend feature was designed keeping
laptops in mind, which when suspended with the power supply turned off
(and hence powered by only batteries), the laptops could lose power and
power off, and hence in that case resuming the saved contents from disk
would be beneficial.

I doubt if anyone would be seriously interested in running virtualization
software on laptops and managing them via libvirt, rather than using
servers for that purpose (which have continuous power-supply).

Though I have already written a patch to add the hybrid-suspend capability
discovery to libvirt and export it in the XML along with S3 and S4, I
would rather prefer not to send that patch if nobody is going to use it.

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