[Linux-HA] mach_down bug/patch (was: kill -9 to heartbeat to test STONITH fails takeover)

[Horms]

On Wed, Jun 22, 2005 at 11:02:09AM +0900, Horms wrote:
> On Tue, Jun 21, 2005 at 06:46:11PM -0500, Dave Dykstra wrote:
> > I found out what was causing my STONITH tests to fail intermittently.
> > I've now been using 'halt -f' to test STONITH and ran into the problem
> > again.  It turns out the problem is the line
> >      mdown=`echo $1 | tr [A-Z] [a-z]`; # The name of the downed machine...
> > in /usr/lib/heartbeat/mach_down in that the [A-Z] and [a-z] are not
> > quoted from the shell: if it happens to be run from a directory that has
> > more than one file with a one-character file name in it (more precisely,
> > more than one single-letter lower-case file name or more than one
> > single-letter upper-case file name), the shell translates one of those
> > wildcards into a usage error for tr.  Here's a patch:
> > 
> > --- heartbeat/lib/mach_down.in.O        2005-06-21 18:43:41.000000000 -0500
> > +++ heartbeat/lib/mach_down.in  2005-06-21 18:43:57.000000000 -0500
> > @@ -10,7 +10,7 @@
> >  
> >  : Now running $0: $*
> >  
> > -mdown=`echo $1 | tr [A-Z] [a-z]`; # The name of the downed machine...
> > +mdown=`echo $1 | tr "[A-Z]" "[a-z]"`; # The name of the downed machine...
> >  
> >  #ha_log "info: DEBUG: $0 -- taking resources for $mdown"
> 
> That fix looks good to me, but I believe these kind of problems
> are rife within the tree and the proliferation of scripts in core
> parts of linux-ha makes this problem worse.
> 
> For instance, in the same script, on the same line, are
> we sure that $1 contains no special shell sequences,
> for instance *. I think the following fix caches all
> the possible shell interpreattion of variables for this script.

Alan sent a reply to my concerns, which unfortuantely I don't have
handy right now. He made a few good points about context, but I think
overall we will have to agree to disagree on this one. Certainly,
making wholesale chanves to the code, such as the patch I submitted,
is problematic, and probably best left undone. To this end,
are there any objections to applying Dave's original patch, which
does fix the problem at hand?

-- 
Horms