cluster: STABLE3 - Add error messages to fence_scsi as well as a few comments.

rohara rohara at fedoraproject.org
Fri Dec 11 18:22:16 UTC 2009


Gitweb:        http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=commitdiff;h=cd97e03f5753bd5af87e809031a54bda67d8745d
Commit:        cd97e03f5753bd5af87e809031a54bda67d8745d
Parent:        2e5967a3b52a53eedb36887604fb063c0e9fec01
Author:        Ryan O'Hara <rohara at redhat.com>
AuthorDate:    Fri Dec 11 12:20:11 2009 -0600
Committer:     Ryan O'Hara <rohara at redhat.com>
CommitterDate: Fri Dec 11 12:20:11 2009 -0600

Add error messages to fence_scsi as well as a few comments.

    - Include current time in error/debug message.
    - Print usage when incorrect command-line options used.
    - Check for empty device list.
    - Check for key value of zero.
    - Verify devices exist and are block device.
    - Add comments and code cleanup.
---
 fence/agents/scsi/fence_scsi.pl |  189 +++++++++++++++++++++++----------------
 1 files changed, 111 insertions(+), 78 deletions(-)

diff --git a/fence/agents/scsi/fence_scsi.pl b/fence/agents/scsi/fence_scsi.pl
index e58a5b1..811e4d4 100644
--- a/fence/agents/scsi/fence_scsi.pl
+++ b/fence/agents/scsi/fence_scsi.pl
@@ -3,6 +3,7 @@
 use File::Basename;
 use Getopt::Std;
 use IPC::Open3;
+use POSIX;
 
 #BEGIN_VERSION_GENERATION
 $RELEASE_VERSION="";
@@ -16,69 +17,81 @@ my $ME = fileparse ($0, ".pl");
 
 sub log_debug ($)
 {
+    my $time = strftime "%b %e %T", localtime;
     my ($msg) = @_;
 
-    print STDOUT "[debug]: $msg\n" unless defined ($opt_q);
+    print STDOUT "$time $ME: [debug] $msg\n" unless defined ($opt_q);
+
+    return;
 }
 
 sub log_error ($)
 {
+    my $time = strftime "%b %e %T", localtime;
     my ($msg) = @_;
 
-    print STDERR "[error]: $msg\n" unless defined ($opt_q);
+    print STDERR "$time $ME: [error] $msg\n" unless defined ($opt_q);
+
+    exit (1);
 }
 
 sub do_action_on ($@)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($node_key, @devices) = @_;
 
     key_write ($node_key);
 
     foreach $dev (@devices) {
-	do_register_ignore ($node_key, $dev);
+	log_error ("device $dev does not exist") if (! -e $dev);
+	log_error ("device $dev is not a block device") if (! -b $dev);
 
+	do_register_ignore ($node_key, $dev);
 	if (!get_reservation_key ($dev)) {
 	    do_reserve ($node_key, $dev);
 	}
     }
+
+    return;
 }
 
 sub do_action_off ($@)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($node_key, @devices) = @_;
 
     my $host_key = key_read ();
 
     if ($host_key eq $node_key) {
 	log_error ($self);
-	exit (1);
     }
 
     foreach $dev (@devices) {
-	my @keys = grep { /$node_key/ } get_registration_keys ($dev);
+	log_error ("device $dev does not exist") if (! -e $dev);
+	log_error ("device $dev is not a block device") if (! -b $dev);
 
+	my @keys = grep { /$node_key/ } get_registration_keys ($dev);
 	if (scalar (@keys) != 0) {
 	    do_preempt_abort ($host_key, $node_key, $dev);
 	}
     }
+
+    return;
 }
 
 sub do_action_status ($@)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($node_key, @devices) = @_;
 
     my $dev_count = 0;
     my $key_count = 0;
 
     foreach $dev (@devices) {
-	my @keys = grep { /$node_key/ } get_registration_keys ($dev);
+	log_error ("device $dev does not exist") if (! -e $dev);
+	log_error ("device $dev is not a block device") if (! -b $dev);
 
+	my @keys = grep { /$node_key/ } get_registration_keys ($dev);
 	if (scalar (@keys) != 0) {
 	    $dev_count++;
 	}
@@ -94,13 +107,11 @@ sub do_action_status ($@)
 
 sub do_register ($$$)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($host_key, $node_key, $dev) = @_;
 
     if (substr ($dev, 5) =~ /^dm/) {
 	my @slaves = get_mpath_slaves ($dev);
-
 	foreach (@slaves) {
 	    do_register ($node_key, $_);
 	}
@@ -108,26 +119,29 @@ sub do_register ($$$)
     }
 
     my $cmd = "sg_persist -n -o -G -K $host_key -S $node_key -d $dev";
+
     $cmd .= " -Z" if (defined $opt_a);
 
     my $pid = open3 (\*IN, \*OUT, \*ERR, $cmd) or die "$!\n";
 
+    waitpid ($pid, 0);
+
     die "[error]: $self\n" if ($?>>8);
 
     close (IN);
     close (OUT);
     close (ERR);
+
+    return;
 }
 
 sub do_register_ignore ($$)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($node_key, $dev) = @_;
 
     if (substr ($dev, 5) =~ /^dm/) {
 	my @slaves = get_mpath_slaves ($dev);
-
 	foreach (@slaves) {
 	    do_register_ignore ($node_key, $_);
 	}
@@ -135,6 +149,7 @@ sub do_register_ignore ($$)
     }
 
     my $cmd = "sg_persist -n -o -I -S $node_key -d $dev";
+
     $cmd .= " -Z" if (defined $opt_a);
 
     my $pid = open3 (\*IN, \*OUT, \*ERR, $cmd) or die "$!\n";
@@ -146,12 +161,13 @@ sub do_register_ignore ($$)
     close (IN);
     close (OUT);
     close (ERR);
+
+    return;
 }
 
 sub do_reserve ($$)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($host_key, $dev) = @_;
 
     my $cmd = "sg_persist -n -o -R -T 5 -K $host_key -d $dev";
@@ -164,12 +180,13 @@ sub do_reserve ($$)
     close (IN);
     close (OUT);
     close (ERR);
+
+    return;
 }
 
 sub do_release ($$)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($host_key, $dev) = @_;
 
     my $cmd = "sg_persist -n -o -L -T 5 -K $host_key -d $dev";
@@ -182,12 +199,13 @@ sub do_release ($$)
     close (IN);
     close (OUT);
     close (ERR);
+
+    return;
 }
 
 sub do_preempt ($$$)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($host_key, $node_key, $dev) = @_;
 
     my $cmd = "sg_persist -n -o -P -T 5 -K $host_key -S $node_key -d $dev";
@@ -200,12 +218,13 @@ sub do_preempt ($$$)
     close (IN);
     close (OUT);
     close (ERR);
+
+    return;
 }
 
 sub do_preempt_abort ($$$)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($host_key, $node_key, $dev) = @_;
 
     my $cmd = "sg_persist -n -o -A -T 5 -K $host_key -S $node_key -d $dev";
@@ -218,16 +237,17 @@ sub do_preempt_abort ($$$)
     close (IN);
     close (OUT);
     close (ERR);
+
+    return;
 }
 
 sub key_read ()
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
+    my $self = (caller(0))[3];
+    my $key;
 
     open (\*FILE, "</var/lib/cluster/fence_scsi.key") or die "$!\n";
-
-    chomp (my $key = <FILE>);
-
+    chomp ($key = <FILE>);
     close (FILE);
 
     return ($key);
@@ -235,19 +255,18 @@ sub key_read ()
 
 sub key_write ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
+    my $self = (caller(0))[3];
 
     open (\*FILE, ">/var/lib/cluster/fence_scsi.key") or die "$!\n";
-
     print FILE "$_[0]\n";
-
     close (FILE);
+
+    return;
 }
 
 sub get_key ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my $key = sprintf ("%.4x%.4x", get_cluster_id (), get_node_id ($_[0]));
 
     return ($key);
@@ -255,7 +274,8 @@ sub get_key ($)
 
 sub get_node_id ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
+    my $self = (caller(0))[3];
+    my $node_id;
 
     my $cmd = "cman_tool nodes -n $_[0] -F id";
     my $pid = open3 (\*IN, \*OUT, \*ERR, $cmd) or die "$!\n";
@@ -264,7 +284,7 @@ sub get_node_id ($)
 
     die "[error]: $self\n" if ($?>>8);
 
-    chomp (my $node_id = <OUT>);
+    chomp ($node_id = <OUT>);
 
     close (IN);
     close (OUT);
@@ -275,7 +295,8 @@ sub get_node_id ($)
 
 sub get_cluster_id ()
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
+    my $self = (caller(0))[3];
+    my $cluster_id;
 
     my $cmd = "cman_tool status";
     my $pid = open3 (\*IN, \*OUT, \*ERR, $cmd) or die "$!\n";
@@ -284,8 +305,6 @@ sub get_cluster_id ()
 
     die "[error]: $self\n" if ($?>>8);
 
-    my $cluster_id;
-
     while (<OUT>) {
 	chomp;
 
@@ -305,7 +324,8 @@ sub get_cluster_id ()
 
 sub get_devices_clvm ()
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
+    my $self = (caller(0))[3];
+    my @devices;
 
     my $cmd = "vgs --noheadings " .
               "    --separator : " .
@@ -320,8 +340,6 @@ sub get_devices_clvm ()
 
     die "[error]: $self\n" if ($?>>8);
 
-    my @devices;
-
     while (<OUT>) {
 	chomp;
 
@@ -341,12 +359,11 @@ sub get_devices_clvm ()
 
 sub get_devices_scsi ()
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
+    my $self = (caller(0))[3];
+    my @devices;
 
     opendir (\*DIR, "/sys/block/") or die "$!\n";
-
-    my @devices = grep { /^sd/ } readdir (DIR);
-
+    @devices = grep { /^sd/ } readdir (DIR);
     closedir (DIR);
 
     return (@devices);
@@ -354,18 +371,16 @@ sub get_devices_scsi ()
 
 sub get_mpath_name ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($dev) = @_;
+    my $name;
 
     if ($dev =~ /^\/dev\//) {
 	$dev = substr ($dev, 5);
     }
 
     open (\*FILE, "/sys/block/$dev/dm/name") or die "$!\n";
-
-    chomp (my $name = <FILE>);
-
+    chomp ($name = <FILE>);
     close (FILE);
 
     return ($name);
@@ -373,18 +388,16 @@ sub get_mpath_name ($)
 
 sub get_mpath_uuid ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($dev) = @_;
+    my $uuid;
 
     if ($dev =~ /^\/dev\//) {
 	$dev = substr ($dev, 5);
     }
 
     open (\*FILE, "/sys/block/$dev/dm/uuid") or die "$!\n";
-
-    chomp (my $uuid = <FILE>);
-
+    chomp ($uuid = <FILE>);
     close (FILE);
 
     return ($name);
@@ -392,9 +405,9 @@ sub get_mpath_uuid ($)
 
 sub get_mpath_slaves ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($dev) = @_;
+    my @slaves;
 
     if ($dev =~ /^\/dev\//) {
 	$dev = substr ($dev, 5);
@@ -402,12 +415,11 @@ sub get_mpath_slaves ($)
 
     opendir (\*DIR, "/sys/block/$dev/slaves/") or die "$!\n";
 
-    my @slaves = grep { !/^\./ } readdir (DIR);
+    @slaves = grep { !/^\./ } readdir (DIR);
 
     if ($slaves[0] =~ /^dm/) {
 	@slaves = get_mpath_slaves ($slaves[0]);
-    }
-    else {
+    } else {
 	@slaves = map { "/dev/$_" } @slaves;
     }
 
@@ -418,9 +430,9 @@ sub get_mpath_slaves ($)
 
 sub get_registration_keys ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($dev) = @_;
+    my @keys;
 
     my $cmd = "sg_persist -n -i -k -d $dev";
     my $pid = open3 (\*IN, \*OUT, \*ERR, $cmd) or die "$!\n";
@@ -429,10 +441,8 @@ sub get_registration_keys ($)
 
     die "[error]: $self\n" if ($?>>8);
 
-    my @keys;
-
     while (<OUT>) {
-    	chomp;
+	chomp;
 
 	if ($_ =~ s/^\s+0x//i) {
 	    push (@keys, $_);
@@ -448,9 +458,9 @@ sub get_registration_keys ($)
 
 sub get_reservation_key ($)
 {
-    (my $self = (caller(0))[3]) =~ s/^main:://;
-
+    my $self = (caller(0))[3];
     my ($dev) = @_;
+    my $key;
 
     my $cmd = "sg_persist -n -i -r -d $dev";
     my $pid = open3 (\*IN, \*OUT, \*ERR, $cmd) or die "$!\n";
@@ -459,10 +469,8 @@ sub get_reservation_key ($)
 
     die "[error]: $self\n" if ($?>>8);
 
-    my $key;
-
     while (<OUT>) {
-    	chomp;
+	chomp;
 
 	if ($_ =~ s/^\s+key=0x//i) {
 	    $key = $_;
@@ -481,8 +489,7 @@ sub get_options_stdin ()
 {
     my $num = 0;
 
-    while (<STDIN>)
-    {
+    while (<STDIN>) {
 	chomp;
 
 	s/^\s*//;
@@ -551,7 +558,7 @@ sub print_version ()
 ################################################################################
 
 if (@ARGV > 0) {
-    getopts ("ad:f:hk:n:o:qV") or die "$!\n";
+    getopts ("ad:f:hk:n:o:qV") or print_usage;
 
     print_usage if (defined $opt_h);
     print_version if (defined $opt_V);
@@ -560,17 +567,23 @@ else {
     get_options_stdin ();
 }
 
+## if the logfile (-f) parameter was specified, open the logfile
+## and redirect STDOUT and STDERR to the logfile.
+##
 if (defined $opt_f) {
     open (LOG, ">$opt_f") or die "$!\n";
     open (STDOUT, ">&LOG");
     open (STDERR, ">&LOG");
 }
 
+## verify that either key or nodename have been specified
+##
 if ((!defined $opt_n) && (!defined $opt_k)) {
-    log_error ("No '-n' or '-k' flag specified.");
-    exit (1);
+    print_usage ();
 }
 
+## determine key value
+##
 if (defined $opt_k) {
     $key = $opt_k;
 }
@@ -578,6 +591,14 @@ else {
     $key = get_key ($opt_n);
 }
 
+## verify that key is not zero
+##
+if ($key == 0) {
+    log_error ("key cannot be zero");
+}
+
+## get devices
+##
 if (defined $opt_d) {
     @devices = split (/\s*,\s*/, $opt_d);
 }
@@ -585,10 +606,20 @@ else {
     @devices = get_devices_clvm ();
 }
 
+## verify that device list is not empty
+##
+if (scalar (@devices) == 0) {
+    log_error ("no devices found");
+}
+
+## default action is "off"
+##
 if (!defined $opt_o) {
     $opt_o = "off";
 }
 
+## determine the action to perform
+##
 if ($opt_o =~ /^on$/i) {
     do_action_on ($key, @devices);
 }
@@ -600,9 +631,11 @@ elsif ($opt_o =~ /^status/i) {
 }
 else {
     log_error ("unknown action '$opt_o'");
-    exit (1);
+exit (1);
 }
 
+## close the logfile
+##
 if (defined $opt_f) {
     close (LOG);
 }


More information about the cluster-commits mailing list