From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 8512A1FF142 for ; Fri, 03 Jul 2026 13:44:55 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 955DB2139D; Fri, 03 Jul 2026 13:44:54 +0200 (CEST) Message-ID: <1536da22-7e3b-41b9-b7f4-b9c7acfa80cf@proxmox.com> Date: Fri, 3 Jul 2026 13:44:45 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server 3/3] api: update vm: fork before locking To: Dominik Csapak , pve-devel@lists.proxmox.com References: <20260629124625.115457-1-f.ebner@proxmox.com> <20260629124625.115457-4-f.ebner@proxmox.com> <4d763f1f-59e0-4ad5-b7c1-72b4e5b65dd1@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <4d763f1f-59e0-4ad5-b7c1-72b4e5b65dd1@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1783079081993 X-SPAM-LEVEL: Spam detection results: 0 DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: VZBUS5ROGYVYOZLVKZVZG7FEA6ZGUG6A X-Message-ID-Hash: VZBUS5ROGYVYOZLVKZVZG7FEA6ZGUG6A X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 03.07.26 um 11:41 AM schrieb Dominik Csapak: > one remark/question: > > would it make for a nicer interface for the load_and_check_config > if we'd pass the delete list into it instead of using > catching the outer closure variable? > > that way we wouldn't have to guard the 'push @delete' statements > and could simply pass an empty list into it when we don't want > to add them. > > the only difference it would make is that inside, we wouldn't > delete the $conf->{lock} there but since we discard that > before using it when we don't want to update delete this > should be fine? As discussed during lunch too, in case the vmstate is being deleted, it's important that the load_and_check_config() gets the actual $delete list. Because if we pass an empty list, then we don't delete the suspended lock, and then the check_lock() right below would fail. Of course, this could be handled by adapting the function with additional arguments, but the guard seems rather straightforward to me, so I would be inclined to just keep it like that.