* [Cluster-devel] [PATCH] fence_scsi: fix simultaneous unfence operations
@ 2011-09-15 19:20 Ryan O'Hara
2011-09-19 15:15 ` Fabio M. Di Nitto
0 siblings, 1 reply; 2+ messages in thread
From: Ryan O'Hara @ 2011-09-15 19:20 UTC (permalink / raw)
To: cluster-devel.redhat.com
This patch fixes an issue where multiple nodes attempt to unfence at
the same time and fail when attempting to create a reservation. Each
node checks to see if a reservation exists, and it not, it will attempt
to create one. The problem occurs when multiple nodes see that no
reservation exists and attempt to create one.
This patch checs the return code of do_reserve. If this call fails,
check again to see if a reservation was created by another node, in
which case there is no error.
Resolves: rhbz#738384
Signed-off-by: Ryan O'Hara <rohara@redhat.com>
---
fence/agents/scsi/fence_scsi.pl | 87 ++++++++++++++++++++-------------------
1 files changed, 44 insertions(+), 43 deletions(-)
diff --git a/fence/agents/scsi/fence_scsi.pl b/fence/agents/scsi/fence_scsi.pl
index 2751bed..93f5056 100644
--- a/fence/agents/scsi/fence_scsi.pl
+++ b/fence/agents/scsi/fence_scsi.pl
@@ -48,10 +48,16 @@ sub do_action_on ($@)
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 (do_register_ignore ($node_key, $dev) != 0) {
+ log_error ("failed to create registration (key=$node_key, device=$dev)");
+ }
if (!get_reservation_key ($dev)) {
- do_reserve ($node_key, $dev);
+ if (do_reserve ($node_key, $dev) != 0) {
+ if (!get_reservation_key ($dev)) {
+ log_error ("failed to create reservation (key=$node_key, device=$dev)");
+ }
+ }
}
}
@@ -106,8 +112,7 @@ sub do_action_status ($@)
if ($dev_count != 0) {
exit (0);
- }
- else {
+ } else {
exit (2);
}
}
@@ -199,13 +204,13 @@ sub do_register ($$$)
$out = qx { $cmd 2> /dev/null };
$err = ($?>>8);
- if ($err != 0) {
- log_error ("$self (err=$err)");
- }
+ # if ($err != 0) {
+ # log_error ("$self (err=$err)");
+ # }
- # die "[error]: $self\n" if ($?>>8);
+ log_debug ("$self (err=$err)");
- return;
+ return ($err);
}
sub do_register_ignore ($$)
@@ -236,13 +241,13 @@ sub do_register_ignore ($$)
$out = qx { $cmd 2> /dev/null };
$err = ($?>>8);
- if ($err != 0) {
- log_error ("$self (err=$err)");
- }
+ # if ($err != 0) {
+ # log_error ("$self (err=$err)");
+ # }
- # die "[error]: $self ($dev)\n" if ($?>>8);
+ log_debug ("$self (err=$err)");
- return;
+ return ($err);
}
sub do_reserve ($$)
@@ -256,13 +261,13 @@ sub do_reserve ($$)
my $out = qx { $cmd 2> /dev/null };
my $err = ($?>>8);
- if ($err != 0) {
- log_error ("$self (err=$err)");
- }
+ # if ($err != 0) {
+ # log_error ("$self (err=$err)");
+ # }
- # die "[error]: $self\n" if ($?>>8);
+ log_debug ("$self (err=$err)");
- return;
+ return ($err);
}
sub do_release ($$)
@@ -276,13 +281,13 @@ sub do_release ($$)
my $out = qx { $cmd 2> /dev/null };
my $err = ($?>>8);
- if ($err != 0) {
- log_error ("$self (err=$err)");
- }
+ # if ($err != 0) {
+ # log_error ("$self (err=$err)");
+ # }
- # die "[error]: $self\n" if ($?>>8);
+ log_debug ("$self (err=$err)");
- return;
+ return ($err);
}
sub do_preempt ($$$)
@@ -296,13 +301,13 @@ sub do_preempt ($$$)
my $out = qx { $cmd 2> /dev/null };
my $err = ($?>>8);
- if ($err != 0) {
- log_error ("$self (err=$err)");
- }
+ # if ($err != 0) {
+ # log_error ("$self (err=$err)");
+ # }
- # die "[error]: $self\n" if ($?>>8);
+ log_debug ("$self (err=$err)");
- return;
+ return ($err);
}
sub do_preempt_abort ($$$)
@@ -316,13 +321,13 @@ sub do_preempt_abort ($$$)
my $out = qx { $cmd 2> /dev/null };
my $err = ($?>>8);
- if ($err != 0) {
- log_error ("$self (err=$err)");
- }
+ # if ($err != 0) {
+ # log_error ("$self (err=$err)");
+ # }
- # die "[error]: $self\n" if ($?>>8);
+ log_debug ("$self (err=$err)");
- return;
+ return ($err);
}
sub do_reset (S)
@@ -339,7 +344,7 @@ sub do_reset (S)
log_debug ("$self (dev=$dev, status=$err)");
- return;
+ return ($err);
}
sub dev_unlink ()
@@ -756,8 +761,7 @@ if (@ARGV > 0) {
if ($opt_o =~ /^metadata$/i) {
print_metadata;
}
-}
-else {
+} else {
get_options_stdin ();
}
@@ -780,8 +784,7 @@ if ((!defined $opt_n) && (!defined $opt_k)) {
##
if (defined $opt_k) {
$key = $opt_k;
-}
-else {
+} else {
$key = get_key ($opt_n);
}
@@ -801,8 +804,7 @@ if ($key =~ /^0/) {
##
if (defined $opt_d) {
@devices = split (/\s*,\s*/, $opt_d);
-}
-else {
+} else {
@devices = get_devices_clvm ();
}
@@ -830,8 +832,7 @@ elsif ($opt_o =~ /^off$/i) {
}
elsif ($opt_o =~ /^status/i) {
do_action_status ($key, @devices);
-}
-else {
+} else {
log_error ("unknown action '$opt_o'");
exit (1);
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [Cluster-devel] [PATCH] fence_scsi: fix simultaneous unfence operations
2011-09-15 19:20 [Cluster-devel] [PATCH] fence_scsi: fix simultaneous unfence operations Ryan O'Hara
@ 2011-09-19 15:15 ` Fabio M. Di Nitto
0 siblings, 0 replies; 2+ messages in thread
From: Fabio M. Di Nitto @ 2011-09-19 15:15 UTC (permalink / raw)
To: cluster-devel.redhat.com
ACK
Fabio
On 9/15/2011 9:20 PM, Ryan O'Hara wrote:
> This patch fixes an issue where multiple nodes attempt to unfence at
> the same time and fail when attempting to create a reservation. Each
> node checks to see if a reservation exists, and it not, it will attempt
> to create one. The problem occurs when multiple nodes see that no
> reservation exists and attempt to create one.
>
> This patch checs the return code of do_reserve. If this call fails,
> check again to see if a reservation was created by another node, in
> which case there is no error.
>
> Resolves: rhbz#738384
>
> Signed-off-by: Ryan O'Hara <rohara@redhat.com>
> ---
> fence/agents/scsi/fence_scsi.pl | 87 ++++++++++++++++++++-------------------
> 1 files changed, 44 insertions(+), 43 deletions(-)
>
> diff --git a/fence/agents/scsi/fence_scsi.pl b/fence/agents/scsi/fence_scsi.pl
> index 2751bed..93f5056 100644
> --- a/fence/agents/scsi/fence_scsi.pl
> +++ b/fence/agents/scsi/fence_scsi.pl
> @@ -48,10 +48,16 @@ sub do_action_on ($@)
> 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 (do_register_ignore ($node_key, $dev) != 0) {
> + log_error ("failed to create registration (key=$node_key, device=$dev)");
> + }
>
> if (!get_reservation_key ($dev)) {
> - do_reserve ($node_key, $dev);
> + if (do_reserve ($node_key, $dev) != 0) {
> + if (!get_reservation_key ($dev)) {
> + log_error ("failed to create reservation (key=$node_key, device=$dev)");
> + }
> + }
> }
> }
>
> @@ -106,8 +112,7 @@ sub do_action_status ($@)
>
> if ($dev_count != 0) {
> exit (0);
> - }
> - else {
> + } else {
> exit (2);
> }
> }
> @@ -199,13 +204,13 @@ sub do_register ($$$)
> $out = qx { $cmd 2> /dev/null };
> $err = ($?>>8);
>
> - if ($err != 0) {
> - log_error ("$self (err=$err)");
> - }
> + # if ($err != 0) {
> + # log_error ("$self (err=$err)");
> + # }
>
> - # die "[error]: $self\n" if ($?>>8);
> + log_debug ("$self (err=$err)");
>
> - return;
> + return ($err);
> }
>
> sub do_register_ignore ($$)
> @@ -236,13 +241,13 @@ sub do_register_ignore ($$)
> $out = qx { $cmd 2> /dev/null };
> $err = ($?>>8);
>
> - if ($err != 0) {
> - log_error ("$self (err=$err)");
> - }
> + # if ($err != 0) {
> + # log_error ("$self (err=$err)");
> + # }
>
> - # die "[error]: $self ($dev)\n" if ($?>>8);
> + log_debug ("$self (err=$err)");
>
> - return;
> + return ($err);
> }
>
> sub do_reserve ($$)
> @@ -256,13 +261,13 @@ sub do_reserve ($$)
> my $out = qx { $cmd 2> /dev/null };
> my $err = ($?>>8);
>
> - if ($err != 0) {
> - log_error ("$self (err=$err)");
> - }
> + # if ($err != 0) {
> + # log_error ("$self (err=$err)");
> + # }
>
> - # die "[error]: $self\n" if ($?>>8);
> + log_debug ("$self (err=$err)");
>
> - return;
> + return ($err);
> }
>
> sub do_release ($$)
> @@ -276,13 +281,13 @@ sub do_release ($$)
> my $out = qx { $cmd 2> /dev/null };
> my $err = ($?>>8);
>
> - if ($err != 0) {
> - log_error ("$self (err=$err)");
> - }
> + # if ($err != 0) {
> + # log_error ("$self (err=$err)");
> + # }
>
> - # die "[error]: $self\n" if ($?>>8);
> + log_debug ("$self (err=$err)");
>
> - return;
> + return ($err);
> }
>
> sub do_preempt ($$$)
> @@ -296,13 +301,13 @@ sub do_preempt ($$$)
> my $out = qx { $cmd 2> /dev/null };
> my $err = ($?>>8);
>
> - if ($err != 0) {
> - log_error ("$self (err=$err)");
> - }
> + # if ($err != 0) {
> + # log_error ("$self (err=$err)");
> + # }
>
> - # die "[error]: $self\n" if ($?>>8);
> + log_debug ("$self (err=$err)");
>
> - return;
> + return ($err);
> }
>
> sub do_preempt_abort ($$$)
> @@ -316,13 +321,13 @@ sub do_preempt_abort ($$$)
> my $out = qx { $cmd 2> /dev/null };
> my $err = ($?>>8);
>
> - if ($err != 0) {
> - log_error ("$self (err=$err)");
> - }
> + # if ($err != 0) {
> + # log_error ("$self (err=$err)");
> + # }
>
> - # die "[error]: $self\n" if ($?>>8);
> + log_debug ("$self (err=$err)");
>
> - return;
> + return ($err);
> }
>
> sub do_reset (S)
> @@ -339,7 +344,7 @@ sub do_reset (S)
>
> log_debug ("$self (dev=$dev, status=$err)");
>
> - return;
> + return ($err);
> }
>
> sub dev_unlink ()
> @@ -756,8 +761,7 @@ if (@ARGV > 0) {
> if ($opt_o =~ /^metadata$/i) {
> print_metadata;
> }
> -}
> -else {
> +} else {
> get_options_stdin ();
> }
>
> @@ -780,8 +784,7 @@ if ((!defined $opt_n) && (!defined $opt_k)) {
> ##
> if (defined $opt_k) {
> $key = $opt_k;
> -}
> -else {
> +} else {
> $key = get_key ($opt_n);
> }
>
> @@ -801,8 +804,7 @@ if ($key =~ /^0/) {
> ##
> if (defined $opt_d) {
> @devices = split (/\s*,\s*/, $opt_d);
> -}
> -else {
> +} else {
> @devices = get_devices_clvm ();
> }
>
> @@ -830,8 +832,7 @@ elsif ($opt_o =~ /^off$/i) {
> }
> elsif ($opt_o =~ /^status/i) {
> do_action_status ($key, @devices);
> -}
> -else {
> +} else {
> log_error ("unknown action '$opt_o'");
> exit (1);
> }
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-09-19 15:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-15 19:20 [Cluster-devel] [PATCH] fence_scsi: fix simultaneous unfence operations Ryan O'Hara
2011-09-19 15:15 ` Fabio M. Di Nitto
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).