[pve-devel] [PATCH cluster v2 5/7] corosync: add atomic_write_conf and cleanup

[Fabian Grünbichler]

On Thu, Sep 14, 2017 at 10:00:39AM +0200, Thomas Lamprecht wrote:
> Signed-off-by: Thomas Lamprecht <t.lamprecht at 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?

>  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 ;)

> +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 at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel