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 018A3968B4 for ; Wed, 25 Jan 2023 13:20:00 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D563CF9BE for ; Wed, 25 Jan 2023 13:19:29 +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)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Wed, 25 Jan 2023 13:19:28 +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 22C7446170 for ; Wed, 25 Jan 2023 13:19:28 +0100 (CET) From: Christoph Heiss To: pbs-devel@lists.proxmox.com Date: Wed, 25 Jan 2023 13:18:55 +0100 Message-Id: <20230125121902.404950-1-c.heiss@proxmox.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 2.513 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 RCVD_IN_DNSWL_HI -5 Sender listed at https://www.dnswl.org/, high 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. [benchmark.rs, upload-speed.rs, mod.rs, vzdump.pm, proxmox.com, qemuserver.pm, main.rs, backup.rs, lib.rs, commands.rs] Subject: [pbs-devel] [RFC PATCH v2 proxmox-backup{, -qemu}, pve-{qemu, manager}, qemu-server 0/7] fix #4289: Set protected flag on backup creation X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 25 Jan 2023 12:20:00 -0000 TL;DR ----- This fixes '#4289: Making protected backups from Web UI with "Verify New" in datastore enabled fails' [0] by introducing a new API parameter (and corresponding CLI switches) for setting a backup as protected before finishing the backup job. The problem ----------- When a datastore has the "Verify New Snapshots" flag set and a backup with the protected flag set is created (e.g. using `vzdump --protected 1 ..`), the job might fail in the final stages due to a locking race. The "Verify New Snapshots" flag means that backups are immediately locked for verification as soon as their transfer is finished and the `/finish` endpoint is called. If vzdump then tries to set the `protected` flag on that backup, it can fail due to being currently locked by the verification task. The solution ------------ This adds a new parameter `protected` to the `/finish` endpoint of the backup API. If this parameter is set to `true`, the backup will be set as protected before finishing and unlocking it. To be able to use this new parameter in a backwards-compatible way with older servers, clients need to know whether this new parameter is supported by the server or not, to not fail the schema verification. Thus introduce a "server features" discovery mechanism (patch 1). The `/version` endpoint gets a new field `features`, which is an array of strings indicating what extra "features" are supported by the client, as generally proposed by Fabian in [1]. This is new as of v2. The `proxmox-backup-client` gets a new CLI switch `--protected`, in the same manner as `vzdump`. This simply determines the value of the `protected` API parameter, or falling back to the separate API endpoint for older servers, on a "best-effort" basis (as this can still fail the exact same way as #4289 describes). Further, this also introduces an (unfortunately) *API-breaking* change to the QEMU side of things (patches 4 & 5). This is needed so that this works with disk backups of VMs. If there is some better way, I'd be happy to iterate on it. Fiona already hinted me at the problem of API-breaking changes in [2], since we do not force a specific QEMU version upon users. What's the best way to go about this? Notes ----- I marked this whole series RFC, to generally get feedback on the overall approach and further guidance on how such API/ABI-breaking things need to be handled. Also, although patches 4-7 are PVE-specific, I opted to send the whole series to pbs-devel for this spin, to ease reviewing the whole series. Later on I can split these out for pve-devel as soon as the Rust/API-side is merged. Patches 1-3 are otherwise completely independent of the other patches, as they only add the API parameter and client functionality on the Rust side. [0] https://bugzilla.proxmox.com/show_bug.cgi?id=4289 [1] https://lists.proxmox.com/pipermail/pbs-devel/2023-January/005867.html [2] https://lists.proxmox.com/pipermail/pve-devel/2023-January/055394.html Series history -------------- Changes are documented indivdual per patch. v1: https://lists.proxmox.com/pipermail/pbs-devel/2023-January/005865.html --- Christoph Heiss (3): api2: Introduce server features discovery mechanism api2: Add `protected` parameter to `finish` endpoint client: Add `--protected` CLI flag to backup command examples/upload-speed.rs | 2 +- pbs-client/src/backup_writer.rs | 6 +-- pbs-client/src/tools/mod.rs | 25 +++++++++++- proxmox-backup-client/src/benchmark.rs | 2 +- proxmox-backup-client/src/main.rs | 72 +++++++++++++++++++++++++++------- 5 files changed, 86 insertions(+), 21 deletions(-) Christoph Heiss (1): api: Supply `protected` parameter to the `finish` API call current-api.h | 3 ++- simpletest.c | 2 +- src/backup.rs | 24 ++++++++++++++++++++---- src/commands.rs | 10 +++++++++- src/lib.rs | 5 ++++- 5 files changed, 36 insertions(+), 8 deletions(-) Christoph Heiss (1): pve: Add patch to support new proxmox-backup-qemu API ...Add-support-for-protected-flag-to-proxmox.patch | 150 +++++++++++++++++++++ debian/patches/series | 1 + 2 files changed, 151 insertions(+) Christoph Heiss (1): vzdump: Only set protected attribute if not already done PVE/VZDump.pm | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Christoph Heiss (1): vzdump: Set protected flag on backup start if supported PVE/VZDump/QemuServer.pm | 14 ++++++++++++++ 1 file changed, 14 insertions(+) -- 2.34.1