Re: [PATCH] Re: [PATCHv2] cld: use XDR for all messages

2010-02-05 Thread Jeff Garzik

On 02/03/2010 04:45 PM, Colin McCabe wrote:

On Tue, Feb 2, 2010 at 10:35 PM, Jeff Garzikj...@garzik.org  wrote:

I will continue whittling down the patch until it just contains the XDR
changes themselves.


In tools/Makefile.am, I don't think you need $(top_srcdir)/lib any
more, since cld_msg_rpc.h is now an installed header.



Build environment should always refer to the current in-tree header, not 
the installed header.  The header might not be installed (yet, at build 
time), or it might be an older version of the header you need.  Who 
knows what lives in system headers :)


Jeff


--
To unsubscribe from this list: send the line unsubscribe hail-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Re: [PATCHv2] cld: use XDR for all messages

2010-02-03 Thread Colin McCabe
On Tue, Feb 2, 2010 at 10:35 PM, Jeff Garzik j...@garzik.org wrote:
 On 01/29/2010 08:01 PM, Colin McCabe wrote:

 That seems reasonable. All of those functions could be looked at as
 common.

 The intention was to group together a bunch of functions that were
 useful for packet formatting. Although the bulk of the formatting is
 done by XDR, there are some things that XDR doesn't do, like
 generating and checking SHA sums. cld_dump_buf and cld_pkt_hdr_to_str
 are purely for debugging purposes, but they seemed like something that
 would be generally useful for developers making packet format changes.
 I know that they were useful to me.

 I finally pushed out several changes split off from your main XDR patch, to
 the upstream cld git repo.  It took longer than expected because I would
 make changes of my own along the way, which, each time, necessitated a
 rediff+rebuild between current cld working tree and cld + xdr working
 tree.

 The attached patch is what remains to be split up and committed.  I have
 definitely whittled it down, and along the way, moved and renamed some
 things on my own.  With regards to cld_fmt.*, my main objection was to
 creating too many to-be-installed headers.  One more header can be a pain
 for us and for users, while one more source file in cld/lib/ is no big deal.

 Thus, the intention is to eliminate cld_fmt.h (newly renamed to cld_pkt.h)
 altogether, while keeping the arrangement you created in cld_fmt.c (newly
 renamed to lib/pkt.c).

Yeah, after I looked at the v2 patch I realized it wasn't separated
out well enough.
So I was working on a v3 patch series that had the following changes:

1. kill CLD_MAX_PKT_MSG and add CLD_MAX_PAYLOAD_SZ
2. reformat Makefile.am files (to one file per line for nicer diffs)
3. move cld_msg macros into cld_common
4. some style issues
5. add cld_fmt.c (now called cld_pkt.c)
6. create cldc_call_opts_get_data and switch to using it in test/ and tool/
7. the big XDR changes.

It looks like 2, 3, 4, 5 have already come to pass (thanks Jeff!)

I just sent out #1 and another patch that is related to CLD_INODE_NAME_MAX
I think I'll send out a patch for #6 soon.

 I will continue whittling down the patch until it just contains the XDR
 changes themselves.

In tools/Makefile.am, I don't think you need $(top_srcdir)/lib any
more, since cld_msg_rpc.h is now an installed header.

Colin
--
To unsubscribe from this list: send the line unsubscribe hail-devel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html