From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCHv2] cld: use XDR for all messages Date: Wed, 20 Jan 2010 20:38:39 -0500 Message-ID: <4B57B01F.5080903@garzik.org> References: <1263577788-2081-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=KPloVBemRvWod3PwKFgIxMYF91tYf/qqNVVX+HlmIT8=; b=fTv+Kqysdsep9XIJQVCwyzbKEBDRmJhDpc02RN3aa4ToeRilDk6oQuMIDc68KW8A1+ lEh3TkhPzzXnwWoIMc2bZFdasmWNCQiEd4yP8Ffu9a61wNNNkwNA5xYSeGAt2DcIlXYO bw6sOWxCvPYhqAcjgT58hSLdCSnjmMb7XaqBg= In-Reply-To: <1263577788-2081-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/15/2010 12:49 PM, 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. > > This patch introduces a minor libcldc API change in struct cldc_call_opts. > > v2 changes: > * Added __cld prefix to functions in cld_fmt.c > > * When decoding CMT_GET messages, we should store the payload in the > session structure, rather than in cldc_call_opts. > > * Got rid of pkt_is_first, pkt_is_last, in favor of a flags-based approach. > > * Killed CLD_MAX_PKT_MSG. It's more efficient to only allocate as much space > as you need, rather than always allocating space for 128 packets in a message. > > * Created CLD_MAX_PAYLOAD_SZ to mean the maximum size of the data that can be > sent with GET or PUT. This is different (and smaller than!) the maximum > message size. > > * automake: Add cld_msg_rpc.h to BUILT_SOURCES > > * automake: Add lib dir to INCLUDES > > * automake: "make clean" now deletes XDR build products > > Signed-off-by: Colin McCabe > --- > .gitignore | 4 + > include/Makefile.am | 2 +- > include/cld_common.h | 10 + > include/cld_fmt.h | 89 +++++ > include/cld_msg.h | 252 ------------- > include/cldc.h | 36 +- > lib/Makefile.am | 19 +- > lib/cld_fmt.c | 193 ++++++++++ > lib/cld_msg_rpc.x | 218 +++++++++++ > lib/cldc.c | 966 +++++++++++++++++++++--------------------------- > 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/Makefile.am | 5 +- > test/load-file-event.c | 14 +- > test/save-file-event.c | 2 - > tools/cldcli.c | 22 +- > 21 files changed, 1777 insertions(+), 1612 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 Looking good... we are definitely getting closer. Currently, it still does not pass "make distcheck", which presages a lot of installation problems. Notably, #include is not consistent with the rest of the headers, producing things such as "make distcheck" build breakage: > libtool: compile: gcc -DHAVE_CONFIG_H -I. -I../../lib -I.. -I../../include -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -g -O2 -MT cldc.lo -MD -MP -MF .deps/cldc.Tpo -c ../../lib/cldc.c -fPIC -DPIC -o .libs/cldc.o > In file included from ../../lib/cldc.c:37: > ../../include/cldc.h:26:29: error: lib/cld_msg_rpc.h: No such file or directory You will either need to move the header to $cld/include/ or update INCLUDES in various */Makefile.am files to reference $(top_srcdir)/lib (which I see you actually did, in a few cases). Also, it would be nice if this did not introduce warnings. A big point with the testsuite, and minor point with compiler warnings, is that -- more than anything else -- we do not want to introduce regressions. Jeff