]> git.proxmox.com Git - qemu-server.git/commitdiff
replace change_config_nolock with update_config_nolock
authorDietmar Maurer <dietmar@proxmox.com>
Thu, 2 Feb 2012 13:01:08 +0000 (14:01 +0100)
committerDietmar Maurer <dietmar@proxmox.com>
Thu, 2 Feb 2012 13:18:41 +0000 (14:18 +0100)
We now use cfs_file_write() in order to avoid race conditions between
file IO and cfs operations (read after write works now).

PVE/API2/Qemu.pm
PVE/QemuMigrate.pm
PVE/QemuServer.pm
qm

index 59035d7e251362aedb6f01241ddd98a4e95cc089..f2217b67629da307cf26043f4f50596d92c3747a 100644 (file)
@@ -58,15 +58,15 @@ my $check_volume_access = sub {
 # Note: $pool is only needed when creating a VM, because pool permissions
 # are automatically inherited if VM already exists inside a pool.
 my $create_disks = sub {
-    my ($rpcenv, $authuser, $storecfg, $vmid, $pool, $settings, $default_storage) = @_;
+    my ($rpcenv, $authuser, $conf, $storecfg, $vmid, $pool, $settings, $default_storage) = @_;
 
     # check permissions first
 
     my $alloc = [];
-    foreach_drive($settings, sub {
+    PVE::QemuServer::foreach_drive($settings, sub {
        my ($ds, $disk) = @_;
 
-       return if drive_is_cdrom($disk);
+       return if PVE::QemuServer::drive_is_cdrom($disk);
 
        my $volid = $disk->{file};
 
@@ -112,7 +112,7 @@ my $create_disks = sub {
     foreach my $task (@$alloc) {
        my ($ds, $disk) = @$task;
        delete $disk->{format}; # no longer needed
-       $settings->{$ds} = PVE::QemuServer::print_drive($vmid, $disk);
+       $conf->{$ds} = $settings->{$ds} = PVE::QemuServer::print_drive($vmid, $disk);
     }
 
     return $vollist;
@@ -345,24 +345,26 @@ __PACKAGE__->register_method({
 
                my $vollist = [];
 
+               my $conf = $param;
+
                eval {
-                   $vollist = &$create_disks($rpcenv, $authuser, $storecfg, $vmid, $pool, $param, $storage);
+                   $vollist = &$create_disks($rpcenv, $authuser, $conf, $storecfg, $vmid, $pool, $param, $storage);
 
                    # try to be smart about bootdisk
                    my @disks = PVE::QemuServer::disknames();
                    my $firstdisk;
                    foreach my $ds (reverse @disks) {
-                       next if !$param->{$ds};
-                       my $disk = PVE::QemuServer::parse_drive($ds, $param->{$ds});
+                       next if !$conf->{$ds};
+                       my $disk = PVE::QemuServer::parse_drive($ds, $conf->{$ds});
                        next if PVE::QemuServer::drive_is_cdrom($disk);
                        $firstdisk = $ds;
                    }
 
-                   if (!$param->{bootdisk} && $firstdisk) {
-                       $param->{bootdisk} = $firstdisk;
+                   if (!$conf->{bootdisk} && $firstdisk) {
+                       $conf->{bootdisk} = $firstdisk;
                    }
 
-                   PVE::QemuServer::create_conf_nolock($vmid, $param);
+                   PVE::QemuServer::update_conf_nolock($vmid, $conf);
                };
                my $err = $@;
 
@@ -546,9 +548,7 @@ __PACKAGE__->register_method({
     }});
 
 my $delete_drive = sub {
-    my ($conf, $storecfg, $vmid, $drive, $force) = @_;
-
-    my $changes = {};
+    my ($conf, $storecfg, $vmid, $key, $drive, $force) = @_;
 
     if (!PVE::QemuServer::drive_is_cdrom($drive)) {
        my $volid = $drive->{file};
@@ -561,15 +561,14 @@ my $delete_drive = sub {
                    eval { PVE::Storage::vdisk_free($storecfg, $volid); };
                    warn $@ if $@;
                } else {
-                   PVE::QemuServer::add_unused_volume($conf, $volid, $vmid, $changes);
+                   PVE::QemuServer::add_unused_volume($conf, $volid, $vmid);
                }
+               delete $conf->{$key};
            }
        }
     } else {
 
     }
-
-    return $changes;
 };
 
 my $vm_config_perm_list = [
@@ -700,21 +699,20 @@ __PACKAGE__->register_method({
                $conf = PVE::QemuServer::load_config($vmid); # update/reload
 
                next if !defined($conf->{$opt});
-
+               
                die "error hot-unplug $opt" if !PVE::QemuServer::vm_deviceunplug($vmid, $conf, $opt);
 
-               my $changes = {};
-
-               if (my $drive = $drive_hash->{$opt}) { # drives
-                   $changes = &$delete_drive($conf, $storecfg, $vmid, $drive, $force);
+               if (PVE::QemuServer::valid_drivename($opt)) {
+                   my $drive = PVE::QemuServer::parse_drive($opt, $conf->{$opt});
+                   &$delete_drive($conf, $storecfg, $vmid, $opt, $drive, $force);
                } elsif ($opt =~ m/^unused/) {
                    my $drive =  PVE::QemuServer::parse_drive($opt, $conf->{$opt});
                     my $volid = $drive->{file};
                    eval { PVE::Storage::vdisk_free($storecfg, $volid); };
                    warn $@ if $@;
+                   delete $conf->{$opt};
                }
-
-               PVE::QemuServer::change_config_nolock($vmid, $changes, { $opt => 1 }, 1);
+               PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
            }
 
            foreach my $opt (keys %$param) { # add/change
@@ -731,9 +729,9 @@ __PACKAGE__->register_method({
                        !PVE::QemuServer::drive_is_cdrom($old_drive)) {  # delete old disks
 
                        die "error hot-unplug $opt" if !PVE::QemuServer::vm_deviceunplug($vmid, $conf, $opt);
-
-                       my $changes = &$delete_drive($conf, $storecfg, $vmid, $old_drive, 0);
-                       PVE::QemuServer::change_config_nolock($vmid, $changes, { $opt => 1 }, 1);
+                       
+                       &$delete_drive($conf, $storecfg, $vmid, $opt, $old_drive, 0);
+                       PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
                        $conf = PVE::QemuServer::load_config($vmid); # update/reload
                    }
                                    
@@ -749,13 +747,14 @@ __PACKAGE__->register_method({
                            }
                        }
 
-                       PVE::QemuServer::change_config_nolock($vmid, { $opt => $param->{$opt} }, {}, 1);
+                       $conf->{$opt} = $param->{$opt};
+                       PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
 
                    } else { #hdd
 
                        my $settings = { $opt => $param->{$opt} };
-                       &$create_disks($rpcenv, $authuser, $storecfg, $vmid, undef, $settings);
-                       PVE::QemuServer::change_config_nolock($vmid, { $opt => $settings->{$opt} }, {}, 1);
+                       &$create_disks($rpcenv, $authuser, $conf, $storecfg, $vmid, undef, $settings);
+                       PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
                        $conf = PVE::QemuServer::load_config($vmid); # update/reload
 
                        my $newdrive = PVE::QemuServer::parse_drive($opt, $conf->{$opt});
@@ -770,15 +769,18 @@ __PACKAGE__->register_method({
                    die "error hot-unplug $opt for update" if $conf->{$opt} && 
                        !PVE::QemuServer::vm_deviceunplug($vmid, $conf, $opt);
 
-                   PVE::QemuServer::change_config_nolock($vmid, { $opt => $param->{$opt} }, {}, 1);
+                   $conf->{$opt} = $param->{$opt};
+                   PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
                    $conf = PVE::QemuServer::load_config($vmid); # update/reload
 
                    my $newnet = PVE::QemuServer::parse_net($conf->{$opt});
+
                    die "error hotplug $opt" if !PVE::QemuServer::vm_deviceplug($storecfg, $conf, $vmid, $opt, $newnet);
 
                } else {
 
-                   PVE::QemuServer::change_config_nolock($vmid, { $opt => $param->{$opt} }, {}, 1);
+                   $conf->{$opt} = $param->{$opt};
+                   PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
                }
            }
        };
index be74bfe9e7f8e86fdead0fd8ce7e61cf095a33c1..0dc81a50612de03032e1c54f67af47f093f6b015 100644 (file)
@@ -258,7 +258,8 @@ sub phase1 {
     my $conf = $self->{vmconf};
 
     # set migrate lock in config file
-    PVE::QemuServer::change_config_nolock($vmid, { lock => 'migrate' }, {}, 1);
+    $conf->{lock} = 'migrate';
+    PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
 
     sync_disks($self, $vmid);
 
@@ -275,8 +276,9 @@ sub phase1_cleanup {
 
     $self->log('info', "aborting phase 1 - cleanup resources");
 
-    my $unset = { lock => 1 };
-    eval { PVE::QemuServer::change_config_nolock($vmid, {}, $unset, 1) };
+    my $conf = $self->{vmconf};
+    delete $conf->{lock};
+    eval { PVE::QemuServer::update_config_nolock($vmid, $conf, 1) };
     if (my $err = $@) {
        $self->log('err', $err);
     }
index 2c024935e31f178f26136ee3fd8d1ffc3d5e27a8..fcf7b885be943509907d8dbb4a04bd4003d825e3 100644 (file)
@@ -35,7 +35,9 @@ my $cpuinfo = PVE::ProcFSTools::read_cpuinfo();
 # allowed when such lock is set. But you can ignore this kind of
 # lock with the --skiplock flag.
 
-cfs_register_file('/qemu-server/', \&parse_vm_config);
+cfs_register_file('/qemu-server/', 
+                 \&parse_vm_config,
+                 \&write_vm_config);
 
 PVE::JSONSchema::register_standard_option('skiplock', {
     description => "Ignore locks - only root is allowed to use this option.",
@@ -1084,7 +1086,7 @@ sub add_random_macs {
 }
 
 sub add_unused_volume {
-    my ($config, $volid, $changes) = @_;
+    my ($config, $volid) = @_;
 
     my $key;
     for (my $ind = $MAX_UNUSED_DISKS - 1; $ind >= 0; $ind--) {
@@ -1097,8 +1099,10 @@ sub add_unused_volume {
     }
 
     die "To many unused volume - please delete them first.\n" if !$key;
+    
+    $config->{$key} = $volid;
 
-    $changes->{$key} = $config->{$key} = $volid;
+    return $key;
 }
 
 # fixme: remove all thos $noerr parameters?
@@ -1483,127 +1487,74 @@ sub parse_vm_config {
     return $res;
 }
 
-sub change_config {
-    my ($vmid, $settings, $unset, $skiplock) = @_;
+sub write_vm_config {
+    my ($filename, $conf) = @_;
 
-    lock_config($vmid, &change_config_nolock, $settings, $unset, $skiplock);
-}
-
-sub change_config_nolock {
-    my ($vmid, $settings, $unset, $skiplock) = @_;
-
-    my $res = {};
-
-    $unset->{ide2} = $unset->{cdrom} if $unset->{cdrom};
-
-    check_lock($settings) if !$skiplock;
+    if ($conf->{cdrom}) {
+       die "option ide2 conflicts with cdrom\n" if $conf->{ide2};
+       $conf->{ide2} = $conf->{cdrom};
+       delete $conf->{cdrom};
+    }
 
     # we do not use 'smp' any longer
-    if ($settings->{sockets}) {
-       $unset->{smp} = 1;
-    } elsif ($settings->{smp}) {
-       $settings->{sockets} = $settings->{smp};
-       $unset->{smp} = 1;
+    if ($conf->{sockets}) {
+       delete $conf->{smp};
+    } elsif ($conf->{smp}) {
+       $conf->{sockets} = $conf->{smp};
+       delete $conf->{cores};
+       delete $conf->{smp};
     }
 
     my $new_volids = {};
-
-    foreach my $key (keys %$settings) {
+    foreach my $key (keys %$conf) {
        next if $key eq 'digest';
-       my $value = $settings->{$key};
+       my $value = $conf->{$key};
        if ($key eq 'description') {
            $value = PVE::Tools::encode_text($value);
        }
        eval { $value = check_type($key, $value); };
        die "unable to parse value of '$key' - $@" if $@;
-       if ($key eq 'cdrom') {
-           $res->{ide2} = $value;
-       } else {
-           $res->{$key} = $value;
-       }
+
+       $conf->{$key} = $value;
+
        if (valid_drivename($key)) {
            my $drive = PVE::QemuServer::parse_drive($key, $value);
            $new_volids->{$drive->{file}} = 1 if $drive && $drive->{file};
        }
     }
 
-    my $filename = config_file($vmid);
-    my $tmpfn = "$filename.$$.tmp";
-
-    my $fh = new IO::File($filename, "r") ||
-       die "unable to read config for VM $vmid\n";
-
-    my $werror = "unable to write config for VM $vmid\n";
-
-    my $out = new IO::File($tmpfn, "w") || die $werror;
-
-    eval {
-
-       my $done;
-
-       while (my $line = <$fh>) {
-
-           if (($line =~ m/^\#/) || ($line =~ m/^\s*$/)) {
-               die $werror unless print $out $line;
-               next;
-           }
-
-           if ($line =~ m/^([a-z][a-z_]*\d*):\s*(.*\S)\s*$/) {
-               my $key = $1;
-               my $value = $2;
-
-               # remove 'unusedX' settings if we re-add a volume
-               next if $key =~ m/^unused/ && $new_volids->{$value};
-
-               # convert 'smp' to 'sockets'
-               $key = 'sockets' if $key eq 'smp';
-
-               next if $done->{$key};
-               $done->{$key} = 1;
-
-               if (defined($res->{$key})) {
-                   $value = $res->{$key};
-                   delete $res->{$key};
-               }
-               if (!defined($unset->{$key})) {
-                   die $werror unless print $out "$key: $value\n";
-               }
-
-               next;
-           }
-
-           die "unable to parse config file: $line\n";
+    # remove 'unusedX' settings if we re-add a volume
+    foreach my $key (keys %$conf) {
+       my $value = $conf->{$key};
+       if ($key =~ m/^unused/ && $new_volids->{$value}) {
+           delete $conf->{$key};
        }
+    }
 
-       foreach my $key (keys %$res) {
+    # gererate RAW data
+    my $raw = '';
+    foreach my $key (sort keys %$conf) {
+       next if $key eq 'digest';
+       $raw .= "$key: $conf->{$key}\n";
+    }
 
-           if (!defined($unset->{$key})) {
-               die $werror unless print $out "$key: $res->{$key}\n";
-           }
-       }
-    };
+    return $raw;
+}
 
-    my $err = $@;
+sub update_config_nolock {
+    my ($vmid, $conf, $skiplock) = @_;
 
-    $fh->close();
+    check_lock($conf) if !$skiplock;
+    
+    my $cfspath = cfs_config_path($vmid);
 
-    if ($err) {
-       $out->close();
-       unlink $tmpfn;
-       die $err;
-    }
+    PVE::Cluster::cfs_write_file($cfspath, $conf);
+}
 
-    if (!$out->close()) {
-       $err = "close failed - $!\n";
-       unlink $tmpfn;
-       die $err;
-    }
+sub update_config {
+    my ($vmid, $conf, $skiplock) = @_;
 
-    if (!rename($tmpfn, $filename)) {
-       $err = "rename failed - $!\n";
-       unlink $tmpfn;
-       die $err;
-    }
+    lock_config($vmid, &update_config_nolock, $conf, $skiplock);
 }
 
 sub load_defaults {
diff --git a/qm b/qm
index 0f24fda91eb468fdbcd87c7791e169eb569003cc..a1c30b66263c0805ea0cb13015e53687ae6cae9b 100755 (executable)
--- a/qm
+++ b/qm
@@ -185,7 +185,9 @@ __PACKAGE__->register_method ({
        my $vmid = $param->{vmid};
 
        PVE::QemuServer::lock_config ($vmid, sub {
-           PVE::QemuServer::change_config_nolock  ($vmid, {}, { lock => 1 }, 1);
+           my $conf = PVE::QemuServer::load_config($vmid);
+           delete $conf->{lock};
+           PVE::QemuServer::update_config_nolock($vmid, $conf, 1);
        });
 
        return undef;