Refactor update_config_nolock -> write_config

The method update_config wrapped update_config_nolock
using lock_config, but to prevent update races the whole
"read config", "do something", "write config" flow was
always protected by lock_config anyway, and update_config
was never called.

Thus, we can safely drop update_config and rename
update_config_nolock to write_config like in PVE::LXC .
This commit is contained in:
Fabian Grünbichler 2016-02-12 11:14:40 +01:00 committed by Dietmar Maurer
parent a3683dfb03
commit 63be43a947
4 changed files with 34 additions and 41 deletions

View File

@ -457,7 +457,7 @@ __PACKAGE__->register_method({
$conf->{smbios1} = "uuid=$uuid_str"; $conf->{smbios1} = "uuid=$uuid_str";
} }
PVE::QemuServer::update_config_nolock($vmid, $conf); PVE::QemuServer::write_config($vmid, $conf);
}; };
my $err = $@; my $err = $@;
@ -908,7 +908,7 @@ my $update_vm_api = sub {
$rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']); $rpcenv->check_vm_perm($authuser, $vmid, undef, ['VM.Config.Disk']);
if (PVE::QemuServer::try_deallocate_drive($storecfg, $vmid, $conf, $opt, $drive, $rpcenv, $authuser)) { if (PVE::QemuServer::try_deallocate_drive($storecfg, $vmid, $conf, $opt, $drive, $rpcenv, $authuser)) {
delete $conf->{$opt}; delete $conf->{$opt};
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
} }
} elsif (PVE::QemuServer::valid_drivename($opt)) { } elsif (PVE::QemuServer::valid_drivename($opt)) {
&$check_protection($conf, "can't remove drive '$opt'"); &$check_protection($conf, "can't remove drive '$opt'");
@ -916,10 +916,10 @@ my $update_vm_api = sub {
PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt})) PVE::QemuServer::vmconfig_register_unused_drive($storecfg, $vmid, $conf, PVE::QemuServer::parse_drive($opt, $conf->{pending}->{$opt}))
if defined($conf->{pending}->{$opt}); if defined($conf->{pending}->{$opt});
PVE::QemuServer::vmconfig_delete_pending_option($conf, $opt, $force); PVE::QemuServer::vmconfig_delete_pending_option($conf, $opt, $force);
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
} else { } else {
PVE::QemuServer::vmconfig_delete_pending_option($conf, $opt, $force); PVE::QemuServer::vmconfig_delete_pending_option($conf, $opt, $force);
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
} }
} }
@ -943,13 +943,13 @@ my $update_vm_api = sub {
$conf->{pending}->{$opt} = $param->{$opt}; $conf->{pending}->{$opt} = $param->{$opt};
} }
PVE::QemuServer::vmconfig_undelete_pending_option($conf, $opt); PVE::QemuServer::vmconfig_undelete_pending_option($conf, $opt);
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
} }
# remove pending changes when nothing changed # remove pending changes when nothing changed
$conf = PVE::QemuServer::load_config($vmid); # update/reload $conf = PVE::QemuServer::load_config($vmid); # update/reload
my $changes = PVE::QemuServer::vmconfig_cleanup_pending($conf); my $changes = PVE::QemuServer::vmconfig_cleanup_pending($conf);
PVE::QemuServer::update_config_nolock($vmid, $conf, 1) if $changes; PVE::QemuServer::write_config($vmid, $conf, 1) if $changes;
return if !scalar(keys %{$conf->{pending}}); return if !scalar(keys %{$conf->{pending}});
@ -2292,11 +2292,11 @@ __PACKAGE__->register_method({
$newconf->{$opt} = PVE::QemuServer::print_drive($vmid, $newdrive); $newconf->{$opt} = PVE::QemuServer::print_drive($vmid, $newdrive);
PVE::QemuServer::update_config_nolock($newid, $newconf, 1); PVE::QemuServer::write_config($newid, $newconf, 1);
} }
delete $newconf->{lock}; delete $newconf->{lock};
PVE::QemuServer::update_config_nolock($newid, $newconf, 1); PVE::QemuServer::write_config($newid, $newconf, 1);
if ($target) { if ($target) {
# always deactivate volumes - avoid lvm LVs to be active on several nodes # always deactivate volumes - avoid lvm LVs to be active on several nodes
@ -2455,7 +2455,7 @@ __PACKAGE__->register_method({
PVE::QemuServer::add_unused_volume($conf, $old_volid) if !$param->{delete}; PVE::QemuServer::add_unused_volume($conf, $old_volid) if !$param->{delete};
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
eval { eval {
# try to deactivate volumes - avoid lvm LVs to be active on several nodes # try to deactivate volumes - avoid lvm LVs to be active on several nodes
@ -2477,7 +2477,7 @@ __PACKAGE__->register_method({
if (PVE::QemuServer::is_volume_in_use($storecfg, $conf, undef, $old_volid)) { if (PVE::QemuServer::is_volume_in_use($storecfg, $conf, undef, $old_volid)) {
warn "volume $old_volid still has snapshots, can't delete it\n"; warn "volume $old_volid still has snapshots, can't delete it\n";
PVE::QemuServer::add_unused_volume($conf, $old_volid); PVE::QemuServer::add_unused_volume($conf, $old_volid);
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
} else { } else {
eval { PVE::Storage::vdisk_free($storecfg, $old_volid); }; eval { PVE::Storage::vdisk_free($storecfg, $old_volid); };
warn $@ if $@; warn $@ if $@;
@ -2748,7 +2748,7 @@ __PACKAGE__->register_method({
$drive->{size} = $newsize; $drive->{size} = $newsize;
$conf->{$disk} = PVE::QemuServer::print_drive($vmid, $drive); $conf->{$disk} = PVE::QemuServer::print_drive($vmid, $drive);
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
}; };
PVE::QemuServer::lock_config($vmid, $updatefn); PVE::QemuServer::lock_config($vmid, $updatefn);
@ -2953,7 +2953,7 @@ __PACKAGE__->register_method({
$snap->{description} = $param->{description} if defined($param->{description}); $snap->{description} = $param->{description} if defined($param->{description});
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
}; };
PVE::QemuServer::lock_config($vmid, $updatefn); PVE::QemuServer::lock_config($vmid, $updatefn);
@ -3149,7 +3149,7 @@ __PACKAGE__->register_method({
}; };
$conf->{template} = 1; $conf->{template} = 1;
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
return $rpcenv->fork_worker('qmtemplate', $vmid, $authuser, $realcmd); return $rpcenv->fork_worker('qmtemplate', $vmid, $authuser, $realcmd);
}; };

View File

@ -188,7 +188,7 @@ __PACKAGE__->register_method ({
my $conf = PVE::QemuServer::load_config($vmid); my $conf = PVE::QemuServer::load_config($vmid);
delete $conf->{lock}; delete $conf->{lock};
delete $conf->{pending}->{lock} if $conf->{pending}; # just to be sure delete $conf->{pending}->{lock} if $conf->{pending}; # just to be sure
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
}); });
return undef; return undef;

View File

@ -277,7 +277,7 @@ sub phase1 {
# set migrate lock in config file # set migrate lock in config file
$conf->{lock} = 'migrate'; $conf->{lock} = 'migrate';
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); PVE::QemuServer::write_config($vmid, $conf, 1);
sync_disks($self, $vmid); sync_disks($self, $vmid);
@ -290,7 +290,7 @@ sub phase1_cleanup {
my $conf = $self->{vmconf}; my $conf = $self->{vmconf};
delete $conf->{lock}; delete $conf->{lock};
eval { PVE::QemuServer::update_config_nolock($vmid, $conf, 1) }; eval { PVE::QemuServer::write_config($vmid, $conf, 1) };
if (my $err = $@) { if (my $err = $@) {
$self->log('err', $err); $self->log('err', $err);
} }
@ -546,7 +546,7 @@ sub phase2_cleanup {
my $conf = $self->{vmconf}; my $conf = $self->{vmconf};
delete $conf->{lock}; delete $conf->{lock};
eval { PVE::QemuServer::update_config_nolock($vmid, $conf, 1) }; eval { PVE::QemuServer::write_config($vmid, $conf, 1) };
if (my $err = $@) { if (my $err = $@) {
$self->log('err', $err); $self->log('err', $err);
} }

View File

@ -2244,7 +2244,7 @@ sub write_vm_config {
return $raw; return $raw;
} }
sub update_config_nolock { sub write_config {
my ($vmid, $conf, $skiplock) = @_; my ($vmid, $conf, $skiplock) = @_;
check_lock($conf) if !$skiplock; check_lock($conf) if !$skiplock;
@ -2254,12 +2254,6 @@ sub update_config_nolock {
PVE::Cluster::cfs_write_file($cfspath, $conf); PVE::Cluster::cfs_write_file($cfspath, $conf);
} }
sub update_config {
my ($vmid, $conf, $skiplock) = @_;
lock_config($vmid, &update_config_nolock, $conf, $skiplock);
}
sub load_defaults { sub load_defaults {
my $res = {}; my $res = {};
@ -3219,7 +3213,7 @@ sub config_to_command {
#if dimm_memory is not aligned to dimm map #if dimm_memory is not aligned to dimm map
if($current_size > $memory) { if($current_size > $memory) {
$conf->{memory} = $current_size; $conf->{memory} = $current_size;
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
} }
}); });
} }
@ -3876,7 +3870,7 @@ sub qemu_memory_hotplug {
} }
#update conf after each succesful module hotplug #update conf after each succesful module hotplug
$conf->{memory} = $current_size; $conf->{memory} = $current_size;
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
}); });
} else { } else {
@ -3901,7 +3895,7 @@ sub qemu_memory_hotplug {
$conf->{memory} = $current_size; $conf->{memory} = $current_size;
eval { qemu_objectdel($vmid, "mem-$name"); }; eval { qemu_objectdel($vmid, "mem-$name"); };
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
}); });
} }
} }
@ -4154,7 +4148,7 @@ sub vmconfig_hotplug_pending {
} }
if ($changes) { if ($changes) {
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
$conf = load_config($vmid); # update/reload $conf = load_config($vmid); # update/reload
} }
@ -4205,7 +4199,7 @@ sub vmconfig_hotplug_pending {
# save new config if hotplug was successful # save new config if hotplug was successful
delete $conf->{$opt}; delete $conf->{$opt};
vmconfig_undelete_pending_option($conf, $opt); vmconfig_undelete_pending_option($conf, $opt);
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
$conf = load_config($vmid); # update/reload $conf = load_config($vmid); # update/reload
} }
} }
@ -4263,7 +4257,7 @@ sub vmconfig_hotplug_pending {
# save new config if hotplug was successful # save new config if hotplug was successful
$conf->{$opt} = $value; $conf->{$opt} = $value;
delete $conf->{pending}->{$opt}; delete $conf->{pending}->{$opt};
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
$conf = load_config($vmid); # update/reload $conf = load_config($vmid); # update/reload
} }
} }
@ -4319,16 +4313,16 @@ sub vmconfig_apply_pending {
$conf = load_config($vmid); # update/reload $conf = load_config($vmid); # update/reload
if (!defined($conf->{$opt})) { if (!defined($conf->{$opt})) {
vmconfig_undelete_pending_option($conf, $opt); vmconfig_undelete_pending_option($conf, $opt);
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
} elsif (valid_drivename($opt)) { } elsif (valid_drivename($opt)) {
vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force); vmconfig_delete_or_detach_drive($vmid, $storecfg, $conf, $opt, $force);
vmconfig_undelete_pending_option($conf, $opt); vmconfig_undelete_pending_option($conf, $opt);
delete $conf->{$opt}; delete $conf->{$opt};
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
} else { } else {
vmconfig_undelete_pending_option($conf, $opt); vmconfig_undelete_pending_option($conf, $opt);
delete $conf->{$opt}; delete $conf->{$opt};
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
} }
} }
@ -4348,7 +4342,7 @@ sub vmconfig_apply_pending {
} }
delete $conf->{pending}->{$opt}; delete $conf->{pending}->{$opt};
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
} }
} }
@ -5428,7 +5422,7 @@ sub rescan {
my $changes = update_disksize($vmid, $conf, $vm_volids); my $changes = update_disksize($vmid, $conf, $vm_volids);
update_config_nolock($vmid, $conf, 1) if $changes; write_config($vmid, $conf, 1) if $changes;
}; };
if (defined($vmid)) { if (defined($vmid)) {
@ -5953,8 +5947,7 @@ my $snapshot_prepare = sub {
# always overwrite machine if we save vmstate. This makes sure we # always overwrite machine if we save vmstate. This makes sure we
# can restore it later using correct machine type # can restore it later using correct machine type
$snap->{machine} = get_current_qemu_machine($vmid) if $snap->{vmstate}; $snap->{machine} = get_current_qemu_machine($vmid) if $snap->{vmstate};
write_config($vmid, $conf, 1);
update_config_nolock($vmid, $conf, 1);
}; };
lock_config($vmid, $updatefn); lock_config($vmid, $updatefn);
@ -5990,7 +5983,7 @@ my $snapshot_commit = sub {
$newconf->{parent} = $snapname; $newconf->{parent} = $snapname;
update_config_nolock($vmid, $newconf, 1); write_config($vmid, $newconf, 1);
}; };
lock_config($vmid, $updatefn); lock_config($vmid, $updatefn);
@ -6069,7 +6062,7 @@ sub snapshot_rollback {
delete $conf->{machine} if $snap->{vmstate} && !$has_machine_config; delete $conf->{machine} if $snap->{vmstate} && !$has_machine_config;
} }
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
if (!$prepare && $snap->{vmstate}) { if (!$prepare && $snap->{vmstate}) {
my $statefile = PVE::Storage::path($storecfg, $snap->{vmstate}); my $statefile = PVE::Storage::path($storecfg, $snap->{vmstate});
@ -6277,7 +6270,7 @@ sub snapshot_delete {
} }
} }
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
}; };
lock_config($vmid, $updatefn); lock_config($vmid, $updatefn);
@ -6355,7 +6348,7 @@ sub template_create {
my $voliddst = PVE::Storage::vdisk_create_base($storecfg, $volid); my $voliddst = PVE::Storage::vdisk_create_base($storecfg, $volid);
$drive->{file} = $voliddst; $drive->{file} = $voliddst;
$conf->{$ds} = print_drive($vmid, $drive); $conf->{$ds} = print_drive($vmid, $drive);
update_config_nolock($vmid, $conf, 1); write_config($vmid, $conf, 1);
}); });
} }