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.
Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
---
qemud/dispatch.c | 4 +--
qemud/qemud.c | 93 ++++++++++++++++++++++++++++++++----------------------
qemud/qemud.h | 6 +--
qemud/remote.c | 30 +++++++----------
4 files changed, 70 insertions(+), 63 deletions(-)
diff --git a/qemud/dispatch.c b/qemud/dispatch.c
index ce8dbc9..a4e6c3e 100644
--- 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 4952d0b..42bc00e 100644
--- 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 6597429..86b893d 100644
--- 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 bb49c29..4d6ddef 100644
--- 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;
--
1.6.2.5
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list