implement config file locking
authorDietmar Maurer <dietmar@proxmox.com>
Fri, 15 Jun 2018 10:26:28 +0000 (12:26 +0200)
committerDietmar Maurer <dietmar@proxmox.com>
Fri, 15 Jun 2018 10:28:05 +0000 (12:28 +0200)
PVE/APIClient/Commands/config.pm
PVE/APIClient/Commands/remote.pm
PVE/APIClient/Config.pm

index d467b4c..3b208a0 100644 (file)
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use Data::Dumper;
 
+use PVE::APIClient::Helpers;
 use PVE::APIClient::JSONSchema qw(get_standard_option);
 use PVE::APIClient::Tools qw(extract_param);
 use PVE::APIClient::Config;
@@ -24,12 +25,12 @@ __PACKAGE__->register_method ({
     code => sub {
 
        my $config = PVE::APIClient::Config->load();
-       
+
        my $defaults = PVE::APIClient::Config->get_defaults($config);
 
-       
+
        print Dumper($config);
-       
+
        return undef;
     }});
 
@@ -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;
     }});
index 090672e..03960af 100644 (file)
@@ -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;
     }});
index 3878425..a4aa4c6 100644 (file)
@@ -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) = @_;