Change the way client event loop watches are managed

The current qemudRegisterClientEvent() code is used both for
registering the initial socket watch, and updating the already
registered watch. This causes unneccessary complexity in alot
of code which only cares about updating existing watches. The
updating of a watch cannot ever fail, nor is a reference to the
'qemud_server' object required.

This introduces a new qemudUpdateClientEvent() method for that
case, allowing the elimination of unneccessary error checking
and removal of the server back-reference in struct qemud_client.

* qemud/qemud.h: Remove 'server' field from struct qemud_client.
  Add qemudUpdateClientEvent() method. Remove 'update' param
  from qemudRegisterClientEvent method
* qemud/dispatch.c, qemud/qemud.c, qemud/remote.c: Update alot
  of code to use qemudUpdateClientEvent() instead of
  qemudRegisterClientEvent(). Move more logic from remoteRelayDomainEvent
  into remoteDispatchDomainEventSend.

diff --git a/qemud/dispatch.c b/qemud/dispatch.c
index ce8dbc9fc3f62b9b4c34eb4b7e74d275a5cd9d62..a4e6c3e18dbf6f3238629856bf4db52495abce20 100644 (file)
--- a/qemud/dispatch.c
+++ b/qemud/dispatch.c
@@ -389,9 +389,7 @@ rpc_error:
 
     /* Put reply on end of tx queue to send out  */
     qemudClientMessageQueuePush(&client->tx, msg);
-
-    if (qemudRegisterClientEvent(server, client, 1) < 0)
-        qemudDispatchClientFailure(client);
+    qemudUpdateClientEvent(client);
 
     return 0;
 

diff --git a/qemud/qemud.c b/qemud/qemud.c
index 4952d0bdc14bd5af9e20291e18fe98cdd20101c9..42bc00eeb814e0b49247bfbd816a642bb8ad5c36 100644 (file)
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -1276,7 +1276,6 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket
     client->auth = sock->auth;
     memcpy (&client->addr, &addr, sizeof addr);
     client->addrlen = addrlen;
-    client->server = server;
 
     /* Prepare one for packet receive */
     if (VIR_ALLOC(client->rx) < 0)
@@ -1306,7 +1305,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket
 
     if (client->type != QEMUD_SOCK_TYPE_TLS) {
         /* Plain socket, so prepare to read first message */
-        if (qemudRegisterClientEvent (server, client, 0) < 0)
+        if (qemudRegisterClientEvent (server, client) < 0)
             goto cleanup;
     } else {
         int ret;
@@ -1328,13 +1327,13 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket
                 goto cleanup;
 
             /* Handshake & cert check OK,  so prepare to read first message */
-            if (qemudRegisterClientEvent(server, client, 0) < 0)
+            if (qemudRegisterClientEvent(server, client) < 0)
                 goto cleanup;
         } else if (ret == GNUTLS_E_INTERRUPTED || ret == GNUTLS_E_AGAIN) {
             /* Most likely, need to do more handshake data */
             client->handshake = 1;
 
-            if (qemudRegisterClientEvent (server, client, 0) < 0)
+            if (qemudRegisterClientEvent (server, client) < 0)
                 goto cleanup;
         } else {
             VIR_ERROR(_("TLS handshake failed: %s"),
@@ -1699,10 +1698,7 @@ readmore:
         /* Prepare to read rest of message */
         client->rx->bufferLength += len;
 
-        if (qemudRegisterClientEvent(server, client, 1) < 0) {
-            qemudDispatchClientFailure(client);
-            return;
-        }
+        qemudUpdateClientEvent(client);
 
         /* Try and read payload immediately instead of going back
            into poll() because chances are the data is already
@@ -1722,11 +1718,10 @@ readmore:
             if (client->rx)
                 client->rx->bufferLength = REMOTE_MESSAGE_HEADER_XDR_LEN;
 
-            if (qemudRegisterClientEvent(server, client, 1) < 0)
-                qemudDispatchClientFailure(client);
-            else
-                /* Tell one of the workers to get on with it... */
-                virCondSignal(&server->job);
+            qemudUpdateClientEvent(client);
+
+            /* Tell one of the workers to get on with it... */
+            virCondSignal(&server->job);
         }
     }
 }
@@ -1872,8 +1867,7 @@ static ssize_t qemudClientWrite(struct qemud_client *client) {
  * we would block on I/O
  */
 static void
-qemudDispatchClientWrite(struct qemud_server *server,
-                         struct qemud_client *client) {
+qemudDispatchClientWrite(struct qemud_client *client) {
     while (client->tx) {
         ssize_t ret;
 
@@ -1907,16 +1901,16 @@ qemudDispatchClientWrite(struct qemud_server *server,
                 VIR_FREE(reply);
             }
 
-            if (client->closing ||
-                qemudRegisterClientEvent (server, client, 1) < 0)
-                 qemudDispatchClientFailure(client);
+            if (client->closing)
+                qemudDispatchClientFailure(client);
+            else
+                qemudUpdateClientEvent(client);
          }
     }
 }
 
 static void
-qemudDispatchClientHandshake(struct qemud_server *server,
-                             struct qemud_client *client) {
+qemudDispatchClientHandshake(struct qemud_client *client) {
     int ret;
     /* Continue the handshake. */
     ret = gnutls_handshake (client->tlssession);
@@ -1926,15 +1920,14 @@ qemudDispatchClientHandshake(struct qemud_server *server,
         /* Finished.  Next step is to check the certificate. */
         if (remoteCheckAccess (client) == -1)
             qemudDispatchClientFailure(client);
-        else if (qemudRegisterClientEvent (server, client, 1))
-            qemudDispatchClientFailure(client);
+        else
+            qemudUpdateClientEvent(client);
     } else if (ret == GNUTLS_E_AGAIN ||
                ret == GNUTLS_E_INTERRUPTED) {
         /* Carry on waiting for more handshake. Update
            the events just in case handshake data flow
            direction has changed */
-        if (qemudRegisterClientEvent (server, client, 1))
-            qemudDispatchClientFailure(client);
+        qemudUpdateClientEvent (client);
     } else {
         /* Fatal error in handshake */
         VIR_ERROR(_("TLS handshake failed: %s"),
@@ -1974,10 +1967,10 @@ qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) {
     if (events & (VIR_EVENT_HANDLE_WRITABLE |
                   VIR_EVENT_HANDLE_READABLE)) {
         if (client->handshake) {
-            qemudDispatchClientHandshake(server, client);
+            qemudDispatchClientHandshake(client);
         } else {
             if (events & VIR_EVENT_HANDLE_WRITABLE)
-                qemudDispatchClientWrite(server, client);
+                qemudDispatchClientWrite(client);
             if (events & VIR_EVENT_HANDLE_READABLE)
                 qemudDispatchClientRead(server, client);
         }
@@ -1992,9 +1985,12 @@ qemudDispatchClientEvent(int watch, int fd, int events, void *opaque) {
     virMutexUnlock(&client->lock);
 }
 
-int qemudRegisterClientEvent(struct qemud_server *server,
-                             struct qemud_client *client,
-                             int update) {
+
+/*
+ * @client: a locked client object
+ */
+static int
+qemudCalculateHandleMode(struct qemud_client *client) {
     int mode = 0;
 
     if (client->handshake) {
@@ -2014,19 +2010,40 @@ int qemudRegisterClientEvent(struct qemud_server *server,
             mode |= VIR_EVENT_HANDLE_WRITABLE;
     }
 
-    if (update) {
-        virEventUpdateHandleImpl(client->watch, mode);
-    } else {
-        if ((client->watch = virEventAddHandleImpl(client->fd,
-                                                   mode,
-                                                   qemudDispatchClientEvent,
-                                                   server, NULL)) < 0)
-            return -1;
-    }
+    return mode;
+}
+
+/*
+ * @server: a locked or unlocked server object
+ * @client: a locked client object
+ */
+int qemudRegisterClientEvent(struct qemud_server *server,
+                             struct qemud_client *client) {
+    int mode;
+
+    mode = qemudCalculateHandleMode(client);
+
+    if ((client->watch = virEventAddHandleImpl(client->fd,
+                                               mode,
+                                               qemudDispatchClientEvent,
+                                               server, NULL)) < 0)
+        return -1;
 
     return 0;
 }
 
+/*
+ * @client: a locked client object
+ */
+void qemudUpdateClientEvent(struct qemud_client *client) {
+    int mode;
+
+    mode = qemudCalculateHandleMode(client);
+
+    virEventUpdateHandleImpl(client->watch, mode);
+}
+
+
 static void
 qemudDispatchServerEvent(int watch, int fd, int events, void *opaque) {
     struct qemud_server *server = (struct qemud_server *)opaque;

diff --git a/qemud/qemud.h b/qemud/qemud.h
index 659742958df7e5f27e530ee4896c47dd6a0df1f7..86b893da625a6c390656bfa2c631496a73985461 100644 (file)
--- a/qemud/qemud.h
+++ b/qemud/qemud.h
@@ -142,8 +142,6 @@ struct qemud_client {
     virConnectPtr conn;
     int refs;
 
-    /* back-pointer to our server */
-    struct qemud_server *server;
 };
 
 #define QEMUD_CLIENT_MAGIC 0x7788aaee
@@ -204,8 +202,8 @@ void qemudLog(int priority, const char *fmt, ...)
 
 
 int qemudRegisterClientEvent(struct qemud_server *server,
-                             struct qemud_client *client,
-                             int update);
+                             struct qemud_client *client);
+void qemudUpdateClientEvent(struct qemud_client *client);
 
 void qemudDispatchClientFailure(struct qemud_client *client);
 

diff --git a/qemud/remote.c b/qemud/remote.c
index 4008bdb66c9c558eef4c413c444cadad1957308b..3a43472e2b464c8a804ac7828c02499be7402c62 100644 (file)
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -91,11 +91,7 @@ const dispatch_data const *remoteGetDispatchData(int proc)
 /* Prototypes */
 static void
 remoteDispatchDomainEventSend (struct qemud_client *client,
-                               virDomainPtr dom,
-                               int event,
-                               int detail);
-
-
+                               remote_domain_event_msg *data);
 
 int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED,
                             virDomainPtr dom,
@@ -107,12 +103,17 @@ int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED,
     REMOTE_DEBUG("Relaying domain event %d %d", event, detail);
 
     if (client) {
+        remote_domain_event_msg data;
+
         virMutexLock(&client->lock);
 
-        remoteDispatchDomainEventSend (client, dom, event, detail);
+        /* build return data */
+        memset(&data, 0, sizeof data);
+        make_nonnull_domain (&data.dom, dom);
+        data.event = event;
+        data.detail = detail;
 
-        if (qemudRegisterClientEvent(client->server, client, 1) < 0)
-            qemudDispatchClientFailure(client);
+        remoteDispatchDomainEventSend (client, &data);
 
         virMutexUnlock(&client->lock);
     }
@@ -4409,14 +4410,11 @@ remoteDispatchDomainEventsDeregister (struct qemud_server *server ATTRIBUTE_UNUS
 
 static void
 remoteDispatchDomainEventSend (struct qemud_client *client,
-                               virDomainPtr dom,
-                               int event,
-                               int detail)
+                               remote_domain_event_msg *data)
 {
     struct qemud_client_message *msg = NULL;
     XDR xdr;
     unsigned int len;
-    remote_domain_event_msg data;
 
     if (VIR_ALLOC(msg) < 0)
         return;
@@ -4437,12 +4435,7 @@ remoteDispatchDomainEventSend (struct qemud_client *client,
                    msg->bufferLength - msg->bufferOffset,
                    XDR_ENCODE);
 
-    /* build return data */
-    make_nonnull_domain (&data.dom, dom);
-    data.event = event;
-    data.detail = detail;
-
-    if (!xdr_remote_domain_event_msg(&xdr, &data))
+    if (!xdr_remote_domain_event_msg(&xdr, data))
         goto xdr_error;
 
 
@@ -4460,6 +4453,7 @@ remoteDispatchDomainEventSend (struct qemud_client *client,
     msg->bufferLength = len;
     msg->bufferOffset = 0;
     qemudClientMessageQueuePush(&client->tx, msg);
+    qemudUpdateClientEvent(client);
 
     xdr_destroy (&xdr);
     return;