From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id EB4E0738A9 for ; Fri, 18 Jun 2021 14:52:09 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E75D527977 for ; Fri, 18 Jun 2021 14:51:39 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 17DFC2796C for ; Fri, 18 Jun 2021 14:51:39 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id E39E544219 for ; Fri, 18 Jun 2021 14:51:38 +0200 (CEST) From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= To: pve-devel@lists.proxmox.com Date: Fri, 18 Jun 2021 14:51:20 +0200 Message-Id: <20210618125123.2903137-4-f.gruenbichler@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20210618125123.2903137-1-f.gruenbichler@proxmox.com> References: <20210617105201.804786-1-o.bektas@proxmox.com> <20210618125123.2903137-1-f.gruenbichler@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.732 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [lxc.pm] Subject: [pve-devel] [PATCH container 3/6] clone_vm: reduce source flock scope X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Jun 2021 12:52:10 -0000 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 --- 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