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

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?