From 050ebf461d4f810aa41f30edeba7522ae00fb1b7 Mon Sep 17 00:00:00 2001 From: Mark Gurevich Date: Mon, 2 Jul 2018 15:39:43 -0400 Subject: [PATCH 1/3] Lock rflash operation --- perl-xCAT/xCAT/Utils.pm | 2 +- xCAT-server/lib/xcat/plugins/openbmc.pm | 55 ++++++++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/perl-xCAT/xCAT/Utils.pm b/perl-xCAT/xCAT/Utils.pm index 7b76bcb9c..4fbe638cf 100644 --- a/perl-xCAT/xCAT/Utils.pm +++ b/perl-xCAT/xCAT/Utils.pm @@ -2473,7 +2473,7 @@ sub acquire_lock { use Fcntl ":flock"; my $tlock; $tlock->{path} = "/var/lock/xcat/" . $lock_name; - sysopen($tlock->{fd}, $tlock->{path}, POSIX::O_CREAT | POSIX::O_WRONLY) or return undef; + sysopen($tlock->{fd}, $tlock->{path}, POSIX::O_CREAT | POSIX::O_EXCL | POSIX::O_WRONLY) or return undef; unless ($tlock->{fd}) { return undef; } if ($nonblock_mode) { flock($tlock->{fd}, LOCK_EX | LOCK_NB) or return undef; diff --git a/xCAT-server/lib/xcat/plugins/openbmc.pm b/xCAT-server/lib/xcat/plugins/openbmc.pm index bb2b09ce5..0fffa1955 100644 --- a/xCAT-server/lib/xcat/plugins/openbmc.pm +++ b/xCAT-server/lib/xcat/plugins/openbmc.pm @@ -916,6 +916,7 @@ sub process_request { } my $check = parse_node_info($noderange); + return if ($check); my $rst = parse_command_status($command, \@exargs); return if ($rst); @@ -1063,6 +1064,8 @@ rmdir \"/tmp/\$userid\" \n"; $node_info{$node}{rst} = "BMC is not ready" unless ($node_info{$node}{rst}); push @{ $rflash_result{fail} }, "$node: $node_info{$node}{rst}"; } + # End of activation processing, release the lock + xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); } xCAT::MsgUtils->message("I", { data => ["-------------------------------------------------------"], host => [1] }, $callback); my $summary = "Firmware update complete: "; @@ -1424,6 +1427,29 @@ sub parse_args { } } } + # All options parsed and validated. Now lock upload and activate processing, so that only one + # can continue in case multiples are issued for the same node + # + if ($option_flag =~ /^-d$|^-u$|^--upload$|^-a$|^--activate$/) { + foreach my $node (@$noderange) { + # Check if node is alrady locked + my $locked = xCAT::Utils->is_locked("rflash_$node", 1); + if ($locked) { + return ([ 1, "Unable to rflash $node. Another process is aleady flashing this node." ]); + } else { + # Lock each targeted node + if ($verbose) { + xCAT::SvrUtils::sendmsg("Attempting to lock $node for rflash", $callback); + } + my $lock = xCAT::Utils->acquire_lock("rflash_$node", 1); + unless ($lock) { + return ([ 1, "Unable to lock $node for rflash command" ]); + } + # Save lock handle in node_info hash so it can be released on completion + $node_info{$node}{rflash_lock} = $lock; + } + } + } } else { return ([ 1, "Command is not supported." ]); } @@ -2239,10 +2265,18 @@ sub parse_node_info { unless($node_info{$node}{bmc}) { xCAT::SvrUtils::sendmsg("Error: Unable to get attribute bmc", $callback, $node); $rst = 1; + if ($node_info{$node}{rflash_lock}) { + # If we are holding a lock on the node, release the lock + xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); + } next; } unless($node_info{$node}{bmcip}) { xCAT::SvrUtils::sendmsg("Error: Unable to resolved ip address for bmc: $node_info{$node}{bmc}", $callback, $node); + if ($node_info{$node}{rflash_lock}) { + # If we are holding a lock on the node, release the lock + xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); + } delete $node_info{$node}; $rst = 1; next; @@ -2253,6 +2287,10 @@ sub parse_node_info { $node_info{$node}{username} = $passwd_hash->{username}; } else { xCAT::SvrUtils::sendmsg("Error: Unable to get attribute username", $callback, $node); + if ($node_info{$node}{rflash_lock}) { + # If we are holding a lock on the node, release the lock + xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); + } delete $node_info{$node}; $rst = 1; next; @@ -2264,6 +2302,10 @@ sub parse_node_info { $node_info{$node}{password} = $passwd_hash->{password}; } else { xCAT::SvrUtils::sendmsg("Error: Unable to get attribute password", $callback, $node); + if ($node_info{$node}{rflash_lock}) { + # If we are holding a lock on the node, release the lock + xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); + } delete $node_info{$node}; $rst = 1; next; @@ -2276,6 +2318,10 @@ sub parse_node_info { } else { xCAT::SvrUtils::sendmsg("Error: Unable to get information from openbmc table", $callback, $node); $rst = 1; + if ($node_info{$node}{rflash_lock}) { + # If we are holding a lock on the node, release the lock + xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); + } next; } } @@ -4398,7 +4444,14 @@ sub rflash_response { sleep(1) } elsif ($child == 0) { $async->remove_all; - exit(rflash_upload($node, $callback)) + my $RC=rflash_upload($node, $callback); + if ((!$::UPLOAD_AND_ACTIVATE) && (!$::UPLOAD_ACTIVATE_STREAM)) { + # Regular upload is done - release lock + # Lock is kept for "upload and activate" and "upload stream" processing to be + # released at the end of those steps + xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); + } + exit($RC) } else { $child_node_map{$child} = $node; } From 32f4b36dae66e009b5ec370cb02187e81bbb6f2d Mon Sep 17 00:00:00 2001 From: Mark Gurevich Date: Thu, 12 Jul 2018 15:06:03 -0400 Subject: [PATCH 2/3] Simplified locking implementation --- perl-xCAT/xCAT/Utils.pm | 2 +- xCAT-server/lib/xcat/plugins/openbmc.pm | 105 +++++++++++------------- 2 files changed, 47 insertions(+), 60 deletions(-) diff --git a/perl-xCAT/xCAT/Utils.pm b/perl-xCAT/xCAT/Utils.pm index 4fbe638cf..7b76bcb9c 100644 --- a/perl-xCAT/xCAT/Utils.pm +++ b/perl-xCAT/xCAT/Utils.pm @@ -2473,7 +2473,7 @@ sub acquire_lock { use Fcntl ":flock"; my $tlock; $tlock->{path} = "/var/lock/xcat/" . $lock_name; - sysopen($tlock->{fd}, $tlock->{path}, POSIX::O_CREAT | POSIX::O_EXCL | POSIX::O_WRONLY) or return undef; + sysopen($tlock->{fd}, $tlock->{path}, POSIX::O_CREAT | POSIX::O_WRONLY) or return undef; unless ($tlock->{fd}) { return undef; } if ($nonblock_mode) { flock($tlock->{fd}, LOCK_EX | LOCK_NB) or return undef; diff --git a/xCAT-server/lib/xcat/plugins/openbmc.pm b/xCAT-server/lib/xcat/plugins/openbmc.pm index 0fffa1955..59faf369d 100644 --- a/xCAT-server/lib/xcat/plugins/openbmc.pm +++ b/xCAT-server/lib/xcat/plugins/openbmc.pm @@ -916,7 +916,6 @@ sub process_request { } my $check = parse_node_info($noderange); - return if ($check); my $rst = parse_command_status($command, \@exargs); return if ($rst); @@ -1003,6 +1002,25 @@ sub process_request { } } + # All options parsed and validated. Now lock upload and activate processing, so that only one + # can continue in case multiples are issued for the same node + # + if (($next_status{LOGIN_RESPONSE} eq "RFLASH_FILE_UPLOAD_REQUEST") or + ($next_status{LOGIN_RESPONSE} eq "RFLASH_UPDATE_ACTIVATE_REQUEST") or + ($next_status{LOGIN_RESPONSE} eq "RFLASH_UPDATE_HOST_ACTIVATE_REQUEST")) { + + my $lock = xCAT::Utils->acquire_lock("rflash_$node", 1); + unless ($lock) { + xCAT::SvrUtils::sendmsg([ 1, "Unable to rflash $node. Another process is aleady flashing this node." ], $callback, $node); + $wait_node_num--; + next; + } + if ($::VERBOSE) { + xCAT::SvrUtils::sendmsg("Acquired the lock for upload and activate process", $callback, $node); + } + $node_info{$node}{rflash_lock} = $lock; + } + $login_url = "$http_protocol://$node_info{$node}{bmc}/login"; $content = '{ "data": [ "' . $node_info{$node}{username} .'", "' . $node_info{$node}{password} . '" ] }'; if ($xcatdebugmode) { @@ -1064,9 +1082,8 @@ rmdir \"/tmp/\$userid\" \n"; $node_info{$node}{rst} = "BMC is not ready" unless ($node_info{$node}{rst}); push @{ $rflash_result{fail} }, "$node: $node_info{$node}{rst}"; } - # End of activation processing, release the lock - xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); } +<<<<<<< HEAD xCAT::MsgUtils->message("I", { data => ["-------------------------------------------------------"], host => [1] }, $callback); my $summary = "Firmware update complete: "; my $total = keys %node_info; @@ -1080,9 +1097,26 @@ rmdir \"/tmp/\$userid\" \n"; if ($rflash_result{fail}) { foreach (@{ $rflash_result{fail} }) { xCAT::MsgUtils->message("I", { data => ["$_"], host => [1] }, $callback); +======= + my $total = keys %node_info; + # Display summary information but only if there were any nodes to process + if ($total > 0) { + xCAT::MsgUtils->message("I", { data => ["-------------------------------------------------------"], host => [1] }, $callback); + my $summary = "Firmware update complete: "; + my $success = 0; + my $fail = 0; + $success = @{ $rflash_result{success} } if (defined $rflash_result{success} and @{ $rflash_result{success} }); + $fail = @{ $rflash_result{fail} } if (defined $rflash_result{fail} and @{ $rflash_result{fail} }); + $summary .= "Total=$total Success=$success Failed=$fail"; + xCAT::MsgUtils->message("I", { data => ["$summary"], host => [1] }, $callback); + + if ($rflash_result{fail}) { + foreach (@{ $rflash_result{fail} }) { + xCAT::MsgUtils->message("I", { data => ["$_"], host => [1] }, $callback); + } } + xCAT::MsgUtils->message("I", { data => ["-------------------------------------------------------"], host => [1] }, $callback); } - xCAT::MsgUtils->message("I", { data => ["-------------------------------------------------------"], host => [1] }, $callback); } last; } @@ -1427,29 +1461,6 @@ sub parse_args { } } } - # All options parsed and validated. Now lock upload and activate processing, so that only one - # can continue in case multiples are issued for the same node - # - if ($option_flag =~ /^-d$|^-u$|^--upload$|^-a$|^--activate$/) { - foreach my $node (@$noderange) { - # Check if node is alrady locked - my $locked = xCAT::Utils->is_locked("rflash_$node", 1); - if ($locked) { - return ([ 1, "Unable to rflash $node. Another process is aleady flashing this node." ]); - } else { - # Lock each targeted node - if ($verbose) { - xCAT::SvrUtils::sendmsg("Attempting to lock $node for rflash", $callback); - } - my $lock = xCAT::Utils->acquire_lock("rflash_$node", 1); - unless ($lock) { - return ([ 1, "Unable to lock $node for rflash command" ]); - } - # Save lock handle in node_info hash so it can be released on completion - $node_info{$node}{rflash_lock} = $lock; - } - } - } } else { return ([ 1, "Command is not supported." ]); } @@ -2088,10 +2099,13 @@ sub parse_command_status { } } } - if ($upload or $::UPLOAD_AND_ACTIVATE) { - xCAT::SvrUtils::sendmsg("Attempting to upload $::UPLOAD_FILE, please wait...", $callback); - } elsif ($::UPLOAD_ACTIVATE_STREAM) { - xCAT::SvrUtils::sendmsg("Attempting to upload $::UPLOAD_FILE and $::UPLOAD_PNOR, please wait...", $callback); + # Check if there are any valid nodes to work on. If none, do not issue these messages + if (keys %node_info > 0) { + if ($upload or $::UPLOAD_AND_ACTIVATE) { + xCAT::SvrUtils::sendmsg("Attempting to upload $::UPLOAD_FILE, please wait...", $callback); + } elsif ($::UPLOAD_ACTIVATE_STREAM) { + xCAT::SvrUtils::sendmsg("Attempting to upload $::UPLOAD_FILE and $::UPLOAD_PNOR, please wait...", $callback); + } } if ($check_version) { # Display firmware version on BMC @@ -2265,18 +2279,10 @@ sub parse_node_info { unless($node_info{$node}{bmc}) { xCAT::SvrUtils::sendmsg("Error: Unable to get attribute bmc", $callback, $node); $rst = 1; - if ($node_info{$node}{rflash_lock}) { - # If we are holding a lock on the node, release the lock - xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); - } next; } unless($node_info{$node}{bmcip}) { xCAT::SvrUtils::sendmsg("Error: Unable to resolved ip address for bmc: $node_info{$node}{bmc}", $callback, $node); - if ($node_info{$node}{rflash_lock}) { - # If we are holding a lock on the node, release the lock - xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); - } delete $node_info{$node}; $rst = 1; next; @@ -2287,10 +2293,6 @@ sub parse_node_info { $node_info{$node}{username} = $passwd_hash->{username}; } else { xCAT::SvrUtils::sendmsg("Error: Unable to get attribute username", $callback, $node); - if ($node_info{$node}{rflash_lock}) { - # If we are holding a lock on the node, release the lock - xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); - } delete $node_info{$node}; $rst = 1; next; @@ -2302,10 +2304,6 @@ sub parse_node_info { $node_info{$node}{password} = $passwd_hash->{password}; } else { xCAT::SvrUtils::sendmsg("Error: Unable to get attribute password", $callback, $node); - if ($node_info{$node}{rflash_lock}) { - # If we are holding a lock on the node, release the lock - xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); - } delete $node_info{$node}; $rst = 1; next; @@ -2318,10 +2316,6 @@ sub parse_node_info { } else { xCAT::SvrUtils::sendmsg("Error: Unable to get information from openbmc table", $callback, $node); $rst = 1; - if ($node_info{$node}{rflash_lock}) { - # If we are holding a lock on the node, release the lock - xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); - } next; } } @@ -4444,14 +4438,7 @@ sub rflash_response { sleep(1) } elsif ($child == 0) { $async->remove_all; - my $RC=rflash_upload($node, $callback); - if ((!$::UPLOAD_AND_ACTIVATE) && (!$::UPLOAD_ACTIVATE_STREAM)) { - # Regular upload is done - release lock - # Lock is kept for "upload and activate" and "upload stream" processing to be - # released at the end of those steps - xCAT::Utils->release_lock($node_info{$node}{rflash_lock}, 1); - } - exit($RC) + exit(rflash_upload($node, $callback)); } else { $child_node_map{$child} = $node; } From fb7ad8e1b52d421a68005f253451a64c54f5692a Mon Sep 17 00:00:00 2001 From: Mark Gurevich Date: Mon, 16 Jul 2018 11:23:09 -0400 Subject: [PATCH 3/3] Add summary message --- xCAT-server/lib/xcat/plugins/openbmc.pm | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/xCAT-server/lib/xcat/plugins/openbmc.pm b/xCAT-server/lib/xcat/plugins/openbmc.pm index 59faf369d..b6052f7bc 100644 --- a/xCAT-server/lib/xcat/plugins/openbmc.pm +++ b/xCAT-server/lib/xcat/plugins/openbmc.pm @@ -1011,7 +1011,9 @@ sub process_request { my $lock = xCAT::Utils->acquire_lock("rflash_$node", 1); unless ($lock) { - xCAT::SvrUtils::sendmsg([ 1, "Unable to rflash $node. Another process is aleady flashing this node." ], $callback, $node); + my $lock_msg = "Unable to rflash $node. Another process is already flashing this node."; + xCAT::SvrUtils::sendmsg([ 1, $lock_msg ], $callback, $node); + $node_info{$node}{rst} = $lock_msg; $wait_node_num--; next; } @@ -1083,21 +1085,6 @@ rmdir \"/tmp/\$userid\" \n"; push @{ $rflash_result{fail} }, "$node: $node_info{$node}{rst}"; } } -<<<<<<< HEAD - xCAT::MsgUtils->message("I", { data => ["-------------------------------------------------------"], host => [1] }, $callback); - my $summary = "Firmware update complete: "; - my $total = keys %node_info; - my $success = 0; - my $fail = 0; - $success = @{ $rflash_result{success} } if (defined $rflash_result{success} and @{ $rflash_result{success} }); - $fail = @{ $rflash_result{fail} } if (defined $rflash_result{fail} and @{ $rflash_result{fail} }); - $summary .= "Total=$total Success=$success Failed=$fail"; - xCAT::MsgUtils->message("I", { data => ["$summary"], host => [1] }, $callback); - - if ($rflash_result{fail}) { - foreach (@{ $rflash_result{fail} }) { - xCAT::MsgUtils->message("I", { data => ["$_"], host => [1] }, $callback); -======= my $total = keys %node_info; # Display summary information but only if there were any nodes to process if ($total > 0) {