[pbs-devel] [RFC PATCH v2 proxmox-backup{, -qemu}, pve-{qemu, manager}, qemu-server 0/7] fix #4289: Set protected flag on backup creation

[Christoph Heiss]

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