Hi, thx for advice, ye sit is part of code, code is working fine.
i will fix CAPS on variables soon. quote is fixed. ________________________________ From: Brandon McCaig <bamcc...@gmail.com> To: Rajeev Prasad <rp.ne...@yahoo.com> Cc: perl list <beginners@perl.org> Sent: Tuesday, November 8, 2011 11:01 AM Subject: Re: is this ssh happenign in parallel? Hello: I'm not familiar with Net::OpenSSH so I can't help with that, but I can point out some questionable code and suggest improvements. On Mon, Nov 07, 2011 at 03:56:17PM -0800, Rajeev Prasad wrote: > my @hosts =qw(...); #all hosts > my @allcmds = qw(...); #all commands I don't think it really makes sense to use the qw operator for a list of commands (unless the commands just happen to not require arguments, but that's rare). :\ I'll just trust that you know what you're doing. :P Otherwise, you should use Data::Dumper to ensure that your @allcmds array is built as expected. > for my $host (@hosts){ #loop to process all host in list > chomp($host); Since you are using the qw operator to assign @hosts it doesn't really make sense to chomp it... I'll assume that you're actually doing something different than what you've shown here. > $ssh{$host} = Net::OpenSSH->new($host, > port => $SSHPORT, > user => $USER, > password => $PASS, > default_stderr_fh => $stderr_fh, > default_stdout_fh => $stdout_fh, > async => 1, > master_opts => [-o => 'StrictHostKeyChecking=no', > -o => 'ConnectTimeout 10'], > ); > } > > > open(MFOH,">>$MFRESULT"); You should generally use lexical file handles and the 3-argument version of open. You should also be checking for and handling the potential failure of open (e.g., with or die) (perhaps you're using autodie instead, but it isn't shown so there's no way to know). > for my $host (@hosts) { > foreach $CMD (@allcmds){ > chomp($CMD); > ( @CMDRESULT, $CMDERR ) = $ssh{$host}->capture("$CMD"); These should probably be lexical variables (but it isn't clear since parts of the program appear to be missing). That suggests also that you aren't using the 'strict' pragma, but you should be. It's possible that you've declared these variables globally, but it doesn't seem warranted. Also, you probably shouldn't be quoting $CMD here since IIRC it can only be a string when assigned with qw. It's useless, potentially wasteful, and in other circumstances potentially erroneous to quote a variable. Also, since @CMDRESULT is an array, it will consume all elements of the list returned by Net::OpenSSH::capture. IOW, $CMDERR can never get a value other than undef from the above assignment. Lastly, it's distracting and probably bad style to use uppercase identifiers here. > print MFOH "\n$host COMMAND: $CMD\n"; > foreach my $line (@CMDRESULT) { print MFOH "$host $line"; } > @CMDRESULT = ""; > if (defined $CMDERR ) {print MFOH "$host $CMDERR \n"; $CMDERR = > "";} This if statement is useless because as mentioned above $CMDERR will never be defined. > } > } > close MFOH; > > .... HTH, -- Brandon McCaig <bamcc...@gmail.com> <bamcc...@castopulence.org> Castopulence Software <https://www.castopulence.org/> Blog <http://www.bamccaig.com/> perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }. q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.}; tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'