[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