From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCH] cld: use XDR for all messages Date: Wed, 13 Jan 2010 16:03:45 -0500 Message-ID: <4B4E3531.3010004@garzik.org> References: <1263135651-5407-1-git-send-email-cmccabe@alumni.cmu.edu> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=wcM8Y5CokRcbvs4a5NnnExkkX3kZZX7723ND6/5kc5I=; b=IcJhG97SARC9IoqPRdYInbxRpu+mZAfdWqJ6+0i7M3aha1P4FnWvqpbGTdeE8Q1CtM CVeffS1iewUR0TPXR4pMO27LyYyKFZ0ssWhEBrP316pODPE4vFf27YhFFUJXDQqGgwU5 tewF1szW+zGqQCt+x82w0XTpTUbCd15E8vt5o= In-Reply-To: <1263135651-5407-1-git-send-email-cmccabe@alumni.cmu.edu> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Colin McCabe Cc: Project Hail List , Pete Zaitcev On 01/10/2010 10:00 AM, Colin McCabe wrote: > This patch moves CLD from using manual data serialization to using XDR (via > rpcgen). Both the packet header and the message body are now serialized and > deserialized using XDR. This makes it easy to have a variable-length packet > header, as well as a variable-length message body. > > Message definitions are now in cld_msg.x. Helper functions for using messages > are in cld_fmt.c. The new client and server-side functions for sending > messages don't require the caller to precompute the size of the message. > Instead, you simply tell it what type of message you have, and give it the > data. It handles determining message size and encoding. > > The only fields that I did not choose to deal with using XDR are the packet > sequence ID and the packet SHA checksum. The seqid would have been > inconvenient and inefficient to store in the XDR header, because we have to > check a lot of packets' seqids against the ACKs we receive very frequently. > The SHA checksum can't be computed until the rest of the packet has been > serialized, which is counter to the way XDR works (it is a single-pass > serialization system.) So these fields are in the packet footer, which comes > at the end of every packet. > > This patch introduces a minor libcldc API change in struct cldc_call_opts. > > Signed-off-by: Colin McCabe > --- > .gitignore | 4 + > include/Makefile.am | 2 +- > include/cld_common.h | 10 + > include/cld_fmt.h | 78 ++++ > include/cld_msg.h | 252 ------------- > include/cldc.h | 37 +- > lib/Makefile.am | 14 +- > lib/cld_fmt.c | 189 ++++++++++ > lib/cld_msg_rpc.x | 212 +++++++++++ > lib/cldc.c | 960 +++++++++++++++++++++--------------------------- > lib/common.c | 6 +- > server/Makefile.am | 9 +- > server/cld.h | 97 ++++-- > server/cldb.h | 2 +- > server/msg.c | 304 ++++++---------- > server/server.c | 743 ++++++++++++++++++++----------------- > server/session.c | 396 ++++++++++----------- > test/load-file-event.c | 14 +- > test/save-file-event.c | 2 - > tools/cldcli.c | 22 +- > 20 files changed, 1744 insertions(+), 1609 deletions(-) > create mode 100644 include/cld_fmt.h > delete mode 100644 include/cld_msg.h > create mode 100644 lib/cld_fmt.c > create mode 100644 lib/cld_msg_rpc.x Well, this definitely does not build as-is. lib/Makefile.am needs BUILT_SOURCES = cld_msg_rpc.h otherwise nothing builds at all, because cld_msg_rpc.h does not exist. test/Makefile.am and tools/Makefile.am both need -I$(top_srcdir)/lib added to their INCLUDES. And you should probably examine http://www.gnu.org/software/automake/manual/automake.html#Clean to make sure that "make distclean" and "make clean" work. Still playing with it. You definitely need to make sure your patch builds and runs against a fresh git repo check + autogen.sh + ./configure run. Jeff