Re: [libvirt] [PATCH v4 08/13] Implement keepalive protocol in remote driver

2011-11-07 Thread Daniel P. Berrange
On Thu, Oct 27, 2011 at 06:05:44PM +0200, Jiri Denemark wrote:
 ---
 Notes:
 ACKed
 
 Version 4:
 - no changes
 
 Version 3:
 - remoteStartKeepAlive renamed as remoteSetKeepAlive
 - clients that implement event loop are required to run it, thus
   keepalive is enabled if event loop implementation is found without
   the need to call remoteAllowKeepAlive (which was dropped)
 - keepalive support is advertised to a server implicitly by asking for
   keepalive support between authentication and virConnectOpen
 
 Version 2:
 - no changes
 
  src/remote/remote_driver.c |   52 +++
  src/rpc/virnetclient.c |   83 +--
  src/rpc/virnetclient.h |5 +++
  3 files changed, 136 insertions(+), 4 deletions(-)

ACK

 @@ -663,6 +665,26 @@ doRemoteOpen (virConnectPtr conn,
  if (remoteAuthenticate(conn, priv, auth, authtype) == -1)
  goto failed;
  
 +if (virNetClientKeepAliveIsSupported(priv-client)) {
 +remote_supports_feature_args args =
 +{ VIR_DRV_FEATURE_PROGRAM_KEEPALIVE };
 +remote_supports_feature_ret ret = { 0 };
 +int rc;
 +
 +rc = call(conn, priv, 0, REMOTE_PROC_SUPPORTS_FEATURE,
 +  (xdrproc_t)xdr_remote_supports_feature_args, (char *) 
 args,
 +  (xdrproc_t)xdr_remote_supports_feature_ret, (char *) ret);
 +if (rc == -1)
 +goto failed;
 +
 +if (ret.supported) {
 +priv-serverKeepAlive = true;
 +} else {
 +VIR_WARN(Disabling keepalive protocol since it is not supported
 +  by the server);

Hmm, won't this cause new clients to always issue a warning when talking to
old servers ? Can probably be dropped to VIR_INFO

ACK

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v4 08/13] Implement keepalive protocol in remote driver

2011-10-27 Thread Jiri Denemark
---
Notes:
ACKed

Version 4:
- no changes

Version 3:
- remoteStartKeepAlive renamed as remoteSetKeepAlive
- clients that implement event loop are required to run it, thus
  keepalive is enabled if event loop implementation is found without
  the need to call remoteAllowKeepAlive (which was dropped)
- keepalive support is advertised to a server implicitly by asking for
  keepalive support between authentication and virConnectOpen

Version 2:
- no changes

 src/remote/remote_driver.c |   52 +++
 src/rpc/virnetclient.c |   83 +--
 src/rpc/virnetclient.h |5 +++
 3 files changed, 136 insertions(+), 4 deletions(-)

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index e98ebd7..f99c32d 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -68,6 +68,7 @@
 #endif
 
 static int inside_daemon = 0;
+static virDriverPtr remoteDriver = NULL;
 
 struct private_data {
 virMutex lock;
@@ -84,6 +85,7 @@ struct private_data {
 char *type; /* Cached return from remoteType. */
 int localUses;  /* Ref count for private data */
 char *hostname; /* Original hostname */
+bool serverKeepAlive;   /* Does server support keepalive protocol? */
 
 virDomainEventStatePtr domainEventState;
 };
@@ -663,6 +665,26 @@ doRemoteOpen (virConnectPtr conn,
 if (remoteAuthenticate(conn, priv, auth, authtype) == -1)
 goto failed;
 
+if (virNetClientKeepAliveIsSupported(priv-client)) {
+remote_supports_feature_args args =
+{ VIR_DRV_FEATURE_PROGRAM_KEEPALIVE };
+remote_supports_feature_ret ret = { 0 };
+int rc;
+
+rc = call(conn, priv, 0, REMOTE_PROC_SUPPORTS_FEATURE,
+  (xdrproc_t)xdr_remote_supports_feature_args, (char *) args,
+  (xdrproc_t)xdr_remote_supports_feature_ret, (char *) ret);
+if (rc == -1)
+goto failed;
+
+if (ret.supported) {
+priv-serverKeepAlive = true;
+} else {
+VIR_WARN(Disabling keepalive protocol since it is not supported
+  by the server);
+}
+}
+
 /* Finally we can call the remote side's open function. */
 {
 remote_open_args args = { name, flags };
@@ -4122,6 +4144,33 @@ done:
 }
 
 
+static int
+remoteSetKeepAlive(virConnectPtr conn, int interval, unsigned int count)
+{
+struct private_data *priv = conn-privateData;
+int ret = -1;
+
+remoteDriverLock(priv);
+if (!virNetClientKeepAliveIsSupported(priv-client)) {
+remoteError(VIR_ERR_INTERNAL_ERROR, %s,
+_(the caller doesn't support keepalive protocol;
+   perhaps it's missing event loop implementation));
+goto cleanup;
+}
+
+if (!priv-serverKeepAlive) {
+ret = 1;
+goto cleanup;
+}
+
+ret = virNetClientKeepAliveStart(priv-client, interval, count);
+
+cleanup:
+remoteDriverUnlock(priv);
+return ret;
+}
+
+
 #include remote_client_bodies.h
 #include qemu_client_bodies.h
 
@@ -4474,6 +4523,7 @@ static virDriver remote_driver = {
 .domainGetBlockJobInfo = remoteDomainGetBlockJobInfo, /* 0.9.4 */
 .domainBlockJobSetSpeed = remoteDomainBlockJobSetSpeed, /* 0.9.4 */
 .domainBlockPull = remoteDomainBlockPull, /* 0.9.4 */
+.setKeepAlive = remoteSetKeepAlive, /* 0.9.7 */
 };
 
 static virNetworkDriver network_driver = {
@@ -4624,6 +4674,8 @@ static virStateDriver state_driver = {
 int
 remoteRegister (void)
 {
+remoteDriver = remote_driver;
+
 if (virRegisterDriver (remote_driver) == -1) return -1;
 if (virRegisterNetworkDriver (network_driver) == -1) return -1;
 if (virRegisterInterfaceDriver (interface_driver) == -1) return -1;
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index aaf072a..37e9cc8 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -29,6 +29,7 @@
 
 #include virnetclient.h
 #include virnetsocket.h
+#include virkeepalive.h
 #include memory.h
 #include threads.h
 #include virfile.h
@@ -96,11 +97,12 @@ struct _virNetClient {
 size_t nstreams;
 virNetClientStreamPtr *streams;
 
+virKeepAlivePtr keepalive;
 bool wantClose;
 };
 
 
-void virNetClientRequestClose(virNetClientPtr client);
+static void virNetClientRequestClose(virNetClientPtr client);
 
 static int virNetClientSendInternal(virNetClientPtr client,
 virNetMessagePtr msg,
@@ -130,11 +132,51 @@ static void virNetClientEventFree(void *opaque)
 virNetClientFree(client);
 }
 
+bool
+virNetClientKeepAliveIsSupported(virNetClientPtr client)
+{
+bool supported;
+
+virNetClientLock(client);
+supported = !!client-keepalive;
+virNetClientUnlock(client);
+
+return supported;
+}
+
+int
+virNetClientKeepAliveStart(virNetClientPtr client,
+