[Libvir] PATCH: Remote protocol receive+dispatch cleanup

Daniel P. Berrange berrange at redhat.com
Wed Oct 17 20:26:36 UTC 2007


Dating the from time when we had both the QEMU & generic remote protocol
there are tiny qemud/protocol.{x,c,h} files basically just containing
one struct for the packet header. This is more or less duplicated in the
remote protocol & with a few small tweaks can simplify this code. So the
attached patch removes the protocol.{x,c,h} files. The qemud.c file in
the 'receive header' branch now merely reads a single 4 byte unsigned
int to discover the packet length. The rest of the packet is dealt with
in the 'receive payload' branch. It immediately dispatches to the remote
program. There is no wire format change here - shouuld just be code cleanup

 a/qemud/protocol.c      |   17 --------------
 a/qemud/protocol.h      |   39 --------------------------------
 a/qemud/protocol.x      |   40 ---------------------------------
 qemud/Makefile.am       |    5 ----
 qemud/internal.h        |    1 
 qemud/qemud.c           |   57 +++++++++++++++---------------------------------
 qemud/remote.c          |    2 -
 qemud/remote_protocol.h |    1 
 qemud/remote_protocol.x |    3 ++
 src/Makefile.am         |    1 
 10 files changed, 24 insertions(+), 142 deletions(-)


Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 
-------------- next part --------------
diff -r ee2425c4d587 qemud/Makefile.am
--- a/qemud/Makefile.am	Wed Oct 17 11:35:03 2007 -0400
+++ b/qemud/Makefile.am	Wed Oct 17 15:02:48 2007 -0400
@@ -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.s
 
 libvirtd_SOURCES = \
 		qemud.c internal.h \
-		protocol.h protocol.c \
 		remote_protocol.h remote_protocol.c \
 		remote.c \
 		event.c event.h
@@ -76,7 +74,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 \
diff -r ee2425c4d587 qemud/internal.h
--- a/qemud/internal.h	Wed Oct 17 11:35:03 2007 -0400
+++ b/qemud/internal.h	Wed Oct 17 15:02:48 2007 -0400
@@ -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"
 
diff -r ee2425c4d587 qemud/protocol.c
--- a/qemud/protocol.c	Wed Oct 17 11:35:03 2007 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,17 +0,0 @@
-/*
- * 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;
-}
diff -r ee2425c4d587 qemud/protocol.h
--- a/qemud/protocol.h	Wed Oct 17 11:35:03 2007 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,39 +0,0 @@
-/*
- * 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 */
diff -r ee2425c4d587 qemud/protocol.x
--- a/qemud/protocol.x	Wed Oct 17 11:35:03 2007 -0400
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,40 +0,0 @@
-/* -*- 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 at 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;
-};
diff -r ee2425c4d587 qemud/qemud.c
--- a/qemud/qemud.c	Wed Oct 17 11:35:03 2007 -0400
+++ b/qemud/qemud.c	Wed Oct 17 15:02:48 2007 -0400
@@ -1051,7 +1051,7 @@ static int qemudDispatchServer(struct qe
 
     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(stru
     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(stru
 
         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;
         }
-
-        if (h.prog != REMOTE_PROGRAM) {
-            qemudDebug("Header magic %x mismatch", h.prog);
+        xdr_destroy (&x);
+
+        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(stru
     }
 
     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(str
         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;
 
diff -r ee2425c4d587 qemud/remote.c
--- a/qemud/remote.c	Wed Oct 17 11:35:03 2007 -0400
+++ b/qemud/remote.c	Wed Oct 17 15:03:02 2007 -0400
@@ -83,7 +83,7 @@ remoteDispatchClientRequest (struct qemu
 #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");
diff -r ee2425c4d587 qemud/remote_protocol.h
--- a/qemud/remote_protocol.h	Wed Oct 17 11:35:03 2007 -0400
+++ b/qemud/remote_protocol.h	Wed Oct 17 15:02:48 2007 -0400
@@ -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;
diff -r ee2425c4d587 qemud/remote_protocol.x
--- a/qemud/remote_protocol.x	Wed Oct 17 11:35:03 2007 -0400
+++ b/qemud/remote_protocol.x	Wed Oct 17 15:02:48 2007 -0400
@@ -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 */
diff -r ee2425c4d587 src/Makefile.am
--- a/src/Makefile.am	Wed Oct 17 11:35:03 2007 -0400
+++ b/src/Makefile.am	Wed Oct 17 15:02:48 2007 -0400
@@ -60,7 +60,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)


More information about the libvir-list mailing list