[sheepdog] [PATCH v3] tests: add a DynamoRIO client for testing the jounaling mechanism

Hitoshi Mitake mitake.hitoshi at gmail.com
Wed Jul 3 06:29:25 CEST 2013


At Wed, 03 Jul 2013 12:53:36 +0900,
MORITA Kazutaka wrote:
> 
> At Wed, 03 Jul 2013 10:36:15 +0900,
> Hitoshi Mitake wrote:
> > 
> > > 
> > > This must be as same as the one in sheep/journal.c and I think it's
> > > likely to be broken when we update the definition.  I think we should
> > > define journal_descriptor in include/internal_proto.h and this file
> > > should include the header file.
> > 
> > Moving the definition of the struct would break the intention of the
> > journaling implementation. So I thought it shouldn't be done for
> > testing purpose.
> 
> I don't think so.  From the point of view of maintainability, we
> shouldn't have the same struct definitions at different places.  I
> think moving journal_descriptor from sheep/journaling.c to
> include/internal_proto.h is okay.  The header file contains sheep
> specific proto types but can be accessed by another program.
> 
> > 
> > There might be some effective ways for obtaining these kind of
> > information from the code of sheep.
> > 
> > 1. parsing DWARF information and extracting definitions of data types
> > 2. employing static analysis techniques which work on source code
> 
> Those look really unacceptable to me.  Too hacky.

OK, I'll move the definition to include/internal_proto.h in v4.

> 
> > > No plan to use DynamoRIO for other sheepdog features than journaling?
> > > I guess some functions in this file should be defined in a more common
> > > place like common.c.
> > 
> > Of course I'll use DR for testing other parts of sheepdog
> > (e.g. shepherd). But I believe extracting common functions as APIs
> > should be done when we add 2nd one for better desgin. How do you
> > think?
> 
> If you have a plan to add other tests, I think it's better to place
> the common functions into, e.g, common.c from the first
> implementation.

OK, I'll do this cleaning in v4, too.

Thanks,
Hitoshi



More information about the sheepdog mailing list