Separate code for encoding outgoing remote message headers

Introduces an API for encoding the header field for outgoing messages
allowing some duplicated code to be eliminated

* qemud/dispatch.c, qemud/dispatch.h: add remoteEncodeClientMessageHeader
  for encoding message header. Update remoteDispatchClientRequest to
  use this method.
* qemud/remote.c: Update remoteDispatchDomainEventSend to use the
  generic remoteEncodeClientMessageHeader() for encoding event
  message hedaders. Push some logic from remoteRelayDomainEvent
  down into remoteDispatchDomainEventSend.

qemud/dispatch.c

@@ -127,7 +127,10 @@ void remoteDispatchConnError (remote_error *rerr,
  * @msg: the complete incoming message, whose header to decode
  *
  * Decodes the header part of the client message, but does not
- * validate the decoded fields in the header.
+ * validate the decoded fields in the header. It expects
+ * bufferLength to refer to length of the data packet. Upon
+ * return bufferOffset will refer to the amount of the packet
+ * consumed by decoding of the header.
  *
  * returns 0 if successfully decoded, -1 upon fatal error
  */
@@ -158,6 +161,61 @@ cleanup:
 }
 
 
+/*
+ * @msg: the outgoing message, whose header to encode
+ *
+ * Encodes the header part of the client message, setting the
+ * message offset ready to encode the payload. Leaves space
+ * for the length field later. Upon return bufferLength will
+ * refer to the total available space for message, while
+ * bufferOffset will refer to current space used by header
+ *
+ * returns 0 if successfully encoded, -1 upon fatal error
+ */
+int
+remoteEncodeClientMessageHeader (struct qemud_client_message *msg)
+{
+    XDR xdr;
+    int ret = -1;
+    unsigned int len = 0;
+
+    msg->bufferLength = sizeof(msg->buffer);
+    msg->bufferOffset = 0;
+
+    /* Format the header. */
+    xdrmem_create (&xdr,
+                   msg->buffer,
+                   msg->bufferLength,
+                   XDR_ENCODE);
+
+    /* The real value is filled in shortly */
+    if (!xdr_u_int (&xdr, &len)) {
+        goto cleanup;
+    }
+
+    if (!xdr_remote_message_header (&xdr, &msg->hdr))
+        goto cleanup;
+
+    len = xdr_getpos(&xdr);
+    xdr_setpos(&xdr, 0);
+
+    /* Fill in current length - may be re-written later
+     * if a payload is added
+     */
+    if (!xdr_u_int (&xdr, &len)) {
+        goto cleanup;
+    }
+
+    msg->bufferOffset += len;
+
+    ret = 0;
+
+cleanup:
+    xdr_destroy(&xdr);
+    return ret;
+}
+
+
 /*
  * @server: the unlocked server object
  * @client: the locked client object
@@ -177,7 +235,6 @@ remoteDispatchClientRequest (struct qemud_server *server,
                              struct qemud_client_message *msg)
 {
     XDR xdr;
-    remote_message_header rep;
     remote_error rerr;
     dispatch_args args;
     dispatch_ret ret;
@@ -277,27 +334,29 @@ remoteDispatchClientRequest (struct qemud_server *server,
 
 rpc_error:
 
-    /* Return header. */
+    /* Return header. We're re-using same message object, so
-    rep.prog = msg->hdr.prog;
+     * only need to tweak direction/status fields */
-    rep.vers = msg->hdr.vers;
+    /*msg->hdr.prog = msg->hdr.prog;*/
-    rep.proc = msg->hdr.proc;
+    /*msg->hdr.vers = msg->hdr.vers;*/
-    rep.direction = REMOTE_REPLY;
+    /*msg->hdr.proc = msg->hdr.proc;*/
-    rep.serial = msg->hdr.serial;
+    msg->hdr.direction = REMOTE_REPLY;
-    rep.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK;
+    /*msg->hdr.serial = msg->hdr.serial;*/
+    msg->hdr.status = rv < 0 ? REMOTE_ERROR : REMOTE_OK;
 
-    /* Serialise the return header. */
+    if (remoteEncodeClientMessageHeader(msg) < 0) {
-    xdrmem_create (&xdr, msg->buffer, sizeof msg->buffer, XDR_ENCODE);
-
-    len = 0; /* We'll come back and write this later. */
-    if (!xdr_u_int (&xdr, &len)) {
         if (rv == 0) xdr_free (data->ret_filter, (char*)&ret);
         goto fatal_error;
     }
 
-    if (!xdr_remote_message_header (&xdr, &rep)) {
+
-        if (rv == 0) xdr_free (data->ret_filter, (char*)&ret);
+    /* Now for the payload */
+    xdrmem_create (&xdr,
+                   msg->buffer,
+                   msg->bufferLength,
+                   XDR_ENCODE);
+
+    if (xdr_setpos(&xdr, msg->bufferOffset) == 0)
         goto fatal_error;
-    }
 
     /* If OK, serialise return structure, if error serialise error. */
     if (rv >= 0) {
@@ -313,8 +372,9 @@ rpc_error:
         xdr_free((xdrproc_t)xdr_remote_error,  (char *)&rerr);
     }
 
-    /* Write the length word. */
+    /* Update the length word. */
-    len = xdr_getpos (&xdr);
+    msg->bufferOffset += xdr_getpos (&xdr);
+    len = msg->bufferOffset;
     if (xdr_setpos (&xdr, 0) == 0)
         goto fatal_error;
 
@@ -323,6 +383,7 @@ rpc_error:
 
     xdr_destroy (&xdr);
 
+    /* Reset ready for I/O */
     msg->bufferLength = len;
     msg->bufferOffset = 0;
 

qemud/dispatch.h

@@ -30,6 +30,8 @@
 
 int
 remoteDecodeClientMessageHeader (struct qemud_client_message *req);
+int
+remoteEncodeClientMessageHeader (struct qemud_client_message *req);
 
 int
 remoteDispatchClientRequest (struct qemud_server *server,

qemud/remote.c

@@ -91,7 +91,6 @@ const dispatch_data const *remoteGetDispatchData(int proc)
 /* Prototypes */
 static void
 remoteDispatchDomainEventSend (struct qemud_client *client,
-                               struct qemud_client_message *msg,
                                virDomainPtr dom,
                                int event,
                                int detail);
@@ -108,14 +107,9 @@ int remoteRelayDomainEvent (virConnectPtr conn ATTRIBUTE_UNUSED,
     REMOTE_DEBUG("Relaying domain event %d %d", event, detail);
 
     if (client) {
-        struct qemud_client_message *ev;
-
-        if (VIR_ALLOC(ev) < 0)
-            return -1;
-
         virMutexLock(&client->lock);
 
-        remoteDispatchDomainEventSend (client, ev, dom, event, detail);
+        remoteDispatchDomainEventSend (client, dom, event, detail);
 
         if (qemudRegisterClientEvent(client->server, client, 1) < 0)
             qemudDispatchClientFailure(client);
@@ -4433,72 +4427,65 @@ remoteDispatchDomainEventsDeregister (struct qemud_server *server ATTRIBUTE_UNUS
 
 static void
 remoteDispatchDomainEventSend (struct qemud_client *client,
-                               struct qemud_client_message *msg,
                                virDomainPtr dom,
                                int event,
                                int detail)
 {
-    remote_message_header rep;
+    struct qemud_client_message *msg = NULL;
     XDR xdr;
     unsigned int len;
     remote_domain_event_ret data;
 
-    if (!client)
+    if (VIR_ALLOC(msg) < 0)
         return;
 
-    rep.prog = REMOTE_PROGRAM;
+    msg->hdr.prog = REMOTE_PROGRAM;
-    rep.vers = REMOTE_PROTOCOL_VERSION;
+    msg->hdr.vers = REMOTE_PROTOCOL_VERSION;
-    rep.proc = REMOTE_PROC_DOMAIN_EVENT;
+    msg->hdr.proc = REMOTE_PROC_DOMAIN_EVENT;
-    rep.direction = REMOTE_MESSAGE;
+    msg->hdr.direction = REMOTE_MESSAGE;
-    rep.serial = 1;
+    msg->hdr.serial = 1;
-    rep.status = REMOTE_OK;
+    msg->hdr.status = REMOTE_OK;
+
+    if (remoteEncodeClientMessageHeader(msg) < 0)
+        goto error;
 
     /* Serialise the return header and event. */
-    xdrmem_create (&xdr, msg->buffer, sizeof msg->buffer, XDR_ENCODE);
+    xdrmem_create (&xdr,
-
+                   msg->buffer + msg->bufferOffset,
-    len = 0; /* We'll come back and write this later. */
+                   msg->bufferLength - msg->bufferOffset,
-    if (!xdr_u_int (&xdr, &len)) {
+                   XDR_ENCODE);
-        /*remoteDispatchError (client, NULL, "%s", _("xdr_u_int failed (1)"));*/
-        xdr_destroy (&xdr);
-        return;
-    }
-
-    if (!xdr_remote_message_header (&xdr, &rep)) {
-        xdr_destroy (&xdr);
-        return;
-    }
 
     /* build return data */
     make_nonnull_domain (&data.dom, dom);
     data.event = event;
     data.detail = detail;
 
-    if (!xdr_remote_domain_event_ret(&xdr, &data)) {
+    if (!xdr_remote_domain_event_ret(&xdr, &data))
-        /*remoteDispatchError (client, NULL, "%s", _("serialise return struct"));*/
+        goto xdr_error;
-        xdr_destroy (&xdr);
-        return;
-    }
 
-    len = xdr_getpos (&xdr);
-    if (xdr_setpos (&xdr, 0) == 0) {
-        /*remoteDispatchError (client, NULL, "%s", _("xdr_setpos failed"));*/
-        xdr_destroy (&xdr);
-        return;
-    }
 
-    if (!xdr_u_int (&xdr, &len)) {
+    /* Update length word */
-        /*remoteDispatchError (client, NULL, "%s", _("xdr_u_int failed (2)"));*/
+    msg->bufferOffset += xdr_getpos (&xdr);
-        xdr_destroy (&xdr);
+    len = msg->bufferOffset;
-        return;
+    if (xdr_setpos (&xdr, 0) == 0)
-    }
+        goto xdr_error;
 
-    xdr_destroy (&xdr);
+    if (!xdr_u_int (&xdr, &len))
+        goto xdr_error;
 
     /* Send it. */
     msg->async = 1;
     msg->bufferLength = len;
     msg->bufferOffset = 0;
     qemudClientMessageQueuePush(&client->tx, msg);
+
+    xdr_destroy (&xdr);
+    return;
+
+xdr_error:
+    xdr_destroy(&xdr);
+error:
+    VIR_FREE(msg);
 }
 
 /*----- Helpers. -----*/