Refactor processing of incoming packets to remove dead code

ChangeLog

+Sat Nov 17 11:04:33 UTC 2007 Daniel P. Berrange <berrange@redhat.com>
+
+        * qemud/Makefile.am: Remove protocol.[chx] from EXTRA_DIST.
+          Remove protocol.h and protocol.c from libvirtd_SOURCES.
+          Remove protocol.c dependency.
+        * qemud/protocol.c, qemud/protocol.h, qemud/protocol.x: Remove files.
+        * qemud/internal.h: Don't include "protocol.h".
+        * qemud/qemud.c: Clean up qemudDispatchClientRead.
+        s/QEMUD_PKT_HEADER_XDR_LEN/REMOTE_MESSAGE_HEADER_XDR_LEN/
+        * qemud/remote.c: In remoteDispatchClientRequest, reflect that the
+        client buffer no longer starts with the 4-byte XDR header length.
+        * qemud/remote_protocol.h: Regenerate.
+        * qemud/remote_protocol.x: Define REMOTE_MESSAGE_HEADER_XDR_LEN.
+        * src/Makefile.am: Remove protocol.h, protocol.c from SERVER_SOURCES.
+
 Thu Nov 15 17:43:00 UTC 2007 Richard W.M. Jones <rjones@redhat.com>
 
 	* src/stats_linux.c: Fix parsing of short interface names

qemud/Makefile.am

@@ -10,8 +10,7 @@ conf_DATA = libvirtd.conf
 # Distribute the generated files so that rpcgen isn't required on the
 # target machine (although almost any Unix machine will have it).
 EXTRA_DIST = libvirtd.init.in libvirtd.sysconf default-network.xml \
-	protocol.x remote_protocol.x \
-	protocol.c protocol.h \
+	remote_protocol.x \
 	remote_protocol.c remote_protocol.h \
 	remote_generate_stubs.pl rpcgen_fix.pl \
 	remote_dispatch_prototypes.h \
@@ -22,7 +21,6 @@ EXTRA_DIST = libvirtd.init.in libvirtd.sysconf default-network.xml \
 
 libvirtd_SOURCES = \
 		qemud.c internal.h \
-		protocol.h protocol.c \
 		remote_protocol.h remote_protocol.c \
 		remote.c \
 		event.c event.h
@@ -80,7 +78,6 @@ uninstall-local: uninstall-init
 	rm -f $@
 	rpcgen -h -o $@ $<
 
-protocol.c: protocol.h
 remote_protocol.c: remote_protocol.h
 
 remote.c: remote_dispatch_prototypes.h \

qemud/internal.h

@@ -29,7 +29,6 @@
 #include <gnutls/x509.h>
 #include "../src/gnutls_1_0_compat.h"
 
-#include "protocol.h"
 #include "remote_protocol.h"
 #include "../config.h"
 

qemud/protocol.c

-/*
- * Please do not edit this file.
- * It was generated using rpcgen.
- */
-
-#include "protocol.h"
-
-bool_t
-xdr_qemud_packet_header (XDR *xdrs, qemud_packet_header *objp)
-{
-
-	 if (!xdr_uint32_t (xdrs, &objp->length))
-		 return FALSE;
-	 if (!xdr_uint32_t (xdrs, &objp->prog))
-		 return FALSE;
-	return TRUE;
-}

qemud/protocol.h

-/*
- * Please do not edit this file.
- * It was generated using rpcgen.
- */
-
-#ifndef _PROTOCOL_H_RPCGEN
-#define _PROTOCOL_H_RPCGEN
-
-#include <rpc/rpc.h>
-
-
-#ifdef __cplusplus
-extern "C" {
-#endif
-
-#define QEMUD_PROGRAM 0x20001A64
-#define QEMUD_PKT_HEADER_XDR_LEN 8
-
-struct qemud_packet_header {
-	uint32_t length;
-	uint32_t prog;
-};
-typedef struct qemud_packet_header qemud_packet_header;
-
-/* the xdr functions */
-
-#if defined(__STDC__) || defined(__cplusplus)
-extern  bool_t xdr_qemud_packet_header (XDR *, qemud_packet_header*);
-
-#else /* K&R C */
-extern bool_t xdr_qemud_packet_header ();
-
-#endif /* K&R C */
-
-#ifdef __cplusplus
-}
-#endif
-
-#endif /* !_PROTOCOL_H_RPCGEN */

qemud/protocol.x

-/* -*- c -*-
- * protocol_xdr.x: wire protocol message format & data structures
- *
- * Copyright (C) 2006, 2007 Red Hat, Inc.
- * Copyright (C) 2006 Daniel P. Berrange
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2.1 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
- *
- * Author: Daniel P. Berrange <berrange@redhat.com>
- */
-
-
-/* The first two words in the messages are length and program number
- * (previously called "magic").  This makes the protocol compatible
- * with the remote protocol, although beyond the first two words
- * the protocols are completely different.
- *
- * Note the length is the total number of bytes in the message
- * _including_ the length and program number.
- */
-
-const QEMUD_PROGRAM = 0x20001A64;
-const QEMUD_PKT_HEADER_XDR_LEN = 8;
-
-struct qemud_packet_header {
-  uint32_t length;
-  uint32_t prog;
-};

qemud/qemud.c

@@ -1051,7 +1051,7 @@ static int qemudDispatchServer(struct qemud_server *server, struct qemud_socket
 
     if (!client->tls) {
         client->mode = QEMUD_MODE_RX_HEADER;
-        client->bufferLength = QEMUD_PKT_HEADER_XDR_LEN;
+        client->bufferLength = REMOTE_MESSAGE_HEADER_XDR_LEN;
 
         if (qemudRegisterClientEvent (server, client, 0) < 0)
             goto cleanup;
@@ -1180,7 +1180,7 @@ static void qemudDispatchClientRead(struct qemud_server *server, struct qemud_cl
     switch (client->mode) {
     case QEMUD_MODE_RX_HEADER: {
         XDR x;
-        qemud_packet_header h;
+        unsigned int len;
 
         if (qemudClientRead(server, client) < 0)
             return; /* Error, or blocking */
@@ -1190,33 +1190,32 @@ static void qemudDispatchClientRead(struct qemud_server *server, struct qemud_cl
 
         xdrmem_create(&x, client->buffer, client->bufferLength, XDR_DECODE);
 
-        if (!xdr_qemud_packet_header(&x, &h)) {
-            qemudDebug("Failed to decode packet header");
+        if (!xdr_u_int(&x, &len)) {
+            xdr_destroy (&x);
+            qemudDebug("Failed to decode packet length");
             qemudDispatchClientFailure(server, client);
             return;
         }
+        xdr_destroy (&x);
 
-        if (h.prog != REMOTE_PROGRAM) {
-            qemudDebug("Header magic %x mismatch", h.prog);
+        if (len > REMOTE_MESSAGE_MAX) {
+            qemudDebug("Packet length %u too large", len);
             qemudDispatchClientFailure(server, client);
             return;
         }
 
-        /* NB: h.length is unsigned. */
-        if (h.length > REMOTE_MESSAGE_MAX) {
-            qemudDebug("Packet length %u too large", h.length);
+        /* Length include length of the length field itself, so
+         * check minimum size requirements */
+        if (len <= REMOTE_MESSAGE_HEADER_XDR_LEN) {
+            qemudDebug("Packet length %u too small", len);
             qemudDispatchClientFailure(server, client);
             return;
         }
 
         client->mode = QEMUD_MODE_RX_PAYLOAD;
-        client->bufferLength = h.length;
+        client->bufferLength = len - REMOTE_MESSAGE_HEADER_XDR_LEN;
+        client->bufferOffset = 0;
         if (client->tls) client->direction = QEMUD_TLS_DIRECTION_READ;
-        /* Note that we don't reset bufferOffset here because we want
-         * to retain the whole message, including header.
-         */
-
-        xdr_destroy (&x);
 
         if (qemudRegisterClientEvent(server, client, 1) < 0) {
             qemudDispatchClientFailure(server, client);
@@ -1227,35 +1226,15 @@ static void qemudDispatchClientRead(struct qemud_server *server, struct qemud_cl
     }
 
     case QEMUD_MODE_RX_PAYLOAD: {
-        XDR x;
-        qemud_packet_header h;
-
         if (qemudClientRead(server, client) < 0)
             return; /* Error, or blocking */
 
         if (client->bufferOffset < client->bufferLength)
             return; /* Not read enough */
 
-        /* Reparse the header to decide if this is for qemud or remote. */
-        xdrmem_create(&x, client->buffer, client->bufferLength, XDR_DECODE);
-
-        if (!xdr_qemud_packet_header(&x, &h)) {
-            qemudDebug("Failed to decode packet header");
+        remoteDispatchClientRequest (server, client);
+        if (qemudRegisterClientEvent(server, client, 1) < 0)
             qemudDispatchClientFailure(server, client);
-            return;
-        }
-
-        if (h.prog == REMOTE_PROGRAM) {
-            remoteDispatchClientRequest (server, client);
-            if (qemudRegisterClientEvent(server, client, 1) < 0)
-                qemudDispatchClientFailure(server, client);
-        } else {
-            /* An internal error. */
-            qemudDebug ("Not REMOTE_PROGRAM");
-            qemudDispatchClientFailure(server, client);
-        }
-
-        xdr_destroy (&x);
 
         break;
     }
@@ -1336,7 +1315,7 @@ static void qemudDispatchClientWrite(struct qemud_server *server, struct qemud_c
         if (client->bufferOffset == client->bufferLength) {
             /* Done writing, switch back to receive */
             client->mode = QEMUD_MODE_RX_HEADER;
-            client->bufferLength = QEMUD_PKT_HEADER_XDR_LEN;
+            client->bufferLength = REMOTE_MESSAGE_HEADER_XDR_LEN;
             client->bufferOffset = 0;
             if (client->tls) client->direction = QEMUD_TLS_DIRECTION_READ;
 

qemud/remote.c

@@ -83,7 +83,7 @@ remoteDispatchClientRequest (struct qemud_server *server ATTRIBUTE_UNUSED,
 #include "remote_dispatch_localvars.h"
 
     /* Parse the header. */
-    xdrmem_create (&xdr, client->buffer+4, client->bufferLength-4, XDR_DECODE);
+    xdrmem_create (&xdr, client->buffer, client->bufferLength, XDR_DECODE);
 
     if (!xdr_remote_message_header (&xdr, &req)) {
         remoteDispatchError (client, NULL, "xdr_remote_message_header");

qemud/remote_protocol.h

@@ -743,6 +743,7 @@ enum remote_message_status {
 	REMOTE_ERROR = 1,
 };
 typedef enum remote_message_status remote_message_status;
+#define REMOTE_MESSAGE_HEADER_XDR_LEN 4
 
 struct remote_message_header {
 	u_int prog;

qemud/remote_protocol.x

@@ -717,6 +717,9 @@ enum remote_message_status {
     REMOTE_ERROR = 1
 };
 
+/* 4 byte length word per header */
+const REMOTE_MESSAGE_HEADER_XDR_LEN = 4;
+
 struct remote_message_header {
     unsigned prog;              /* REMOTE_PROGRAM */
     unsigned vers;              /* REMOTE_PROTOCOL_VERSION */

src/Makefile.am

@@ -61,7 +61,6 @@ CLIENT_SOURCES =						\
 		util.c util.h
 
 SERVER_SOURCES = 						\
-		../qemud/protocol.h ../qemud/protocol.c		\
 		../qemud/remote_protocol.c ../qemud/remote_protocol.h
 
 libvirt_la_SOURCES = $(CLIENT_SOURCES) $(SERVER_SOURCES)