[pve-devel] [PATCH cluster 4/4] pvecm: lock corosync config on addition and deletion
Thomas Lamprecht
t.lamprecht at proxmox.com
Thu Sep 21 13:31:24 CEST 2017
This avoids potential races which would lead to an inconsistent
corosync config.
Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
(cherry picked from commit b778c013a2fc245008512365ac0b1a37287b30b3)
---
use `git show -w` to see what really changed.
Note: I squashed the "pass code ref correctly" fixes here too
data/PVE/CLI/pvecm.pm | 172 ++++++++++++++++++++++++++------------------------
1 file changed, 91 insertions(+), 81 deletions(-)
diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
index 05a68e5..36fe93b 100755
--- a/data/PVE/CLI/pvecm.pm
+++ b/data/PVE/CLI/pvecm.pm
@@ -308,87 +308,92 @@ __PACKAGE__->register_method ({
PVE::Cluster::check_cfs_quorum();
- my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
+ my $code = sub {
+ my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
- my $nodelist = PVE::Cluster::corosync_nodelist($conf);
+ my $nodelist = PVE::Cluster::corosync_nodelist($conf);
- my $totem_cfg = PVE::Cluster::corosync_totem_config($conf);
+ my $totem_cfg = PVE::Cluster::corosync_totem_config($conf);
- my $name = $param->{node};
+ my $name = $param->{node};
- # ensure we do not reuse an address, that can crash the whole cluster!
- my $check_duplicate_addr = sub {
- my $addr = shift;
- return if !defined($addr);
+ # ensure we do not reuse an address, that can crash the whole cluster!
+ my $check_duplicate_addr = sub {
+ my $addr = shift;
+ return if !defined($addr);
- while (my ($k, $v) = each %$nodelist) {
- next if $k eq $name; # allows re-adding a node if force is set
- if ($v->{ring0_addr} eq $addr || ($v->{ring1_addr} && $v->{ring1_addr} eq $addr)) {
- die "corosync: address '$addr' already defined by node '$k'\n";
+ while (my ($k, $v) = each %$nodelist) {
+ next if $k eq $name; # allows re-adding a node if force is set
+ if ($v->{ring0_addr} eq $addr || ($v->{ring1_addr} && $v->{ring1_addr} eq $addr)) {
+ die "corosync: address '$addr' already defined by node '$k'\n";
+ }
}
- }
- };
+ };
- &$check_duplicate_addr($param->{ring0_addr});
- &$check_duplicate_addr($param->{ring1_addr});
+ &$check_duplicate_addr($param->{ring0_addr});
+ &$check_duplicate_addr($param->{ring1_addr});
- $param->{ring0_addr} = $name if !$param->{ring0_addr};
+ $param->{ring0_addr} = $name if !$param->{ring0_addr};
- die "corosync: using 'ring1_addr' parameter needs a configured ring 1 interface!\n"
- if $param->{ring1_addr} && !defined($totem_cfg->{interface}->{1});
+ die "corosync: using 'ring1_addr' parameter needs a configured ring 1 interface!\n"
+ if $param->{ring1_addr} && !defined($totem_cfg->{interface}->{1});
- die "corosync: ring 1 interface configured but 'ring1_addr' parameter not defined!\n"
- if defined($totem_cfg->{interface}->{1}) && !defined($param->{ring1_addr});
+ die "corosync: ring 1 interface configured but 'ring1_addr' parameter not defined!\n"
+ if defined($totem_cfg->{interface}->{1}) && !defined($param->{ring1_addr});
- if (defined(my $res = $nodelist->{$name})) {
- $param->{nodeid} = $res->{nodeid} if !$param->{nodeid};
- $param->{votes} = $res->{quorum_votes} if !defined($param->{votes});
+ if (defined(my $res = $nodelist->{$name})) {
+ $param->{nodeid} = $res->{nodeid} if !$param->{nodeid};
+ $param->{votes} = $res->{quorum_votes} if !defined($param->{votes});
- if ($res->{quorum_votes} == $param->{votes} &&
- $res->{nodeid} == $param->{nodeid}) {
- print "node $name already defined\n";
- if ($param->{force}) {
- exit (0);
+ if ($res->{quorum_votes} == $param->{votes} &&
+ $res->{nodeid} == $param->{nodeid}) {
+ print "node $name already defined\n";
+ if ($param->{force}) {
+ exit (0);
+ } else {
+ exit (-1);
+ }
} else {
- exit (-1);
+ die "can't add existing node\n";
}
- } else {
- die "can't add existing node\n";
- }
- } elsif (!$param->{nodeid}) {
- my $nodeid = 1;
-
- while(1) {
- my $found = 0;
- foreach my $v (values %$nodelist) {
- if ($v->{nodeid} eq $nodeid) {
- $found = 1;
- $nodeid++;
- last;
+ } elsif (!$param->{nodeid}) {
+ my $nodeid = 1;
+
+ while(1) {
+ my $found = 0;
+ foreach my $v (values %$nodelist) {
+ if ($v->{nodeid} eq $nodeid) {
+ $found = 1;
+ $nodeid++;
+ last;
+ }
}
- }
- last if !$found;
- };
+ last if !$found;
+ };
- $param->{nodeid} = $nodeid;
- }
+ $param->{nodeid} = $nodeid;
+ }
- $param->{votes} = 1 if !defined($param->{votes});
+ $param->{votes} = 1 if !defined($param->{votes});
- PVE::Cluster::gen_local_dirs($name);
+ PVE::Cluster::gen_local_dirs($name);
- eval { PVE::Cluster::ssh_merge_keys(); };
- warn $@ if $@;
+ eval { PVE::Cluster::ssh_merge_keys(); };
+ warn $@ if $@;
+
+ $nodelist->{$name} = {
+ ring0_addr => $param->{ring0_addr},
+ nodeid => $param->{nodeid},
+ name => $name,
+ };
+ $nodelist->{$name}->{ring1_addr} = $param->{ring1_addr} if $param->{ring1_addr};
+ $nodelist->{$name}->{quorum_votes} = $param->{votes} if $param->{votes};
- $nodelist->{$name} = {
- ring0_addr => $param->{ring0_addr},
- nodeid => $param->{nodeid},
- name => $name,
+ PVE::Cluster::corosync_update_nodelist($conf, $nodelist);
};
- $nodelist->{$name}->{ring1_addr} = $param->{ring1_addr} if $param->{ring1_addr};
- $nodelist->{$name}->{quorum_votes} = $param->{votes} if $param->{votes};
- PVE::Cluster::corosync_update_nodelist($conf, $nodelist);
+ PVE::Cluster::cfs_lock_file('corosync.conf', 10, $code);
+ die $@ if $@;
exit (0);
}});
@@ -415,38 +420,43 @@ __PACKAGE__->register_method ({
PVE::Cluster::check_cfs_quorum();
- my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
+ my $code = sub {
+ my $conf = PVE::Cluster::cfs_read_file("corosync.conf");
- my $nodelist = PVE::Cluster::corosync_nodelist($conf);
+ my $nodelist = PVE::Cluster::corosync_nodelist($conf);
- my $node;
- my $nodeid;
+ my $node;
+ my $nodeid;
- foreach my $tmp_node (keys %$nodelist) {
- my $d = $nodelist->{$tmp_node};
- my $ring0_addr = $d->{ring0_addr};
- my $ring1_addr = $d->{ring1_addr};
- if (($tmp_node eq $param->{node}) ||
- (defined($ring0_addr) && ($ring0_addr eq $param->{node})) ||
- (defined($ring1_addr) && ($ring1_addr eq $param->{node}))) {
- $node = $tmp_node;
- $nodeid = $d->{nodeid};
- last;
+ foreach my $tmp_node (keys %$nodelist) {
+ my $d = $nodelist->{$tmp_node};
+ my $ring0_addr = $d->{ring0_addr};
+ my $ring1_addr = $d->{ring1_addr};
+ if (($tmp_node eq $param->{node}) ||
+ (defined($ring0_addr) && ($ring0_addr eq $param->{node})) ||
+ (defined($ring1_addr) && ($ring1_addr eq $param->{node}))) {
+ $node = $tmp_node;
+ $nodeid = $d->{nodeid};
+ last;
+ }
}
- }
- die "Node/IP: $param->{node} is not a known host of the cluster.\n"
+ die "Node/IP: $param->{node} is not a known host of the cluster.\n"
if !defined($node);
- my $our_nodename = PVE::INotify::nodename();
- die "Cannot delete myself from cluster!\n" if $node eq $our_nodename;
+ my $our_nodename = PVE::INotify::nodename();
+ die "Cannot delete myself from cluster!\n" if $node eq $our_nodename;
- delete $nodelist->{$node};
+ delete $nodelist->{$node};
- PVE::Cluster::corosync_update_nodelist($conf, $nodelist);
+ PVE::Cluster::corosync_update_nodelist($conf, $nodelist);
+
+ PVE::Tools::run_command(['corosync-cfgtool','-k', $nodeid])
+ if defined($nodeid);
+ };
- PVE::Tools::run_command(['corosync-cfgtool','-k', $nodeid])
- if defined($nodeid);
+ PVE::Cluster::cfs_lock_file('corosync.conf', 10, $code);
+ die $@ if $@;
return undef;
}});
--
2.11.0
More information about the pve-devel
mailing list