[ofa-general] [PATCH] infiniband-diags: terminate perl scripts with error if not root
Sasha Khapyorsky
sashak at voltaire.com
Sun May 25 12:10:47 PDT 2008
Hi Tim, Ira,
On 08:58 Fri 23 May , Timothy A. Meier wrote:
>
> Following Hals advice, authorization is based on the umad permissions.
I will send some more comments about this method later today. But
basically still think that some things could be broken and that it is
not really trivial to separate in this way wrong usage from desired
behavior reliably (with some approximation it is possible of course).
> The intent is simply to provide a consistent and
> non-silent fail mechanism.
OTOH I fully agree with yours and Ira's arguments about this - 'Silent'
fails are bad. I thought about how to solve this and and started to run
diag perl scripts from unprivileged account in various conditions (cache
file exists or not, cache dir is readable or not, etc.).
First thing I saw was that even on bad usage most scripts return 0. Then
I found that on many failures return status is not checked or ignored
and program return 0. I did those two patches (below) and up to now it
works fine for me (but likely I didn't cover everything). What do you
say?
Sasha
>From cbbc155996c9f6efe91b78f055a643809b997468 Mon Sep 17 00:00:00 2001
From: root <root at castor.voltaire.com>
Date: Sat, 24 May 2008 11:04:08 +0300
Subject: [PATCH] infiniband-diags/scripts/*.pl: exit 2 on usage errors
Add non-zero exit status (2) on usage errors for perl scripts.
Signed-off-by: root <root at castor.voltaire.com>
---
infiniband-diags/scripts/check_lft_balance.pl | 2 +-
infiniband-diags/scripts/ibfindnodesusing.pl | 2 +-
infiniband-diags/scripts/ibidsverify.pl | 2 +-
infiniband-diags/scripts/iblinkinfo.pl | 2 +-
infiniband-diags/scripts/ibprintca.pl | 2 +-
infiniband-diags/scripts/ibprintrt.pl | 2 +-
infiniband-diags/scripts/ibprintswitch.pl | 2 +-
infiniband-diags/scripts/ibqueryerrors.pl | 2 +-
infiniband-diags/scripts/ibswportwatch.pl | 2 +-
9 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/infiniband-diags/scripts/check_lft_balance.pl b/infiniband-diags/scripts/check_lft_balance.pl
index 66f5f0f..b0f0fef 100755
--- a/infiniband-diags/scripts/check_lft_balance.pl
+++ b/infiniband-diags/scripts/check_lft_balance.pl
@@ -70,7 +70,7 @@ sub usage
print "Usage: $prog [-R -v]\n";
print " -R recalculate all cached information\n";
print " -v verbose output\n";
- exit 0;
+ exit 2;
}
sub is_port_up
diff --git a/infiniband-diags/scripts/ibfindnodesusing.pl b/infiniband-diags/scripts/ibfindnodesusing.pl
index 1bf0987..71656b3 100755
--- a/infiniband-diags/scripts/ibfindnodesusing.pl
+++ b/infiniband-diags/scripts/ibfindnodesusing.pl
@@ -80,7 +80,7 @@ sub usage_and_exit
print " -R Recalculate ibnetdiscover information\n";
print " -C <ca_name> use selected Channel Adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibidsverify.pl b/infiniband-diags/scripts/ibidsverify.pl
index de78e6b..1a236c8 100755
--- a/infiniband-diags/scripts/ibidsverify.pl
+++ b/infiniband-diags/scripts/ibidsverify.pl
@@ -46,7 +46,7 @@ sub usage_and_exit
print " -h This help message\n";
print
" -R Recalculate ibnetdiscover information (Default is to reuse ibnetdiscover output)\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/iblinkinfo.pl b/infiniband-diags/scripts/iblinkinfo.pl
index a195474..a7a3df5 100755
--- a/infiniband-diags/scripts/iblinkinfo.pl
+++ b/infiniband-diags/scripts/iblinkinfo.pl
@@ -62,7 +62,7 @@ sub usage_and_exit
print " -C <ca_name> use selected Channel Adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
print " -g print port guids instead of node guids\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibprintca.pl b/infiniband-diags/scripts/ibprintca.pl
index 38b4330..0baea0b 100755
--- a/infiniband-diags/scripts/ibprintca.pl
+++ b/infiniband-diags/scripts/ibprintca.pl
@@ -51,7 +51,7 @@ sub usage_and_exit
print " -l list cas\n";
print " -C <ca_name> use selected channel adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibprintrt.pl b/infiniband-diags/scripts/ibprintrt.pl
index 86dcb64..0b3db19 100755
--- a/infiniband-diags/scripts/ibprintrt.pl
+++ b/infiniband-diags/scripts/ibprintrt.pl
@@ -51,7 +51,7 @@ sub usage_and_exit
print " -l list rts\n";
print " -C <ca_name> use selected channel adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibprintswitch.pl b/infiniband-diags/scripts/ibprintswitch.pl
index 6712201..c7377a9 100755
--- a/infiniband-diags/scripts/ibprintswitch.pl
+++ b/infiniband-diags/scripts/ibprintswitch.pl
@@ -50,7 +50,7 @@ sub usage_and_exit
print " -l list switches\n";
print " -C <ca_name> use selected channel adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibqueryerrors.pl b/infiniband-diags/scripts/ibqueryerrors.pl
index c807c02..5f2e167 100755
--- a/infiniband-diags/scripts/ibqueryerrors.pl
+++ b/infiniband-diags/scripts/ibqueryerrors.pl
@@ -149,7 +149,7 @@ sub usage_and_exit
print " -d include the data counters in the output\n";
print " -C <ca_name> use selected Channel Adaptor name for queries\n";
print " -P <ca_port> use selected channel adaptor port for queries\n";
- exit 0;
+ exit 2;
}
my $argv0 = `basename $0`;
diff --git a/infiniband-diags/scripts/ibswportwatch.pl b/infiniband-diags/scripts/ibswportwatch.pl
index 6d6ba1c..d888f51 100755
--- a/infiniband-diags/scripts/ibswportwatch.pl
+++ b/infiniband-diags/scripts/ibswportwatch.pl
@@ -81,7 +81,7 @@ sub usage_and_exit
print " -n <cycles> run n cycles then exit (default -1 == forever)\n";
print " -G Address provided is a GUID\n";
print " -b report bytes/second packets/second\n";
- exit 0;
+ exit 2;
}
# =========================================================================
--
1.5.4.rc2.60.gb2e62
>From a3d4a44d668912526466f591931a099a0978f943 Mon Sep 17 00:00:00 2001
From: root <root at castor.voltaire.com>
Date: Sun, 25 May 2008 15:54:00 +0300
Subject: [PATCH] infiniband-diags/scripts/*.pl: prevent some zero exists on errors
Upon failures break execution and drop error status.
Signed-off-by: root <root at castor.voltaire.com>
---
infiniband-diags/scripts/IBswcountlimits.pm | 19 +++++++++++--------
infiniband-diags/scripts/ibprintca.pl | 6 +++---
infiniband-diags/scripts/ibprintrt.pl | 6 +++---
infiniband-diags/scripts/ibprintswitch.pl | 4 ++--
infiniband-diags/scripts/ibqueryerrors.pl | 6 ++++--
infiniband-diags/scripts/ibswportwatch.pl | 6 ++----
6 files changed, 25 insertions(+), 22 deletions(-)
diff --git a/infiniband-diags/scripts/IBswcountlimits.pm b/infiniband-diags/scripts/IBswcountlimits.pm
index 9bc356f..9794ff1 100755
--- a/infiniband-diags/scripts/IBswcountlimits.pm
+++ b/infiniband-diags/scripts/IBswcountlimits.pm
@@ -219,8 +219,9 @@ sub any_counts
#
sub ensure_cache_dir
{
- if (!(-d "$IBswcountlimits::cache_dir")) {
- mkdir $IBswcountlimits::cache_dir, 0700;
+ if (!(-d "$IBswcountlimits::cache_dir") &&
+ !mkdir($IBswcountlimits::cache_dir, 0700)) {
+ die "cannot create $IBswcountlimits::cache_dir: $!\n";
}
}
@@ -260,9 +261,8 @@ sub generate_ibnetdiscover_topology
my $cache_file = get_cache_file($ca_name, $ca_port);
my $extra_params = get_ca_name_port_param_string($ca_name, $ca_port);
- `ibnetdiscover -g $extra_params > $cache_file`;
- if ($? != 0) {
- die "Execution of ibnetdiscover failed with errors\n";
+ if (`ibnetdiscover -g $extra_params > $cache_file`) {
+ die "Execution of ibnetdiscover failed: $!\n";
}
}
@@ -421,7 +421,8 @@ sub get_num_ports
my $num_ports = 0;
my $extra_params = get_ca_name_port_param_string($ca_name, $ca_port);
- my $data = `smpquery $extra_params -G nodeinfo $guid`;
+ my $data = `smpquery $extra_params -G nodeinfo $guid` ||
+ die "'smpquery $extra_params -G nodeinfo $guid' faild\n";
my @lines = split("\n", $data);
my $pkt_lifetime = "";
foreach my $line (@lines) {
@@ -457,7 +458,8 @@ sub convert_dr_to_guid
{
my $guid = undef;
- my $data = `smpquery nodeinfo -D $_[0]`;
+ my $data = `smpquery nodeinfo -D $_[0]` ||
+ die "'mpquery nodeinfo -D $_[0]' failed\n";
my @lines = split("\n", $data);
foreach my $line (@lines) {
if ($line =~ /^PortGuid:\.+(.*)/) { $guid = $1; }
@@ -480,7 +482,8 @@ sub get_node_type
$query_arg .= "-D " . $_[0];
}
- my $data = `$query_arg`;
+ my $data = `$query_arg` ||
+ die "'$query_arg' failed\n";
my @lines = split("\n", $data);
foreach my $line (@lines) {
if ($line =~ /^NodeType:\.+(.*)/) { $type = $1; }
diff --git a/infiniband-diags/scripts/ibprintca.pl b/infiniband-diags/scripts/ibprintca.pl
index 0baea0b..7de0801 100755
--- a/infiniband-diags/scripts/ibprintca.pl
+++ b/infiniband-diags/scripts/ibprintca.pl
@@ -118,9 +118,9 @@ sub main
print $ports{$port};
}
if (!$found_hca) {
- print "\"$target_hca\" not found\n";
- print " Try running with the \"-R\" option.\n";
- print " If still not found the node is probably down.\n";
+ die "\"$target_hca\" not found\n" .
+ " Try running with the \"-R\" option.\n" .
+ " If still not found the node is probably down.\n";
}
close IBNET_TOPO;
}
diff --git a/infiniband-diags/scripts/ibprintrt.pl b/infiniband-diags/scripts/ibprintrt.pl
index 0b3db19..43323ca 100755
--- a/infiniband-diags/scripts/ibprintrt.pl
+++ b/infiniband-diags/scripts/ibprintrt.pl
@@ -118,9 +118,9 @@ sub main
print $ports{$port};
}
if (!$found_rt) {
- print "\"$target_rt\" not found\n";
- print " Try running with the \"-R\" option.\n";
- print " If still not found the node is probably down.\n";
+ die "\"$target_rt\" not found\n" .
+ " Try running with the \"-R\" option.\n" .
+ " If still not found the node is probably down.\n";
}
close IBNET_TOPO;
}
diff --git a/infiniband-diags/scripts/ibprintswitch.pl b/infiniband-diags/scripts/ibprintswitch.pl
index c7377a9..8af3f48 100755
--- a/infiniband-diags/scripts/ibprintswitch.pl
+++ b/infiniband-diags/scripts/ibprintswitch.pl
@@ -117,8 +117,8 @@ sub main
print $ports{$port};
}
if (!$found_switch) {
- print "Switch \"$target_switch\" not found\n";
- print " Try running with the \"-R\" option.\n";
+ die "Switch \"$target_switch\" not found\n" .
+ " Try running with the \"-R\" option.\n";
}
close IBNET_TOPO;
}
diff --git a/infiniband-diags/scripts/ibqueryerrors.pl b/infiniband-diags/scripts/ibqueryerrors.pl
index 5f2e167..a6128b5 100755
--- a/infiniband-diags/scripts/ibqueryerrors.pl
+++ b/infiniband-diags/scripts/ibqueryerrors.pl
@@ -104,7 +104,8 @@ sub get_counts
my $ca_port = $_[3];
my $extra_params = get_ca_name_port_param_string($ca_name, $ca_port);
- my $data = `perfquery $extra_params -G $addr $port`;
+ my $data = `perfquery $extra_params -G $addr $port` ||
+ die "'perfquery $extra_params -G $addr $port' FAILED.\n";
my @lines = split("\n", $data);
foreach my $line (@lines) {
foreach my $count (@IBswcountlimits::counters) {
@@ -121,7 +122,8 @@ my %switches = ();
sub get_switches
{
- my $data = `ibswitches $cache_file`;
+ my $data = `ibswitches $cache_file` ||
+ die "'ibswitches $cache_file' failed.\n";
my @lines = split("\n", $data);
foreach my $line (@lines) {
if ($line =~ /^Switch\s+:\s+(\w+)\s+ports\s+(\d+)\s+.*/) {
diff --git a/infiniband-diags/scripts/ibswportwatch.pl b/infiniband-diags/scripts/ibswportwatch.pl
index d888f51..92066d1 100755
--- a/infiniband-diags/scripts/ibswportwatch.pl
+++ b/infiniband-diags/scripts/ibswportwatch.pl
@@ -121,12 +121,10 @@ sub get_new_counts
)
)
{
- print "perfquery failed : \"perfquery $GUID $addr $port\"\n";
- system("cat $IBswcountlimits::cache_dir/perfquery.out");
- exit 1;
+ die "perfquery failed : \"perfquery $GUID $addr $port\"\n";
}
open PERF_QUERY, "<$IBswcountlimits::cache_dir/perfquery.out"
- or die "perfquery failed";
+ or die "cannot read '$IBswcountlimits::cache_dir/perfquery.out': $!\n";
while (my $line = <PERF_QUERY>) {
foreach my $count (@IBswcountlimits::counters) {
if ($line =~ /^$count:\.+(\d+)/) {
--
1.5.4.rc2.60.gb2e62
More information about the general
mailing list