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 F4202738E2 for ; Fri, 18 Jun 2021 14:52:15 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EB4A0279D9 for ; Fri, 18 Jun 2021 14:51:45 +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 41324279CD for ; Fri, 18 Jun 2021 14:51:45 +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 18A6844207 for ; Fri, 18 Jun 2021 14:51:45 +0200 (CEST) From: =?UTF-8?q?Fabian=20Gr=C3=BCnbichler?= To: pve-devel@lists.proxmox.com Date: Fri, 18 Jun 2021 14:51:22 +0200 Message-Id: <20210618125123.2903137-6-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.718 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 5/6] clone_vm: refactor locking further 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:16 -0000 introduce a new helper handling - obtaining the flock - (re)loading the config - checking that the 'create' lock is still there before calling a passed-in sub with the current config, since this pattern was used quite a lot here. intentionally changed behaviour: - flock is now held for the post_clone hook call - failure to remove the 'create' lock or to move the config to the target node if applicable will not undo the clone, since either is trivially fixable ('pct unlock' or a no-op migration), and copying all those volumes might have been quite expensive.. Signed-off-by: Fabian Grünbichler --- there are probably quite a few places in the container/qemu-server code where we should employ this or a similar mechanism with digest tracking to be 100% on the safe side.. src/PVE/API2/LXC.pm | 76 ++++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 35 deletions(-) diff --git a/src/PVE/API2/LXC.pm b/src/PVE/API2/LXC.pm index 5406923..4877dd9 100644 --- a/src/PVE/API2/LXC.pm +++ b/src/PVE/API2/LXC.pm @@ -1395,6 +1395,16 @@ __PACKAGE__->register_method({ PVE::LXC::Config->create_and_lock_config($newid, 0); + my $lock_and_reload = sub { + my ($vmid, $code) = @_; + return PVE::LXC::Config->lock_config($vmid, sub { + my $conf = PVE::LXC::Config->load_config($vmid); + die "Lost 'create' config lock, aborting.\n" + if !PVE::LXC::Config->has_lock($conf, 'create'); + + return $code->($conf); + }); + }; my $src_conf = PVE::LXC::Config->set_lock($vmid, 'disk'); @@ -1481,12 +1491,7 @@ __PACKAGE__->register_method({ $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 + $lock_and_reload->($newid, sub { PVE::LXC::Config->write_config($newid, $newconf); }); }; @@ -1495,10 +1500,7 @@ __PACKAGE__->register_method({ 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'); + $lock_and_reload->($newid, sub { PVE::LXC::Config->destroy_config($newid); }); }; @@ -1509,10 +1511,8 @@ __PACKAGE__->register_method({ my $update_conf = sub { my ($key, $value) = @_; - return 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'); + return $lock_and_reload->($newid, sub { + my $conf = shift; $conf->{$key} = $value; PVE::LXC::Config->write_config($newid, $conf); }); @@ -1559,23 +1559,20 @@ __PACKAGE__->register_method({ PVE::AccessControl::add_vm_to_pool($newid, $pool) if $pool; - $newconf = PVE::LXC::Config->load_config($newid); - die "Lost 'create' config lock, aborting.\n" - if !PVE::LXC::Config->has_lock($newconf, 'create'); - my $rootdir = PVE::LXC::mount_all($newid, $storecfg, $newconf, 1); - my $lxc_setup = PVE::LXC::Setup->new($newconf, $rootdir); - $lxc_setup->post_clone_hook($newconf); - PVE::LXC::umount_all($newid, $storecfg, $newconf, 1); - - PVE::LXC::Config->remove_lock($newid, 'create'); - - PVE::LXC::Config->lock_config($newid, sub { - if ($target) { - # always deactivate volumes - avoid lvm LVs to be active on several nodes - PVE::Storage::deactivate_volumes($storecfg, $vollist, $snapname) if !$running; - PVE::Storage::deactivate_volumes($storecfg, $newvollist); - - PVE::LXC::Config->move_config_to_node($newid, $target); + $lock_and_reload->($newid, sub { + my $conf = shift; + my $rootdir = PVE::LXC::mount_all($newid, $storecfg, $conf, 1); + eval { + my $lxc_setup = PVE::LXC::Setup->new($conf, $rootdir); + $lxc_setup->post_clone_hook($conf); + }; + my $err = $@; + eval { PVE::LXC::umount_all($newid, $storecfg, $conf, 1); }; + if ($err) { + warn "$@\n" if $@; + die $err; + } else { + die $@ if $@; } }); }; @@ -1594,10 +1591,7 @@ __PACKAGE__->register_method({ } 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'); + $lock_and_reload->($newid, sub { PVE::LXC::Config->destroy_config($newid); }); }; @@ -1606,6 +1600,18 @@ __PACKAGE__->register_method({ die "clone failed: $err"; } + $lock_and_reload->($newid, sub { + PVE::LXC::Config->remove_lock($newid, 'create'); + + if ($target) { + # always deactivate volumes - avoid lvm LVs to be active on several nodes + PVE::Storage::deactivate_volumes($storecfg, $vollist, $snapname) if !$running; + PVE::Storage::deactivate_volumes($storecfg, $newvollist); + + PVE::LXC::Config->move_config_to_node($newid, $target); + } + }); + PVE::Firewall::clone_vmfw_conf($vmid, $newid); return; }; -- 2.30.2