[pve-devel] [RFC cluster 2/2] corosync config parser: move to hash format

Thomas Lamprecht t.lamprecht at proxmox.com
Mon Jun 26 14:10:58 CEST 2017


The old parser itself was simple and easy but resulted in quite a bit
of headache when changing corosync config sections, especially if
multiple section levels should be touched.

Move to a more practical internal format which represents the
corosync configuration in hash format where subsections are saved in
the hash keys itself, not as an value. This reduces the level needed
for each corosync (sub)section from two to one.
We do not need to cycle through the parsed config when searching for
a specific subsection, we can use the respective section name as key.

The parser test need a small reordering, as we do now sort on all
keys at the same time, not first all key,value pairs then all
subsections like it was done by the old write_config logic.

I kept the 'nodelist' and 'totem_config' helpers for now so that the
API and CLI code do not need to get touched.

Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
---
 data/PVE/Corosync.pm                           | 170 +++++++++----------------
 data/test/corosync_configs/multiple-nodes.conf |   6 +-
 data/test/corosync_configs/single-node.conf    |   6 +-
 3 files changed, 63 insertions(+), 119 deletions(-)

diff --git a/data/PVE/Corosync.pm b/data/PVE/Corosync.pm
index 59b813e..297728c 100644
--- a/data/PVE/Corosync.pm
+++ b/data/PVE/Corosync.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 
 use Digest::SHA;
+use Clone 'clone';
 
 use PVE::Cluster;
 
@@ -25,21 +26,27 @@ sub parse_conf {
 
     my @tokens = split(/\s/, $raw);
 
-    my $conf = { section => 'main', children => [] };
+    my $conf = { 'main' => {} };
 
     my $stack = [];
-    my $section = $conf;
+    my $section = $conf->{main};
 
     while (defined(my $token = shift @tokens)) {
 	my $nexttok = $tokens[0];
 
 	if ($nexttok && ($nexttok eq '{')) {
 	    shift @tokens; # skip '{'
-	    my $new_section = {
-		section => $token,
-		children => [],
-	    };
-	    push @{$section->{children}}, $new_section;
+	    my $new_section = {};
+	    if (defined($section->{$token})) {
+		if (ref($section->{$token}) ne 'ARRAY') {
+		    my $tmp = $section->{$token};
+		    $section->{$token} = [ $tmp, $new_section ];
+		} else {
+		    push @{$section->{$token}}, $new_section;
+		}
+	    } else {
+		$section->{$token} = $new_section;
+	    }
 	    push @$stack, $section;
 	    $section = $new_section;
 	    next;
@@ -57,7 +64,7 @@ sub parse_conf {
 	die "parse error - no value for '$key'\n" if !defined($nexttok);
 	my $value = shift @tokens;
 
-	push @{$section->{children}}, { key => $key, value => $value };
+	$section->{$key} = $value;
     }
 
     $conf->{digest} = $digest;
@@ -69,20 +76,28 @@ my $dump_section;
 $dump_section = sub {
     my ($section, $prefix) = @_;
 
-    my $raw = $prefix . $section->{section} . " {\n";
-
-    my @list = grep { defined($_->{key}) } @{$section->{children}};
-    foreach my $child (sort {$a->{key} cmp $b->{key}} @list) {
-	$raw .= $prefix . "  $child->{key}: $child->{value}\n";
-    }
+    my $raw = '';
 
-    @list = grep { defined($_->{section}) } @{$section->{children}};
-    foreach my $child (sort {$a->{section} cmp $b->{section}} @list) {
-	$raw .= &$dump_section($child, "$prefix  ");
+    foreach my $k (sort keys %$section) {
+	my $v = $section->{$k};
+	if (ref($v) eq 'HASH') {
+	    $raw .= $prefix . "$k {\n";
+	    $raw .= &$dump_section($v, "$prefix  ");
+	    $raw .=  $prefix . "}\n";
+	    $raw .= "\n" if !$prefix; # add extra newline at 1st level only
+	} elsif (ref($v) eq 'ARRAY') {
+	    foreach my $child (@$v) {
+		$raw .= $prefix . "$k {\n";
+		$raw .= &$dump_section($child, "$prefix  ");
+		$raw .=  $prefix . "}\n";
+	    }
+	} elsif (!ref($v)) {
+	    $raw .= $prefix . "$k: $v\n";
+	} else {
+	    die "unexpected reference in config hash: ". ref($v) ."\n";
+	}
     }
 
-    $raw .= $prefix . "}\n\n";
-
     return $raw;
 
 };
@@ -90,21 +105,9 @@ $dump_section = sub {
 sub write_conf {
     my ($filename, $conf) = @_;
 
-    my $raw = '';
-
-    my $prefix = '';
-
-    die "no main section" if $conf->{section} ne 'main';
-
-    my @list = grep { defined($_->{key}) } @{$conf->{children}};
-    foreach my $child (sort {$a->{key} cmp $b->{key}} @list) {
-	$raw .= "$child->{key}: $child->{value}\n";
-    }
+    die "no main section" if !defined($conf->{main});
 
-    @list = grep { defined($_->{section}) } @{$conf->{children}};
-    foreach my $child (sort {$a->{section} cmp $b->{section}} @list) {
-	$raw .= &$dump_section($child, $prefix);
-    }
+    my $raw = &$dump_section($conf->{main}, '');
 
     return $raw;
 }
@@ -112,22 +115,11 @@ sub write_conf {
 sub conf_version {
     my ($conf, $noerr, $new_value) = @_;
 
-    foreach my $child (@{$conf->{children}}) {
-	next if !defined($child->{section});
-	if ($child->{section} eq 'totem') {
-	    foreach my $e (@{$child->{children}}) {
-		next if !defined($e->{key});
-		if ($e->{key} eq 'config_version') {
-		    if ($new_value) {
-			$e->{value} = $new_value;
-			return $new_value;
-		    } elsif (my $version = int($e->{value})) {
-			return $version;
-		    }
-		    last;
-		}
-	    }
+    if (defined($conf->{totem}) && defined($conf->{totem}->{conf_version})) {
+	if ($new_value) {
+	    $conf->{totem}->{conf_version} = $new_value;
 	}
+	return $conf->{totem}->{conf_version};
     }
 
     return undef if $noerr;
@@ -162,25 +154,7 @@ sub update_nodelist {
     my $version = conf_version($conf);
     conf_version($conf, undef, $version + 1);
 
-    my $children = [];
-    foreach my $v (values %$nodelist) {
-	next if !($v->{ring0_addr} || $v->{name});
-	my $kv = [];
-	foreach my $k (keys %$v) {
-	    push @$kv, { key => $k, value => $v->{$k} };
-	}
-	my $ns = { section => 'node', children => $kv };
-	push @$children, $ns;
-    }
-
-    foreach my $main (@{$conf->{children}}) {
-	next if !defined($main->{section});
-	if ($main->{section} eq 'nodelist') {
-	    $main->{children} = $children;
-	    last;
-	}
-    }
-
+    $conf->{main}->{nodelist} = [values %$nodelist];
 
     PVE::Cluster::cfs_write_file("corosync.conf.new", $conf);
 
@@ -193,65 +167,35 @@ sub nodelist {
 
     my $nodelist = {};
 
-    foreach my $main (@{$conf->{children}}) {
-	next if !defined($main->{section});
-	if ($main->{section} eq 'nodelist') {
-	    foreach my $ne (@{$main->{children}}) {
-		next if !defined($ne->{section}) || ($ne->{section} ne 'node');
-		my $node = { quorum_votes => 1 };
-		my $name;
-		foreach my $child (@{$ne->{children}}) {
-		    next if !defined($child->{key});
-		    $node->{$child->{key}} = $child->{value};
-		    # use 'name' over 'ring0_addr' if set
-		    if ($child->{key} eq 'name') {
-			delete $nodelist->{$name} if $name;
-			$name = $child->{value};
-			$nodelist->{$name} = $node;
-		    } elsif(!$name && $child->{key} eq 'ring0_addr') {
-			$name = $child->{value};
-			$nodelist->{$name} = $node;
-		    }
-		}
-	    }
+    my $nodes = $conf->{main}->{nodelist}->{node};
+    $nodes = [$nodes] if (ref($nodes) ne 'ARRAY');
+
+    foreach my $node (@$nodes) {
+	# use 'name' over 'ring0_addr' if set
+	my $name = $node->{name} // $node->{ring0_addr};
+	if ($name) {
+	    $nodelist->{$name} = $node;
 	}
     }
 
     return $nodelist;
 }
 
-# get a hash representation of the corosync config totem section
 sub totem_config {
     my ($conf) = @_;
 
-    my $res = {};
-
-    foreach my $main (@{$conf->{children}}) {
-	next if !defined($main->{section}) ||
-	    $main->{section} ne 'totem';
-
-	foreach my $e (@{$main->{children}}) {
-
-	    if ($e->{section} && $e->{section} eq 'interface') {
-		my $entry = {};
+    # we reorder elements from totem->interface and don't want to change $conf
+    my $totem = clone($conf->{main}->{totem});
 
-		$res->{interface} = {};
+    my $ifs = $totem->{interface};
+    $ifs = [$ifs] if (ref($ifs) ne 'ARRAY');
 
-		foreach my $child (@{$e->{children}}) {
-		    next if !defined($child->{key});
-		    $entry->{$child->{key}} = $child->{value};
-		    if($child->{key} eq 'ringnumber') {
-			$res->{interface}->{$child->{value}} = $entry;
-		    }
-		}
-
-	    } elsif  ($e->{key}) {
-		$res->{$e->{key}} = $e->{value};
-	    }
-	}
+    $totem->{interface} = {};
+    foreach my $if (@$ifs) {
+	$totem->{interface}->{$if->{ringnumber}} = $if;
     }
 
-    return $res;
+    return $totem;
 }
 
 1;
diff --git a/data/test/corosync_configs/multiple-nodes.conf b/data/test/corosync_configs/multiple-nodes.conf
index a67cc0c..c951f5b 100644
--- a/data/test/corosync_configs/multiple-nodes.conf
+++ b/data/test/corosync_configs/multiple-nodes.conf
@@ -41,9 +41,6 @@ quorum {
 totem {
   cluster_name: cloud
   config_version: 1
-  ip_version: ipv4
-  secauth: on
-  version: 2
   interface {
     bindnetaddr: 192.168.0.42
     ringnumber: 0
@@ -52,5 +49,8 @@ totem {
     bindnetaddr: 192.168.1.42
     ringnumber: 1
   }
+  ip_version: ipv4
+  secauth: on
+  version: 2
 }
 
diff --git a/data/test/corosync_configs/single-node.conf b/data/test/corosync_configs/single-node.conf
index 7a23f9a..c65a3e5 100644
--- a/data/test/corosync_configs/single-node.conf
+++ b/data/test/corosync_configs/single-node.conf
@@ -19,12 +19,12 @@ quorum {
 totem {
   cluster_name: cloud
   config_version: 1
-  ip_version: ipv4
-  secauth: on
-  version: 2
   interface {
     bindnetaddr: 192.168.0.42
     ringnumber: 0
   }
+  ip_version: ipv4
+  secauth: on
+  version: 2
 }
 
-- 
2.11.0





More information about the pve-devel mailing list