public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Christoph Heiss <c.heiss@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [RFC PATCH v2 proxmox-backup{, -qemu}, pve-{qemu, manager}, qemu-server 0/7] fix #4289: Set protected flag on backup creation
Date: Wed, 25 Jan 2023 13:18:55 +0100	[thread overview]
Message-ID: <20230125121902.404950-1-c.heiss@proxmox.com> (raw)

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





             reply	other threads:[~2023-01-25 12:20 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-25 12:18 Christoph Heiss [this message]
2023-01-25 12:18 ` [pbs-devel] [RFC PATCH v2 proxmox-backup 1/7] api2: Introduce server features discovery mechanism Christoph Heiss
2023-01-25 15:56   ` Thomas Lamprecht
2023-01-26 12:33     ` Christoph Heiss
2023-01-25 12:18 ` [pbs-devel] [RFC PATCH v2 proxmox-backup 2/7] api2: Add `protected` parameter to `finish` endpoint Christoph Heiss
2023-01-25 12:18 ` [pbs-devel] [RFC PATCH v2 proxmox-backup 3/3] client: Add `--protected` CLI flag to backup command Christoph Heiss
2023-01-25 12:18 ` [pbs-devel] [RFC PATCH v2 proxmox-backup-qemu 4/7] api: Supply `protected` parameter to the `finish` API call Christoph Heiss
2023-01-25 12:19 ` [pbs-devel] [RFC PATCH v2 pve-qemu 5/7] pve: Add patch to support new proxmox-backup-qemu API Christoph Heiss
2023-01-25 12:19 ` [pbs-devel] [RFC PATCH v2 qemu-server 6/7] vzdump: Set protected flag on backup start if supported Christoph Heiss
2023-01-25 12:19 ` [pbs-devel] [RFC PATCH v2 pve-manager 7/7] vzdump: Only set protected attribute if not already done Christoph Heiss
2023-01-25 16:00 ` [pbs-devel] [RFC PATCH v2 proxmox-backup{, -qemu}, pve-{qemu, manager}, qemu-server 0/7] fix #4289: Set protected flag on backup creation Thomas Lamprecht
2023-01-26 12:44   ` Christoph Heiss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230125121902.404950-1-c.heiss@proxmox.com \
    --to=c.heiss@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal