Rob Dixon wrote:
Hi Paul and welcome to the list.
I can see a few things wrong with your code, but I have only a Windows
machine so cannot test any changes I am suggestion so please beware.
The reason you get the marked line in your output is because that is
what you have written. This loop
open(EXT,"/usr/sbin/pvdisplay $PV|");
while(<EXT>) {
print $_;
if (/Free PE/) {
$AllocatedPE = (split(/\s+/, <EXT>))[2];
$rec->{$PV} = $AllocatedPE;
print "$PV ";
print $rec->{$PV};
print " \n";
push @vggroup, $rec;
}
}
reads a line from the pvdisplay output and checks whether it contains
'Free PE'. If it does, this
$AllocatedPE = (split(/\s+/, <EXT>))[2];
reads another line (the 'Allocated PE' line), splits it on white space
and puts the third field - the extent - into $AllocatedPE. The code then
goes on to add a single hash element to $rec and then print the volume
name and extent.
The line read within the call to split is never displayed, and the loop
goes on to read and display the line after it in the while loop.
Another problem is that you are pushing the same hash reference $rec
onto @vggroup several times. There is no need for the array, as the hash
holds the information on all the volumes in the group on its own.
Other advice that may help is:
- Perl variables generally use lower-case letters and underscores, and
should be declared at the point of first use
- Lexical file handles should be used with the three-argument form of
open, and the status of /all/ opens should be checked before the file
handle is used
- By default the split operator splits $_ on whitespace, so split on its
own returns a list of separate non-whitespace fields found in $_. This
is usually what is wanted, as is the case here
- There may well be a module already written to do this. It is worh a
look at http://search.cpan.org unless your purpose is to learn the language
Below is a refactored version of your program taking these things into
account. As I explained I am unable to test it so I can only confirm
that it compiles.
HTH,
Rob
use strict;
use warnings;
open my $cmd, '-|', "/usr/sbin/vgdisplay -v vg03" or die $!;
You might as well go all the way:
open my $cmd, '-|', '/usr/sbin/vgdisplay', '-v', 'vg03'
or die "Cannot open pipe from vgdisplay because: $!";
my @pv;
while (<$cmd>) {
if (/PV Name/) {
my $pv = (split)[2];
push @pv, $pv;
}
}
And don't forget to verify that the pipe closed correctly:
close $cmd or warn $! ? "Error closing vgdisplay pipe: $!"
: "Exit status $? from vgdisplay";
Another way to populate @pv:
my @pv = `/usr/sbin/vgdisplay -v vg03` =~ /PV Name\s+(\S+)/g;
print "@pv\n";
my $rec;
foreach my $pv (@pv) {
open my $ext, '-|', "/usr/sbin/pvdisplay $pv" or die $!;
open my $ext, '-|', '/usr/sbin/pvdisplay', $pv
or die "Cannot open pipe from pvdisplay because: $!";
while(<$ext>) {
print;
if (/Allocated PE/) {
my $allocated_pe = (split)[2];
$rec->{$pv} = $allocated_pe;
print "$pv => $allocated_pe\n";
}
}
close $ext or warn $! ? "Error closing pvdisplay pipe: $!"
: "Exit status $? from pvdisplay";
}
use Data::Dumper;
print Dumper $rec;
John
--
Any intelligent fool can make things bigger and
more complex... It takes a touch of genius -
and a lot of courage to move in the opposite
direction. -- Albert Einstein
--
To unsubscribe, e-mail: beginners-unsubscr...@perl.org
For additional commands, e-mail: beginners-h...@perl.org
http://learn.perl.org/
