[pve-devel] [PATCH container 3/6] clone_vm: reduce source flock scope
Fabian Grünbichler
f.gruenbichler at proxmox.com
Fri Jun 18 14:51:20 CEST 2021
set_lock already obtains the flock (since it does a read-modify-write
cycle), and the rest of this code does not touch the config file in any
fashion so no need to hold the flock either..
Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
Notes:
best viewed with -w ;)
src/PVE/API2/LXC.pm | 163 ++++++++++++++++++++++----------------------
1 file changed, 81 insertions(+), 82 deletions(-)
diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm
index 807709a..1554ef2 100644
--- a/src/PVE/API2/LXC.pm
+++ b/src/PVE/API2/LXC.pm
@@ -1395,116 +1395,115 @@ __PACKAGE__->register_method({
PVE::LXC::Config->create_and_lock_config($newid, 0);
- PVE::LXC::Config->lock_config($vmid, sub {
- my $src_conf = PVE::LXC::Config->set_lock($vmid, 'disk');
-
- $running = PVE::LXC::check_running($vmid) || 0;
- my $full = extract_param($param, 'full');
- if (!defined($full)) {
- $full = !PVE::LXC::Config->is_template($src_conf);
- }
- die "parameter 'storage' not allowed for linked clones\n" if defined($storage) && !$full;
+ my $src_conf = PVE::LXC::Config->set_lock($vmid, 'disk');
- eval {
- die "snapshot '$snapname' does not exist\n"
- if $snapname && !defined($src_conf->{snapshots}->{$snapname});
+ $running = PVE::LXC::check_running($vmid) || 0;
- my $src_conf = $snapname ? $src_conf->{snapshots}->{$snapname} : $src_conf;
+ my $full = extract_param($param, 'full');
+ if (!defined($full)) {
+ $full = !PVE::LXC::Config->is_template($src_conf);
+ }
+ die "parameter 'storage' not allowed for linked clones\n" if defined($storage) && !$full;
- my $sharedvm = 1;
- for my $opt (sort keys %$src_conf) {
- next if $opt =~ m/^unused\d+$/;
+ eval {
+ die "snapshot '$snapname' does not exist\n"
+ if $snapname && !defined($src_conf->{snapshots}->{$snapname});
- my $value = $src_conf->{$opt};
+ my $src_conf = $snapname ? $src_conf->{snapshots}->{$snapname} : $src_conf;
- if (($opt eq 'rootfs') || ($opt =~ m/^mp\d+$/)) {
- my $mp = PVE::LXC::Config->parse_volume($opt, $value);
+ my $sharedvm = 1;
+ for my $opt (sort keys %$src_conf) {
+ next if $opt =~ m/^unused\d+$/;
- if ($mp->{type} eq 'volume') {
- my $volid = $mp->{volume};
+ my $value = $src_conf->{$opt};
- my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
- $sid = $storage if defined($storage);
- my $scfg = PVE::Storage::storage_config($storecfg, $sid);
- if (!$scfg->{shared}) {
- $sharedvm = 0;
- warn "found non-shared volume: $volid\n" if $target;
- }
+ if (($opt eq 'rootfs') || ($opt =~ m/^mp\d+$/)) {
+ my $mp = PVE::LXC::Config->parse_volume($opt, $value);
- $rpcenv->check($authuser, "/storage/$sid", ['Datastore.AllocateSpace']);
+ if ($mp->{type} eq 'volume') {
+ my $volid = $mp->{volume};
- if ($full) {
- die "Cannot do full clones on a running container without snapshots\n"
- if $running && !defined($snapname);
- $fullclone->{$opt} = 1;
- } else {
- # not full means clone instead of copy
- die "Linked clone feature for '$volid' is not available\n"
- if !PVE::Storage::volume_has_feature($storecfg, 'clone', $volid, $snapname, $running, {'valid_target_formats' => ['raw', 'subvol']});
- }
+ my ($sid, $volname) = PVE::Storage::parse_volume_id($volid);
+ $sid = $storage if defined($storage);
+ my $scfg = PVE::Storage::storage_config($storecfg, $sid);
+ if (!$scfg->{shared}) {
+ $sharedvm = 0;
+ warn "found non-shared volume: $volid\n" if $target;
+ }
- $mountpoints->{$opt} = $mp;
- push @$vollist, $volid;
+ $rpcenv->check($authuser, "/storage/$sid", ['Datastore.AllocateSpace']);
+ if ($full) {
+ die "Cannot do full clones on a running container without snapshots\n"
+ if $running && !defined($snapname);
+ $fullclone->{$opt} = 1;
} else {
- # TODO: allow bind mounts?
- die "unable to clone mountpoint '$opt' (type $mp->{type})\n";
+ # not full means clone instead of copy
+ die "Linked clone feature for '$volid' is not available\n"
+ if !PVE::Storage::volume_has_feature($storecfg, 'clone', $volid, $snapname, $running, {'valid_target_formats' => ['raw', 'subvol']});
}
- } elsif ($opt =~ m/^net(\d+)$/) {
- # always change MAC! address
- my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg');
- my $net = PVE::LXC::Config->parse_lxc_network($value);
- $net->{hwaddr} = PVE::Tools::random_ether_addr($dc->{mac_prefix});
- $newconf->{$opt} = PVE::LXC::Config->print_lxc_network($net);
+
+ $mountpoints->{$opt} = $mp;
+ push @$vollist, $volid;
+
} else {
- # copy everything else
- $newconf->{$opt} = $value;
+ # TODO: allow bind mounts?
+ die "unable to clone mountpoint '$opt' (type $mp->{type})\n";
}
+ } elsif ($opt =~ m/^net(\d+)$/) {
+ # always change MAC! address
+ my $dc = PVE::Cluster::cfs_read_file('datacenter.cfg');
+ my $net = PVE::LXC::Config->parse_lxc_network($value);
+ $net->{hwaddr} = PVE::Tools::random_ether_addr($dc->{mac_prefix});
+ $newconf->{$opt} = PVE::LXC::Config->print_lxc_network($net);
+ } else {
+ # copy everything else
+ $newconf->{$opt} = $value;
}
- die "can't clone CT to node '$target' (CT uses local storage)\n"
- if $target && !$sharedvm;
+ }
+ die "can't clone CT to node '$target' (CT uses local storage)\n"
+ if $target && !$sharedvm;
- # Replace the 'disk' lock with a 'create' lock.
- $newconf->{lock} = 'create';
+ # Replace the 'disk' lock with a 'create' lock.
+ $newconf->{lock} = 'create';
- delete $newconf->{snapshots};
- delete $newconf->{pending};
- delete $newconf->{template};
- if ($param->{hostname}) {
- $newconf->{hostname} = $param->{hostname};
- }
+ delete $newconf->{snapshots};
+ delete $newconf->{pending};
+ delete $newconf->{template};
+ if ($param->{hostname}) {
+ $newconf->{hostname} = $param->{hostname};
+ }
- if ($param->{description}) {
- $newconf->{description} = $param->{description};
- }
+ if ($param->{description}) {
+ $newconf->{description} = $param->{description};
+ }
+ PVE::LXC::Config->lock_config($newid, sub {
+ # read empty config, lock needs to be still here
+ my $conf = PVE::LXC::Config->load_config($newid);
+ die "Lost 'create' config lock, aborting.\n"
+ if !PVE::LXC::Config->has_lock($conf, 'create');
+ # write the actual new config now to disk
+ PVE::LXC::Config->write_config($newid, $newconf);
+ });
+ };
+ if (my $err = $@) {
+ eval { PVE::LXC::Config->remove_lock($vmid, 'disk') };
+ warn "Failed to remove source CT config lock - $@\n" if $@;
+
+ eval {
PVE::LXC::Config->lock_config($newid, sub {
- # read empty config, lock needs to be still here
my $conf = PVE::LXC::Config->load_config($newid);
die "Lost 'create' config lock, aborting.\n"
if !PVE::LXC::Config->has_lock($conf, 'create');
- # write the actual new config now to disk
- PVE::LXC::Config->write_config($newid, $newconf);
+ PVE::LXC::Config->destroy_config($newid);
});
};
- if (my $err = $@) {
- eval { PVE::LXC::Config->remove_lock($vmid, 'disk') };
- warn "Failed to remove source CT config lock - $@\n" if $@;
-
- eval {
- PVE::LXC::Config->lock_config($newid, sub {
- my $conf = PVE::LXC::Config->load_config($newid);
- die "Lost 'create' config lock, aborting.\n"
- if !PVE::LXC::Config->has_lock($conf, 'create');
- PVE::LXC::Config->destroy_config($newid);
- });
- };
- warn "Failed to remove target CT config - $@\n" if $@;
+ warn "Failed to remove target CT config - $@\n" if $@;
- die $err;
- }
- });
+ die $err;
+ }
my $update_conf = sub {
my ($key, $value) = @_;
--
2.30.2
More information about the pve-devel
mailing list