pvecm: lock corosync config on addition and deletion

This avoids potential races which would lead to an inconsistent
corosync config.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
(cherry picked from commit b778c013a2fc245008512365ac0b1a37287b30b3)

diff --git a/data/PVE/CLI/pvecm.pm b/data/PVE/CLI/pvecm.pm
index 05a68e511d74b1bc0496cc3099cf3fa201870dc0..36fe93b8c4903ec14fc31a7d9e1a7c0ce709801b 100755 (executable)
--- 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;
     }});