On Tue, Nov 06, 2001 at 10:57:36PM +0100, Tommy Rohde wrote:
> This works fine as long as i only have one node in the nodelist.. if i
> have more it just goes:
>
> $ ./ftpbackup.pl filelist.txt hostlist.txt <username> <password>
> /remote/directory
> Connecting to host1.com
> Getting file1
> Getting file2
> Connecting to host2.com
This is because you read all of the data in your file list. Since there is
no more data to read, your while (<FilesToGet>) loop immediately fails on
the next host. The solution is to seek to the beginning of FilesToGet at
the end of HOSTLOOP, or read all of the files in FilesToGet into an array
and iterate over that.
Some commentary on your code and additions to make it work better follows:
> #!/usr/bin/perl -w
> #
> # Comments are allowed in both the file and host list if
> # the comment row begins with a '#' sign.
> # Entries in the file and host list should be seperated with
> # a new row. spaces or other signs will make the script fail.
> #
> use Net::FTP;
> use strict;
Add:
use Fcntl qw(SEEK_SET);
for the later seeking we have to do.
> my $usage = "$0 filelist hostlist username password directory\n";
> my $filelist = $ARGV[0] or die "$usage";
> my $hostlist = $ARGV[1] or die "$usage";
> my $username = $ARGV[2] or die "$usage";
> my $password = $ARGV[3] or die "$usage";
> my $home = $ARGV[4] or die "$usage";
A simpler way of doing this would be:
die("$0 filelist hostlist username password directory\n") unless @ARGV == 5;
my($filelist, $hostlist, $username, $password, $home) = @ARGV;
It's also more correct if 0 is a valid value for one of the arguments.
> my $debug = 5;
> my $backupnode = undef;
Given that $backupnode is used exclusively in the HOSTLOOP loop, and that
it's set everytime, perhaps you should localize it to that loop, instead of
making it global.
> open(FilesToGet, $filelist)
> or die "can't open filelist $filelist: $!\n";
>
> open(HostsToGet, $hostlist)
> or die "can't open hostlist $hostlist: $!\n";
>
> HOSTLOOP:
> while (<HostsToGet>) {
I generally name my loop labels so that they're more readable. Instead of
"next HOSTLOOP" I'd want to say "next HOST". I also prefer to label my
loops on the same line.
So:
HOST: while (<HostsToGet>) {
These are, of course, simply suggestions.
> next HOSTLOOP if /#/;
> print STDERR "Connecting to $_" if $debug > 3;
> chomp;
> $backupnode = $_;
So here you'd say, in keeping with my suggestion to localize it to this
loop:
my $backupnode = $_;
Although I'm at a loss as to why you're using a seperate variable for this.
Why not just use $_?
> FILELOOP:
> while (<FilesToGet>) {
> next FILELOOP if /#/;
> my $ftp = Net::FTP->new($backupnode);
> $ftp->login($username, $password);
> $ftp->cwd($home);
> print STDERR "Getting $_" if $debug > 3;
> chomp;
> $ftp->get($_,"$backupnode." . $_);
> $ftp->quit;
You have some odd indentation going on here.
> }
Here's where you'd add your seek:
seek(FilesToGet, 0, SEEK_SET);
You may also want to add an 'or die(...)' just to make sure it was
successful.
> }
Michael
--
Administrator www.shoebox.net
Programmer, System Administrator www.gallanttech.com
--
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
