[sheepdog] [PATCH 1/4] tests/fucntional: add a function for invoking sheep with zone

Hitoshi Mitake mitake.hitoshi at gmail.com
Sat Jan 11 12:09:16 CET 2014


At Sat, 11 Jan 2014 18:34:54 +0900,
Hitoshi Mitake wrote:
> 
> At Fri, 10 Jan 2014 18:00:14 +0800,
> Liu Yuan wrote:
> > 
> > On Fri, Jan 10, 2014 at 03:20:42PM +0900, Hitoshi Mitake wrote:
> > > Reported-by: Marcin Mirosław <marcin at mejor.pl>
> > > Signed-off-by: Hitoshi Mitake <mitake.hitoshi at lab.ntt.co.jp>
> > > ---
> > >  tests/functional/common.rc |   14 ++++++++++++--
> > >  1 file changed, 12 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tests/functional/common.rc b/tests/functional/common.rc
> > > index 3785bae..f1ee432 100644
> > > --- a/tests/functional/common.rc
> > > +++ b/tests/functional/common.rc
> > > @@ -282,7 +282,7 @@ _valgrind_dog()
> > >      return $ret
> > >  }
> > >  
> > > -_start_sheep()
> > > +_do_start_sheep()
> > >  {
> > >      # ensure that sheep is not running
> > >      local running=true
> > > @@ -304,13 +304,23 @@ _start_sheep()
> > >  	MD_STORE=",$STORE/$1/d0,$STORE/$1/d1,$STORE/$1/d2"
> > >      fi
> > >  
> > > -    $SHEEP $STORE/$1$MD_STORE -z $1 -p $((7000+$1)) -c $DRIVER $SHEEP_OPTIONS $2
> > > +    $SHEEP $STORE/$1$MD_STORE -z $2 -p $((7000+$1)) -c $DRIVER $SHEEP_OPTIONS $3
> > >  
> > >      if [ $? != 0 ]; then
> > >          _die "cannot start sheep $1"
> > >      fi
> > >  }
> > >  
> > > +_start_sheep()
> > > +{
> > > +    _do_start_sheep $1 $1 $2
> > > +}
> > > +
> > > +_start_sheep_with_zone()
> > > +{
> > > +    _do_start_sheep $1 $2 $3
> > > +}
> > > +
> > 
> > I think it would be better to add a more generic helper such as
> > 
> > _start_sheep_raw()
> > {
> > 	$SHEEP $STORE/$1$MD_STORE -p $((7000+$1)) -c $DRIVER $SHEEP_OPTIONS $2
> > }
> > 
> > which allow more flexity, which we can
> > 
> > _start_sheep_raw 1 "-z 1" which explicitly pass options as parameters and no
> > need to add burden to users that what is parameter list for this helper.
> 
> I agree. I'll implement it in v2.

On the second thought, _start_sheep_raw() seems to not a good
approach. If we employ this approach, preparing default values of
every parameter is required. And we need to replace the default values
with passed ones. It requires needless complexity and I think the zone
specific wrapper can be acceptable.

Thanks,
Hitoshi




More information about the sheepdog mailing list