[pbs-devel] [RFC PATCH proxmox-backup{, -qemu}, pve-{qemu, manager}, qemu-server 0/6] 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' 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" 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" 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.

Prior art
---------
A prior, very naive approach to solve this was to simply wait for the
verification to finish. Due to multiple problems and the following
discussion [0], I started from scratch to implement it properly.

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. It defaults to
unprotected as before.

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.

Further, this also introduces an (unfortunately) *API-breaking* change
to the QEMU side of things (patches 3 & 4). 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 on how to handle these
API-breaking changes, since we do not force a specific QEMU version upon
users.

Regarding patch 6: This check might introduce a time-of-check <=>
time-of-set race - could this ever be a problem?

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, 5 & 6 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/if needed.

Patches 1 & 2 are otherwise completely independent of the other patches,
as they only add the API parameter.

[0] https://lists.proxmox.com/pipermail/pve-devel/2023-January/055306.html

---
proxmox-backup:

Christoph Heiss (2):
      api2: Add `protected` parameter to `finish` endpoint
      client: Add `--protected` CLI flag to backup command

 pbs-client/src/backup_writer.rs   |  4 ++--
 proxmox-backup-client/src/main.rs | 11 ++++++++++-
 src/api2/backup/environment.rs    | 11 +++++++++++
 src/api2/backup/mod.rs            | 25 ++++++++++++++++++++-----
 4 files changed, 43 insertions(+), 8 deletions(-)

proxmox-backup-qemu:

Christoph Heiss (1):
      api: Supply `protected` parameter to the `finish` API call

 current-api.h   | 3 ++-
 simpletest.c    | 2 +-
 src/backup.rs   | 3 ++-
 src/commands.rs | 5 ++++-
 src/lib.rs      | 5 ++++-
 5 files changed, 13 insertions(+), 5 deletions(-)

pve-qemu:

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(+)

qemu-server:

Christoph Heiss (1):
      vzdump: Set protected flag on backup start if supported

 PVE/VZDump/QemuServer.pm | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

pve-manager:

Christoph Heiss (1):
      vzdump: Only set protected attribute if not already done

 PVE/VZDump.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
2.34.1