Hi Hideo-san,

On Mon, Jan 16, 2012 at 09:02:25AM +0900, [email protected] wrote:
> Hi Dejan,
> 
> Thank you for comments.
> 
> > > This patch revises the next point.
> > > 
> > >  * When route has been already assigned, RA skips an allotment. 
> > 
> > Is this just a performance improvement? Or did you see anything
> > wrong happen when running the current code?
> 
> The problem is not taking place.
> We found this waste in a review.
> I think that this waste may influence a performance.
> 
> > 
> > >  * Added error log of FINDIF.
> > >  * Deleted the unused sentence.
> > 
> > It would be good to have at least two patches, because we should
> > always try to have patches with single self-contained
> > modification.
> 
> Sorry..
> Because I was small, I did not divide this patch into one patch.

OK. I split them and applied.

Cheers,

Dejan


> Best Regards,
> Hideo Yamauchi.
> 
> --- On Sat, 2012/1/14, Dejan Muhamedagic <[email protected]> wrote:
> 
> > Hi Hideo-san,
> > 
> > Sorry for picking up this so late.
> > 
> > On Tue, Nov 29, 2011 at 02:48:52PM +0900, [email protected] wrote:
> > > Hi All,
> > > 
> > > We made a patch to IPsrcaddr.
> > > 
> > > This patch revises the next point.
> > > 
> > >  * When route has been already assigned, RA skips an allotment. 
> > 
> > Is this just a performance improvement? Or did you see anything
> > wrong happen when running the current code?
> > 
> > >  * Added error log of FINDIF.
> > >  * Deleted the unused sentence.
> > 
> > It would be good to have at least two patches, because we should
> > always try to have patches with single self-contained
> > modification.
> > 
> > Cheers,
> > 
> > Dejan
> > 
> > > Please please confirm my correction. 
> > > And please commit a correction. 
> > > 
> > > Best Regards,
> > > Hideo Yamauchi
> > 
> > > diff -r 2107bc4f5c8b heartbeat/IPsrcaddr
> > > --- a/heartbeat/IPsrcaddr    Thu Nov 24 14:13:11 2011 +0900
> > > +++ b/heartbeat/IPsrcaddr    Thu Nov 24 14:13:53 2011 +0900
> > > @@ -167,13 +167,20 @@
> > >  srca_start() {
> > >      srca_read $1
> > >  
> > > -    ip route replace $NETWORK dev $INTERFACE src $1 || \
> > > -        errorexit "command 'ip route replace $NETWORK dev $INTERFACE src 
> > > $1' failed"
> > > +    rc=$?
> > > +    if [ $rc = 0 ]; then 
> > > +        rc=$OCF_SUCCESS
> > > +        ocf_log info "The ip route has been already set.($NETWORK, 
> > > $INTERFACE, $ROUTE_WO_SRC)"
> > > +    else
> > > +        ip route replace $NETWORK dev $INTERFACE src $1 || \
> > > +            errorexit "command 'ip route replace $NETWORK dev $INTERFACE 
> > > src $1' failed"
> > >  
> > > -    $CMDCHANGE $ROUTE_WO_SRC src $1 || \
> > > -        errorexit "command '$CMDCHANGE $ROUTE_WO_SRC src $1' failed"
> > > +        $CMDCHANGE $ROUTE_WO_SRC src $1 || \
> > > +            errorexit "command '$CMDCHANGE $ROUTE_WO_SRC src $1' failed"
> > > +        rc=$?
> > > +    fi
> > >  
> > > -    return $?
> > > +    return $rc
> > >  }
> > >  
> > >  #
> > > @@ -252,7 +259,6 @@
> > >      else
> > >          true
> > >      fi       
> > > -#    return $OCF_SUCCESS
> > >      ;;
> > >      *) #less than three decimal dots
> > >      false;;
> > > @@ -377,7 +383,6 @@
> > >        
> > >        Linux|SunOS)        
> > >        IF=`find_interface "$BASEIP"`
> > > -#      echo $IF
> > >        if [ -z "$IF" ]; then
> > >            return $OCF_NOT_RUNNING
> > >        fi
> > > @@ -455,7 +460,11 @@
> > >  
> > >  findif_out=`$FINDIF -C`
> > >  rc=$?
> > > -[ $rc -ne 0 ] && exit $rc
> > > +[ $rc -ne 0 ] && {
> > > +    ocf_log err "[$FINDIF -C] failed"
> > > +    exit $rc
> > > +}
> > > +
> > >  INTERFACE=`echo $findif_out | awk '{print $1}'`
> > >  NETWORK=`ip route list dev $INTERFACE scope link|grep -o '^[^ ]*'`
> > >  
> > 
> > > _______________________________________________________
> > > Linux-HA-Dev: [email protected]
> > > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> > > Home Page: http://linux-ha.org/
> > 
> > 
> _______________________________________________________
> Linux-HA-Dev: [email protected]
> http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
> Home Page: http://linux-ha.org/
_______________________________________________________
Linux-HA-Dev: [email protected]
http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev
Home Page: http://linux-ha.org/

Reply via email to