[Dirvish] rsyncd, the initial colon, and BadShellCommandCWD

Keith Lofstrom keithl at kl-ic.com
Tue Jan 18 18:49:48 PST 2005


Way back in September, Eric Mountain posted the suggested 
BadShellCommandCWD patch to the wiki.  This patch is designed to fix
the problem with pre-client and post-client scripts being started in
the wrong directory.  Looking at the existing logic (starting with
version 1.1), dirvish will only change the working directory to $A{dir}
if it starts with a colon.  Eric's fix reverses the logic, changing the
working directory if it does NOT start with a colon.

Original:
------------
        if ($A{dir} =~ /^:/)
        {
                ... with cwd, if colon ...
        } else {
                ... without cwd, if no colon ...
        }
------------

Eric's fix:
------------
        if ($A{dir} !~ /^:/)
        {
                ... with cwd, if no colon ...
        } else {
                ... without cwd, if colon ...
        }
------------

It took me a while to figure out what JW intended:  if you look at 
dirvish.conf, you will see:

------------
 tree: path [alias] (S)
           Specify a directory path on the client to backup.

           If path is prefixed with a colon the transfer will be done  from
           an  rsync  daemon  on  the client otherwise the transfer will be
           done through a remote shell process.
------------

So the colon is a flag to use rsync in daemon mode, which is called
"rsyncd" by the clueful, and rsync --daemon by the rest of us.  If
you look at CHANGELOG, you will see the following comment about the
time this behavior was added to the subroutine scriptrun :

------------
  2002/08/08           TODO.html            1.18
                       dirvish              1.18
                       TODO.html            1.17
                       dirvish              1.17
                       dirvish-runall.8     1.2
  pre/post client scripts won't try to cd srctree if using
  rsyncd on client.
------------

So it looks like this is a simple logical inversion error by JW;
both behaviors of the if make sense given the man page description
of the tree: option, if you reverse them.

One remaining question is, what if $A{dir} (which is now either
$srctree or $destree) is empty?   With Eric's fix, scriptrun would
run the "with cwd" commands; which would be incorrect.  However,
dirvish never gets there;  $srctree is set (more or less) by:

($srctree, $aliastree) = split(/\s+/, $$Options{tree})
        or seppuku 228, "ERROR: no source tree defined";

... so it does a seppuku if $$Options{tree}, and thus $srctree, is empty.

$destree is set by:

  $destree = join("/", $vault, $image, 'tree');

... so it is never empty, either.  Thus we never encounter the "empty
dir" problem.  I am assuming that we never will, but this doesn't seem
very robust - who knows what some almost-clueful programmer will do with
scriptrun in the future?

Another question is, how do we signal rsync to use daemon mode?  Well,
this is a bit of kludge, but the normal (non-daemon) client path is
specified as client:/path/to/source/files .  If you instead call rsync
with client::/path/to/source/files , you will turn on daemon behavior.
So, if we join "client:" and ":/path/to/source/files", we get the
appropriate source string.  Either very clever or very confusing.

All that said, Eric's fix looks correct.  It is interesting that
the misbehavior wasn't noticed by any users until Eric spotted it. 
Forgive my delay in responding, but I want us to understand these
patches completely before we fool around with the behavior of mission
critical programs.  Patches that come with explanations like the 
above will be consider much more quickly than patches without.

Keith

-- 
Keith Lofstrom          keithl at keithl.com         Voice (503)-520-1993
KLIC --- Keith Lofstrom Integrated Circuits --- "Your Ideas in Silicon"
Design Contracting in Bipolar and CMOS - Analog, Digital, and Scan ICs


More information about the Dirvish mailing list