From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <f.ebner@proxmox.com> 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 10390941E0 for <pve-devel@lists.proxmox.com>; Tue, 10 Jan 2023 13:34:53 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DB3D3A4C3 for <pve-devel@lists.proxmox.com>; Tue, 10 Jan 2023 13:34:22 +0100 (CET) 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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for <pve-devel@lists.proxmox.com>; Tue, 10 Jan 2023 13:34:19 +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 2856343634 for <pve-devel@lists.proxmox.com>; Tue, 10 Jan 2023 13:34:19 +0100 (CET) Message-ID: <cd42c9e1-f890-0b6a-b00b-5ef96f74a513@proxmox.com> Date: Tue, 10 Jan 2023 13:34:14 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Content-Language: en-US To: Christoph Heiss <c.heiss@proxmox.com> Cc: pve-devel@lists.proxmox.com References: <20230102123633.2493599-1-c.heiss@proxmox.com> <20230102123633.2493599-3-c.heiss@proxmox.com> <dff207ed-4116-2010-1be0-d3b263469ea9@proxmox.com> <20230110111141.2hxrozsr7fatvswj@maui.proxmox.com> From: Fiona Ebner <f.ebner@proxmox.com> In-Reply-To: <20230110111141.2hxrozsr7fatvswj@maui.proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.022 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 NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pve-devel] [PATCH storage] fix #4289: pbs: wait for backup verification to finish before updating volume attribute X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion <pve-devel.lists.proxmox.com> List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=unsubscribe> List-Archive: <http://lists.proxmox.com/pipermail/pve-devel/> List-Post: <mailto:pve-devel@lists.proxmox.com> List-Help: <mailto:pve-devel-request@lists.proxmox.com?subject=help> List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel>, <mailto:pve-devel-request@lists.proxmox.com?subject=subscribe> X-List-Received-Date: Tue, 10 Jan 2023 12:34:53 -0000 Am 10.01.23 um 12:11 schrieb Christoph Heiss: > On Wed, Jan 04, 2023 at 11:50:38AM +0100, Fiona Ebner wrote: >> It might not seem that big of a deal, because usually only manual >> backups use 'protected'. But by doing it in >> update_volume_attribute(), you also do it for 'notes', where it's not >> needed and which is relevant to backup jobs where the increased wait >> might be very noticeable. So at least, it should only be done for >> 'protected' if doing it in update_volume_attribute(). > That is actually the case now - updating notes takes a different path > through update_volume_notes(). > Sorry, I missed that. >> >> It would be better if the protected flag could be specified upon >> creation already. Would also fix the following race I guess: > It definitely would be a lot cleaner. I'll see what I can do and rework > the whole series. > Probably involves adding a new parameter to the `proxmox-backup-client > backup` command and API(?) AFAICS. But this would not be all that bad > of a feature for the backup client in general, I think. I think you also need to add support in QEMU (new parameter for the 'backup' QMP command) and the proxmox-backup-qemu library (to handle the parameter). Regarding the API, maybe it can be its own endpoint in the backup API (alongside endpoints like 'blob' and 'finish')? As long as we protect the backup before marking it as finished it should be good. Just an idea, not sure if it would be better. > And I guess I need to figure out a way how to detect whether the new > parameter is supported or not? If there is no straightforward way to make that information available in VZDump.pm, we could also just base the decision off of the PBS version. One way to decide if the current behavior should be used as a fallback would be to check the protected status after finishing the backup. That is slightly racy though, because something else could've already changed the protection between finishing and the check. > In case this it not supported, just keeping the current behavior (i.e. > best-effort via the API and maybe failing) is probably the sensible way. Yes, to not break existing setups. Also note that non-PBS backup storages need the current behavior too.