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.
This commit is contained in:
Daniel P. Berrange 2009-07-10 12:48:50 +01:00
parent c40e14b4be
commit af4dad0fa2
4 changed files with 70 additions and 63 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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);

View File

@ -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;