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

[Liu Yuan]

On Sat, Jan 11, 2014 at 08:09:16PM +0900, Hitoshi Mitake wrote:
> 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.
> 

I don't think so. What you want is to add '-z x' to _start_sheep and not affect
old script. _start_sheep() already well perceived with first one as sheep
identity and then apending user-defined parameters as string. Based on this
convention, start a sheep with a specified zone should be:

  _start_sheep 1 "-z 1"

I think we can achieve this objective by modifying _start_sheep as following

_start_sheep()
{
    ...
    if '-z' not specified in the $2, then -z $1	
    ...
}

Then we can use _start_sheep to specify zones and don't affect other scripts.

Thanks
Yuan