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(a)linux.vnet.ibm.com&gt; >> --- >> >> 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.