Skip to content

backup: keep every KVM export checkpoint and start from the one the client names - #14672

Open
nvazquez wants to merge 11 commits into
apache:mainfrom
shapeblue:424-veeam-start-checkpoint
Open

nvazquez wants to merge 11 commits into
apache:mainfrom
shapeblue:424-veeam-start-checkpoint

Conversation

@nvazquez

Copy link
Copy Markdown
Contributor

Description

This PR addresses the following issue for the Veeam KVM integration:

POST /vms/{id}/backups ignored the from_checkpoint_id it was given. The KVM backup export kept a single active checkpoint per Instance in the instance details and every backup continued from it, deleting the previous one on the host when the next backup started. Two backup clients protecting the same Instance therefore shared one chain: each backup moved it on and removed the checkpoint the other client needed, and that client's next incremental was silently served from the wrong checkpoint and missed every block changed in between. The response named the substituted checkpoint, but Veeam reads from the one it requested. DELETE /vms/{id}/checkpoints/{ckp} accepted any id and answered 200 whether or not it did anything.

Change

Checkpoints now live in a vm_checkpoint table, one row per checkpoint present on the host, and each one stays until its client deletes it. Veeam relies on this when one VM is in several jobs.

  • A backup starts from the checkpoint the client names. Any checkpoint recorded for the Instance is valid, including one libvirt forgot after a stop/start, which the host redefines. An unknown id is refused and the Veeam API answers HTTP 400, so the client falls back to a full backup instead of continuing from another checkpoint. A request without a checkpoint is a full backup, and the backup type records FULL or INCREMENTAL.
  • A finalized backup adds its checkpoint and leaves the one it started from. The checkpoint list returns every checkpoint, oldest first, with the newest marked active. There is no cap on the number kept.
  • DELETE removes exactly the named checkpoint: the libvirt checkpoint of a running Instance and the dirty bitmap of the same name in every layer of the disk images, including layers below VM snapshot overlays, and on a stopped Instance with qemu-img, with the passphrase for encrypted volumes. A checkpoint libvirt has forgotten (CloudStack domains are transient, so a stop/start forgets them all) is redefined from the disks that still carry its bitmap, both when an incremental starts from it and when it is deleted. An unknown id is 404.
  • The dirty extents of a stopped Instance's download are computed against the checkpoint the backup started from, not the Instance's newest one.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

abh1sar and others added 10 commits November 10, 2026 20:21
The checkpoint delete on the host removed the bitmap only where libvirt still
listed the checkpoint, and only from the active layer. CloudStack domains are
transient, so a restart or a migration makes libvirt forget every checkpoint
while the bitmaps stay in the images, and a VM snapshot pushes a volume that
carries bitmaps down the backing chain, where the active-layer cleanup cannot
reach them. Both left a bitmap per backup behind, recording every guest write
forever, and the second resurfaced as an orphan in the live image once the
snapshot was committed.

For a running VM all bitmap removal is now left to libvirt, which walks the
backing chain and reopens read-only backing files itself. When libvirt
refuses, because it does not know the checkpoint or a listed disk has lost its
bitmap, the agent asks qemu which disks still hold the bitmap in any layer,
redefines the checkpoint with exactly those disks and the creation time the
command carries, and deletes again; when no layer has it, only libvirt's
record is dropped. For a stopped VM every layer of each volume's backing chain
that lists the bitmap is cleaned with qemu-img; the template never carries one
and is never opened for writing. The passphrase of an encrypted volume is
passed along so its images can be opened for writing, the key file is deleted
after the call, and a layer that no longer has the bitmap does not fail the
delete, so a partial failure can be retried.
The KVM backup export kept one checkpoint per Instance in the instance
details (active.checkpoint.id, with the previous one in last.checkpoint.id
until the next backup start deleted it on the host), and POST
/vms/{id}/backups ignored the from_checkpoint_id it was given: createBackup
always continued from the active checkpoint. Two backup clients protecting
the same Instance therefore shared one chain. Each backup moved it on and
deleted the checkpoint the other client needed, and that client's next
incremental was silently served from the wrong checkpoint and missed every
block changed in between. The response named the substituted checkpoint, but
Veeam reads from the one it requested. DELETE /vms/{id}/checkpoints/{ckp}
accepted any id and did nothing unless it was the active one, answering HTTP
200 either way. oVirt keeps every checkpoint until the client deletes it and
VMware's change tracking is per consumer; Veeam relies on that when the same
VM is in several jobs.

Checkpoints now live in a vm_checkpoint table, one row per checkpoint present
on the host. A backup starts from the checkpoint the client names, which may
be any checkpoint of the Instance; an unknown id is refused with "Checkpoint
... doesn't exist", which the Veeam API answers with HTTP 400 so the client
falls back to a full backup, as the oVirt engine does. A request without a
checkpoint is a full backup even when checkpoints exist, and the backup type
records FULL or INCREMENTAL. A finalized backup adds its checkpoint and leaves
the one it started from. DELETE removes the named checkpoint on the host and
in the table, 404 for any other id; a stopped Instance's delete carries the
passphrases of its encrypted volumes and the checkpoint's creation time. The
list returns every checkpoint, oldest first, the newest marked active. There
is no limit on the number kept: one that evicts the oldest regardless of
whose chain it belongs to would turn the least frequent job's backups into
fulls. Downloads of a stopped Instance export the bitmap of the checkpoint the
backup started from rather than the active one, since the two no longer
match.

The schema upgrade files are not changed. The table, and the move of the old
instance details into it (the previous checkpoint first, then the active one,
then the four detail rows deleted), are applied by hand with the SQL given to
each deployment.

Fixes #51
…unknown Instance

A finalize that failed after the backup job had been stopped on the host left
the backup in FinalizingImageTransfer, and the wait job on the Instance's work
queue kept every other operation on the Instance waiting until it timed out.
Any failure after the stop now marks the backup Error, which ends the wait
job, and the client retries with a full backup. A finalize that arrives before
the start job has run is refused instead of stopping nothing and recording a
checkpoint that does not exist.

Checkpoints are listed in the order they were recorded rather than by the
host's creation time, which is not comparable between hosts. Deleting a
checkpoint of an unknown Instance answers 404 like an unknown checkpoint.
…ts bitmap

CloudStack domains are transient, so a stop/start makes libvirt forget every checkpoint while the bitmaps stay in the images. The start wrapper redefined a forgotten from-checkpoint from its name and creation time only, so libvirt's record listed no disks, and a later delete of that checkpoint succeeded in libvirt without removing a single bitmap. The start wrapper now asks qemu which disks carry the bitmap and redefines the checkpoint with exactly those disks, so libvirt's own delete removes them later. When no disk has the bitmap any more the checkpoint is not redefined and the backup is a full one, as before.
…heckpoint id to the host

A new backup is rejected while any backup of the Instance is Queued, BackingUp,
ReadyForImageTransfer or FinalizingImageTransfer; the old check looked at one
arbitrary row for BackingUp, a state export backups never use, so a second
POST failed later on the host with a less helpful error. The from checkpoint
sent to the host is the recorded id rather than the client's string, which the
database matches regardless of case and trailing spaces. On the agent, disks
are matched to qcow2 block nodes only instead of relying on node order, text
before the JSON of qemu-img info is skipped, and the start wrapper closes the
key file of an encrypted disk. Unused checkpoint detail constants and stale
documentation are removed.
…name

qemu reports an encrypted qcow2 opened by libvirt under a json:{...}
pseudo-filename that carries the key secret, in query-block as well as
query-named-block-nodes. The start wrapper compared that string with the
disk path, found no bitmap for the from checkpoint and silently took a full
backup of every encrypted disk; the delete wrapper found no disk with the
bitmap and answered that the checkpoint was not present, removing the row
and leaving the bitmap in the image. Both now unwrap the pseudo-filename to
the innermost file name before matching.
The Veeam API dispatched the finalize as an asynchronous job and answered 200 with the backup as it was, so a finalize of a backup that had not started, or was already being finalized, failed inside the job where the client could not see it. The backup's status is now checked before the job is dispatched and anything but ReadyForImageTransfer is answered with 400.
A QMP reply without a node list, such as an error object, was read as "no disk has the bitmap" and a dumpxml failure of any kind as "libvirt does not know the checkpoint", so the delete could answer success, and the row be removed, while bitmaps or libvirt metadata were still there. Both now fail the delete so that it is retried.
…d, record a checkpoint once

When stopping an open image transfer failed during the finalize, the backup stayed in FinalizingImageTransfer, where every retry is refused and every new backup of the Instance as well. Nothing has been stopped on the host at that point, so the backup now goes back to ReadyForImageTransfer and the finalize can be retried. A checkpoint that is already recorded for the Instance is not recorded a second time.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@nvazquez

Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@codecov

codecov Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 73.42466% with 97 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.86%. Comparing base (a7be64a) to head (ebc8f35).

Files with missing lines Patch % Lines
...a/org/apache/cloudstack/backup/VmCheckpointVO.java 30.30% 23 Missing ⚠️
...che/cloudstack/backup/dao/VmCheckpointDaoImpl.java 0.00% 21 Missing ⚠️
...apper/LibvirtDeleteVmCheckpointCommandWrapper.java 91.77% 5 Missing and 8 partials ⚠️
.../cloudstack/backup/KVMBackupExportServiceImpl.java 87.17% 5 Missing and 5 partials ⚠️
...urce/wrapper/LibvirtStartBackupCommandWrapper.java 61.90% 8 Missing ⚠️
...he/cloudstack/api/response/CheckpointResponse.java 33.33% 6 Missing ⚠️
...e/cloudstack/backup/DeleteVmCheckpointCommand.java 45.45% 6 Missing ⚠️
...pache/cloudstack/utils/qemu/QemuImageFilename.java 73.33% 2 Missing and 2 partials ⚠️
...stack/api/command/admin/backup/StartBackupCmd.java 0.00% 3 Missing ⚠️
...apache/cloudstack/veeam/adapter/ServerAdapter.java 78.57% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #14672      +/-   ##
============================================
+ Coverage     20.80%   20.86%   +0.05%     
- Complexity    21945    22028      +83     
============================================
  Files          6496     6499       +3     
  Lines        591838   592068     +230     
  Branches      72617    72658      +41     
============================================
+ Hits         123147   123541     +394     
+ Misses       455315   455128     -187     
- Partials      13376    13399      +23     
Flag Coverage Δ
uitests 3.88% <ø> (ø)
unittests 22.18% <73.42%> (+0.06%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI balanced review requested due to automatic review settings November 10, 2026 23:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 19983

@nvazquez

Copy link
Copy Markdown
Contributor Author

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@nvazquez a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19984

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants