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

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



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 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.

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


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