Re: [libvirt] [PATCH v2 03/12] Introduce two public APIs for keepalive protocol
On Fri, Sep 23, 2011 at 10:24:48AM +0200, Jiri Denemark wrote:
This introduces virConnectAllowKeepAlive and
virConnectStartKeepAlive
public APIs which can be used by a client connecting to remote server to
indicate support for keepalive protocol. Both APIs are handled directly
by remote driver and not transmitted over the wire to the server.
---
include/libvirt/libvirt.h.in | 5 ++
src/driver.h | 9 ++++
src/libvirt.c | 107 ++++++++++++++++++++++++++++++++++++++++++
src/libvirt_internal.h | 10 +++-
src/libvirt_public.syms | 6 ++
5 files changed, 135 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 39155a6..6f61cc0 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -3210,6 +3210,11 @@ typedef struct _virTypedParameter virMemoryParameter;
*/
typedef virMemoryParameter *virMemoryParameterPtr;
+int virConnectAllowKeepAlive(virConnectPtr conn);
+int virConnectStartKeepAlive(virConnectPtr conn,
+ int interval,
+ unsigned int count);
With this design, even if both the client and server support
keepalive at a protocol level, it will only ever be enabled if
the client application remembers to call virConnectAllowKeepAlive.
I think this puts too much responsibility on the client, at the
detriment of the server.
An administrator of libvirtd might want to mandate use of keep
alive for all clients (knowing this would exclude any libvirt
client <= 0.9.6 of course). With this design they cannot do this
since they are reliant on the client application programmer to
call virConnectAllowKeepAlive, which I believe 95% of people will
never bother todo.
IMHO we should change this so that
- In remote_driver.c, doRemoteOpen(), after performing
authentication, but before opening the connection
we should send a
supports_feature(KEEPALIVE)
- Upon receiving supports_feature(KEEPALIVE) the server
shall be free to start sending keep alives, if it is
configured todo so.
- Libvirtd should not send server initiated keepalives
by default since this is needlessly short-circuiting
the normal TCP client timeout process.
- libvirt has a configure paramater to turn on keepalives
taking values
0 - disable (default)
1 - enable if supported by client
2 - enable, drop client if not supported
and should error upon virConnectOpen if '2' and clients
do not support it.
- Client simply has a virConnectSetKeepAlive(conn, interval, count).
This returns an error if the earlier feature check during doRemoteOpen
failed. If 'interval' or 'count' is zero then this stops any
previously started keepalive.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|