Skip to content

veeam-kvm: fix issues; improvements - #14593

Draft
shwstppr wants to merge 9 commits into
apache:mainfrom
shapeblue:fix-addl-veeam
Draft

shwstppr wants to merge 9 commits into
apache:mainfrom
shapeblue:fix-addl-veeam

Conversation

@shwstppr

@shwstppr shwstppr commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Description

This PR brings a set of fixes and improvements for the Veeam KVM integration (veeam-control-service, the image server and KVM backup), found while validating it with Veeam.

veeam-control-service: request parameters and oVirt-compatible responses

  • async and detach_only are parsed the way oVirt does: true/false (any case) or 1/0. A parameter given without a value (e.g. ?async) means true.
  • detach_only defaults to what oVirt uses for each endpoint:
    • DELETE /api/vms/{id}: data disks are deleted unless detach_only is set.
    • DELETE /api/vms/{id}/diskattachments/{id}: the disk is only detached unless detach_only=false.
  • Synchronous requests no longer wait forever for their CloudStack job. A new dynamic setting, integration.veeam.control.job.wait.timeout, sets the limit (seconds, default 3600; a value of 0 or less falls back to the default). When the limit is reached, the request stops waiting and the job keeps running in the background.
  • Jobs that end FAILED or CANCELLED are reported as errors. Before, they were returned as successes, and a cancelled job caused an endless wait. Error messages include the job ID, the elapsed time and CloudStack's error text.
  • Responses:
    • DELETE (VM, snapshot, disk, disk attachment, checkpoint, NIC):
      • sync: 200 with status: complete, plus the job link when there is a job;
      • async: 202, with the job link and the job's status when there is a job, otherwise an empty body;
      • failed job: 400 fault with reason Operation Failed;
      • wait timed out or interrupted: 504 with the same fault, since the job may still be running.
    • Actions (start, stop, shutdown, snapshot revert):
      • async: 202, with the status taken from the job (pending while it runs);
      • failed job: 400 action with status: failed, a fault and the job link;
      • wait timed out: 504 with the same body.
  • Added GET and DELETE /api/vms/{vmId}/nics/{nicId}. A NIC that doesn't exist or belongs to another VM returns 404, a NIC CloudStack refuses to remove (e.g. the default NIC) returns 400, and any other method returns 405. Added setVmId/setNicId to RemoveNicFromVMCmd.
  • /api/vms/{id} and its sub-resources return 404 when the VM doesn't exist.
  • GET /api/vms lists each VM once. Before, a VM with several disks, NICs or tags was listed once per view row, and paging counted rows instead of VMs.
  • Added GET /api/hosts?follow=statistics.

Backup restore with a template that can no longer be used

  • When restoring an instance backup, if the original template was deleted, is inactive, isn't available in the target zone, or the owner can't use it, the restore uses the default blank KVM template instead of failing.
  • Root disk size validation is skipped for the blank template, which fixes an NPE in UserVmManagerImpl.validateRootDiskResize.

Image server: dirty-extents merge

  • Allocation and dirty extents are now merged in a single linear pass, one 64 MiB block-status step at a time, keeping only the joined result, as ovirt-imageio does. A short or missing block-status reply can no longer hide changed data.
  • Worst case (extents changing every 64 KiB): a 2 GiB disk went from 594 s to 0.4 s, and a 128 GiB disk peaks at 1.7 GiB of memory (ovirt-imageio: 1.8 GiB).

KVM: backup scratch files

  • Each disk's scratch file is created next to the disk on its primary storage (backup-scratch-<volume uuid>.qcow2) instead of in /var/tmp on the host.
  • Before a backup starts, it is refused unless each scratch directory has room for the worst case: the virtual size of its disks plus 1%. The error names the directory, the bytes needed and the bytes available.
  • A scratch file left behind by an earlier backup is removed, but only when the VM has no active libvirt job.

Backup: image transfers left open by an earlier backup

  • When a backup starts, or requests a transfer for a volume that still has a download open from an earlier backup, that download is finalized first and its qemu-nbd stopped. Before, the volume stayed locked, or every later request failed with "Image transfer already exists for volume".
  • If the old transfer can't be ended (e.g. its host is unreachable), the request is refused with the transfer and host named, and the next attempt retries the cleanup.

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?

@codecov

codecov Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.90761% with 89 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.87%. Comparing base (2a4d7d9) to head (4fc089b).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...urce/wrapper/LibvirtStartBackupCommandWrapper.java 64.06% 21 Missing and 2 partials ⚠️
...apache/cloudstack/veeam/adapter/ServerAdapter.java 89.10% 11 Missing and 6 partials ⚠️
...oudstack/veeam/api/StorageDomainsRouteHandler.java 66.66% 14 Missing and 2 partials ⚠️
...apache/cloudstack/veeam/api/HostsRouteHandler.java 79.48% 6 Missing and 2 partials ⚠️
...g/apache/cloudstack/veeam/api/VmsRouteHandler.java 95.74% 5 Missing and 1 partial ⚠️
...am/api/converter/AsyncJobJoinVOToJobConverter.java 76.92% 2 Missing and 4 partials ⚠️
...ava/com/cloud/api/query/dao/UserVmJoinDaoImpl.java 66.66% 4 Missing ⚠️
.../cloudstack/backup/KVMBackupExportServiceImpl.java 87.87% 0 Missing and 4 partials ⚠️
...dstack/api/command/user/vm/RemoveNicFromVMCmd.java 66.66% 2 Missing ⚠️
...udstack/veeam/adapter/AsyncJobFailedException.java 85.71% 0 Missing and 1 partial ⚠️
... and 2 more
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #14593      +/-   ##
============================================
+ Coverage     20.77%   20.87%   +0.09%     
- Complexity    21930    22122     +192     
============================================
  Files          6496     6500       +4     
  Lines        591656   592298     +642     
  Branches      72592    72667      +75     
============================================
+ Hits         122946   123638     +692     
+ Misses       455366   455299      -67     
- Partials      13344    13361      +17     
Flag Coverage Δ
uitests 3.88% <ø> (ø)
unittests 22.19% <87.90%> (+0.10%) ⬆️

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants