On 09/14/2017 10:18 AM, Fabian Grünbichler wrote:
On Thu, Sep 14, 2017 at 10:00:39AM +0200, Thomas Lamprecht wrote:
Signed-off-by: Thomas Lamprecht <t.lampre...@proxmox.com>
---
  data/PVE/Corosync.pm | 39 ++++++++++++++++-----------------------
  1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
index 1180316..1d58bf0 100644
--- a/data/PVE/Corosync.pm
+++ b/data/PVE/Corosync.pm
@@ -146,20 +146,6 @@ sub write_conf {
      return $raw;
  }
-sub conf_version {
-    my ($conf, $noerr, $new_value) = @_;
-
-    my $totem = $conf->{main}->{totem};
-    if (defined($totem) && defined($totem->{config_version})) {
-       $totem->{config_version} = $new_value if $new_value;
-       return $totem->{config_version};
-    }
-
-    return undef if $noerr;
-
-    die "invalid corosync config - unable to read version\n";
-}
-
  # read only - use "rename corosync.conf.new corosync.conf" to write

minor nitpick: this comment should maybe be updated to refer to the new
atomic_write_conf?


Yes, makes sense to do so!

  PVE::Cluster::cfs_register_file('corosync.conf', \&parse_conf);
  # this is read/write
@@ -182,17 +168,9 @@ sub check_conf_exists {
  sub update_nodelist {
      my ($conf, $nodelist) = @_;
- delete $conf->{digest};
-
-    my $version = conf_version($conf);
-    conf_version($conf, undef, $version + 1);
-
      $conf->{main}->{nodelist}->{node} = $nodelist;
- PVE::Cluster::cfs_write_file("corosync.conf.new", $conf);
-
-    rename("/etc/pve/corosync.conf.new", "/etc/pve/corosync.conf")
-       || die "activate  corosync.conf.new failed - $!\n";
+    atomic_write_conf($conf, 1);
  }
sub nodelist {
@@ -205,4 +183,19 @@ sub totem_config {
      return clone($conf->{main}->{totem});
  }

maybe add a comment here that you need to hold a cluster lock if you
want to call this ;)


Yes, for read-modify-write it must be locked, the current users of this
method also do this (not since long, I added it ~two months ago)
Will do so, thanks.

+sub atomic_write_conf {
+    my ($conf, $increase_version) = @_;
+
+    if ($increase_version) {
+       die "invalid corosync config: unable to read config version\n"
+           if !defined($conf->{main}->{totem}->{config_version});
+       $conf->{main}->{totem}->{config_version}++;
+    }
+
+    PVE::Cluster::cfs_write_file("corosync.conf.new", $conf);
+
+    rename("/etc/pve/corosync.conf.new", "/etc/pve/corosync.conf")
+       || die "activating corosync.conf.new failed - $!\n";
+}
+
  1;
--
2.11.0


_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel



_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to