[pve-devel] [PATCH pve-client] implement config file locking

[Dietmar Maurer]

Signed-off-by: Dietmar Maurer <dietmar at proxmox.com>
---
 PVE/APIClient/Commands/config.pm | 57 +++++++++++++++--------------
 PVE/APIClient/Commands/remote.pm | 78 ++++++++++++++++++++++++----------------
 PVE/APIClient/Config.pm          | 12 +++++++
 3 files changed, 90 insertions(+), 57 deletions(-)

diff --git a/PVE/APIClient/Commands/config.pm b/PVE/APIClient/Commands/config.pm
index d467b4c..3b208a0 100644
--- a/PVE/APIClient/Commands/config.pm
+++ b/PVE/APIClient/Commands/config.pm
@@ -43,35 +44,37 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	# fixme: lock config file
-
 	my $digest = extract_param($param, 'digest');
 	my $delete = extract_param($param, 'delete');
 
-	my $config = PVE::APIClient::Config->load();
-	my $defaults = PVE::APIClient::Config->get_defaults($config);
-	
-	my $plugin = PVE::APIClient::Config->lookup('defaults');
-	my $opts = $plugin->check_config('defaults', $param, 0, 1);
-
-	foreach my $k (%$opts) {
-	    $defaults->{$k} = $opts->{$k};
-	}
-
-	if ($delete) {
-	    my $options = $plugin->private()->{options}->{'defaults'};
-	    foreach my $k (PVE::APIClient::Tools::split_list($delete)) {
-		my $d = $options->{$k} ||
-		    die "no such option '$k'\n";
-		die "unable to delete required option '$k'\n"
-		    if !$d->{optional};
-		die "unable to delete fixed option '$k'\n"
-		    if $d->{fixed};
-		delete $defaults->{$k};
+	my $code = sub {
+	    my $config = PVE::APIClient::Config->load();
+	    my $defaults = PVE::APIClient::Config->get_defaults($config);
+
+	    my $plugin = PVE::APIClient::Config->lookup('defaults');
+	    my $opts = $plugin->check_config('defaults', $param, 0, 1);
+
+	    foreach my $k (%$opts) {
+		$defaults->{$k} = $opts->{$k};
 	    }
-	}
 
-	PVE::APIClient::Config->save($config);
+	    if ($delete) {
+		my $options = $plugin->private()->{options}->{'defaults'};
+		foreach my $k (PVE::APIClient::Tools::split_list($delete)) {
+		    my $d = $options->{$k} ||
+			die "no such option '$k'\n";
+		    die "unable to delete required option '$k'\n"
+			if !$d->{optional};
+		    die "unable to delete fixed option '$k'\n"
+			if $d->{fixed};
+		    delete $defaults->{$k};
+		}
+	    }
+
+	    PVE::APIClient::Config->save($config);
+	};
+
+	PVE::APIClient::Config->lock_config(undef, $code);
 
 	return undef;
     }});
diff --git a/PVE/APIClient/Commands/remote.pm b/PVE/APIClient/Commands/remote.pm
index 090672e..03960af 100644
--- a/PVE/APIClient/Commands/remote.pm
+++ b/PVE/APIClient/Commands/remote.pm
@@ -3,6 +3,7 @@ package PVE::APIClient::Commands::remote;
 use strict;
 use warnings;
 
+use PVE::APIClient::Helpers;
 use PVE::APIClient::JSONSchema qw(get_standard_option);
 use PVE::APIClient::Tools qw(extract_param);
 use PVE::APIClient::Config;
@@ -51,10 +52,9 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	# fixme: lock config file
-
 	my $remote = $param->{name};
 
+	# Note: we try to keep lock time sort, and lock later when we have all info
 	my $config = PVE::APIClient::Config->load();
 
 	die "Remote '$remote' already exists\n"
@@ -90,11 +90,25 @@ __PACKAGE__->register_method ({
 	$api->login();
 
 	$param->{fingerprint} = $last_fp if !defined($param->{fingerprint});
+
 	my $plugin = PVE::APIClient::Config->lookup('remote');
-	my $opts = $plugin->check_config($remote, $param, 1, 1);
-	$config->{ids}->{$remote} = $opts;
 
-	PVE::APIClient::Config->save($config);
+	my $code = sub {
+
+	    $config = PVE::APIClient::Config->load(); # reload
+
+	    # check again (file is locked now)
+	    die "Remote '$remote' already exists\n"
+		if $config->{ids}->{$remote};
+
+	    my $opts = $plugin->check_config($remote, $param, 1, 1);
+
+	    $config->{ids}->{$remote} = $opts;
+
+	    PVE::APIClient::Config->save($config);
+	};
+
+	PVE::APIClient::Config->lock_config(undef, $code);
 
 	return undef;
     }});
@@ -109,36 +123,38 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	# fixme: lock config file
-
 	my $name = extract_param($param, 'name');
 	my $digest = extract_param($param, 'digest');
 	my $delete = extract_param($param, 'delete');
 
-	my $config = PVE::APIClient::Config->load();
-	my $remote = PVE::APIClient::Config->lookup_remote($config, $name);
+	my $code = sub {
+	    my $config = PVE::APIClient::Config->load();
+	    my $remote = PVE::APIClient::Config->lookup_remote($config, $name);
 
-	my $plugin = PVE::APIClient::Config->lookup('remote');
-	my $opts = $plugin->check_config($name, $param, 0, 1);
+	    my $plugin = PVE::APIClient::Config->lookup('remote');
+	    my $opts = $plugin->check_config($name, $param, 0, 1);
 
-	foreach my $k (%$opts) {
-	    $remote->{$k} = $opts->{$k};
-	}
+	    foreach my $k (%$opts) {
+		$remote->{$k} = $opts->{$k};
+	    }
 
-	if ($delete) {
-	    my $options = $plugin->private()->{options}->{'remote'};
-	    foreach my $k (PVE::APIClient::Tools::APIClient::split_list($delete)) {
-		my $d = $options->{$k} ||
-		    die "no such option '$k'\n";
-		die "unable to delete required option '$k'\n"
-		    if !$d->{optional};
-		die "unable to delete fixed option '$k'\n"
-		    if $d->{fixed};
-		delete $remote->{$k};
+	    if ($delete) {
+		my $options = $plugin->private()->{options}->{'remote'};
+		foreach my $k (PVE::APIClient::Tools::APIClient::split_list($delete)) {
+		    my $d = $options->{$k} ||
+			die "no such option '$k'\n";
+		    die "unable to delete required option '$k'\n"
+			if !$d->{optional};
+		    die "unable to delete fixed option '$k'\n"
+			if $d->{fixed};
+		    delete $remote->{$k};
+		}
 	    }
-	}
 
-	PVE::APIClient::Config->save($config);
+	    PVE::APIClient::Config->save($config);
+	};
+
+	PVE::APIClient::Config->lock_config(undef, $code);
 
 	return undef;
     }});
@@ -158,11 +174,13 @@ __PACKAGE__->register_method ({
     code => sub {
 	my ($param) = @_;
 
-	# fixme: lock config
+	my $code = sub {
+	    my $config = PVE::APIClient::Config->load();
+	    delete $config->{ids}->{$param->{name}};
+	    PVE::APIClient::Config->save($config);
+	};
 
-	my $config = PVE::APIClient::Config->load();
-	delete $config->{ids}->{$param->{name}};
-	PVE::APIClient::Config->save($config);
+	PVE::APIClient::Config->lock_config(undef, $code);
 
 	return undef;
     }});
diff --git a/PVE/APIClient/Config.pm b/PVE/APIClient/Config.pm
index 3878425..a4aa4c6 100644
--- a/PVE/APIClient/Config.pm
+++ b/PVE/APIClient/Config.pm
@@ -64,6 +64,18 @@ sub config_filename {
     return "$dir/config";
 }
 
+sub lock_config {
+    my ($class, $timeout, $code, @param) = @_;
+
+    my $filename = $class->config_filename();
+
+    my $res = PVE::APIClient::Tools::lock_file($filename, $timeout, $code, @param);
+
+    die $@ if $@;
+
+    return $res;
+}
+
 sub format_section_header {
     my ($class, $type, $sectionId, $scfg, $done_hash) = @_;
 
-- 
2.11.0