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 0F56A63F02 for ; Thu, 29 Oct 2020 09:26:42 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F1B7C26967 for ; Thu, 29 Oct 2020 09:26:11 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (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 350D72693F for ; Thu, 29 Oct 2020 09:26:11 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id EF98145F82 for ; Thu, 29 Oct 2020 09:26:10 +0100 (CET) To: pve-devel@lists.proxmox.com References: <20201015102426.5662-1-f.ebner@proxmox.com> <1603890752.4d64scs2iw.astroid@nora.none> From: Fabian Ebner Message-ID: <983c73e4-47a5-0599-7a88-3f292eb13d40@proxmox.com> Date: Thu, 29 Oct 2020 09:26:00 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: <1603890752.4d64scs2iw.astroid@nora.none> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.029 Adjusted score from AWL reputation of From: address KAM_ASCII_DIVIDERS 0.8 Spam that uses ascii formatting tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.921 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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. [migrate.pm, proxmox.com] Subject: Re: [pve-devel] [PATCH container] fix #3030: activate volumes at the right time for restart migration 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: Thu, 29 Oct 2020 08:26:42 -0000 Am 28.10.20 um 14:15 schrieb Fabian Grünbichler: > On October 15, 2020 12:24 pm, Fabian Ebner wrote: >> The lxc-pve-poststop-hook deactivates volumes when a container is stopped. >> To make sure that volumes are active when using the restart mode, >> move activate_volumes to after the conditional vm_stop. The lxc-stop command >> used in vm_stop waits for the hook script to complete, so there is no race. >> >> Signed-off-by: Fabian Ebner >> --- >> >> For VMs we don't have restart migration, so no similar bug there. >> >> An alternative would be to communicate to the hook script to >> not deactivate the volumes. That would mean writing the lock=migrate >> to the config earlier (currently it's being set in phase1) and >> then checking for the lock in the hookscript. > > isn't this still wrong, as it only activates the volumes directly > referenced by the config, but we storage migrate unused (referenced and > unreferenced) and snapshot volumes as well? wouldn't it make more sense > that storage_migrate ensures the passed-in volid is activated before > accessing it? and then before switching the container over, we ensure > all volids we passed to storage_migrate get deactivated.. the others > were already deactivated by the container shutting down anyway. > You're right, and with volumes not referenced in the config, the QEMU code is also affected by the issue. Should we still keep the current activate_volumes in prepare? I imagine one reason it's there, is that we would die early. If we move activation to within storage_migrate we'd lose that. But I can check that the volumes are not required to be active somewhere else during migration and remove the call in prepare if that's preferred. Alternatively, we could move the activate_volumes call to before the loop where we repeatedly call storage_migrate. In the long run, we might want to rework storage_migrate a bit, having it take a list of volumes instead, and separating the checks+activation and the transfer itself. With the goal of being able to die early for problems like "target storage doesn't support format" or "activation failed". >> >> src/PVE/LXC/Migrate.pm | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/src/PVE/LXC/Migrate.pm b/src/PVE/LXC/Migrate.pm >> index 90d74b4..5ef16d2 100644 >> --- a/src/PVE/LXC/Migrate.pm >> +++ b/src/PVE/LXC/Migrate.pm >> @@ -90,8 +90,6 @@ sub prepare { >> >> }); >> >> - PVE::Storage::activate_volumes($self->{storecfg}, $need_activate); >> - >> # todo: test if VM uses local resources >> >> # test ssh connection >> @@ -110,6 +108,8 @@ sub prepare { >> $running = 0; >> } >> >> + PVE::Storage::activate_volumes($self->{storecfg}, $need_activate); >> + >> return $running; >> } >> >> -- >> 2.20.1 >> >> >> >> _______________________________________________ >> pve-devel mailing list >> pve-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >> >> >> > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > >