GeezerVison

Copy link

Copy Markdown

Contributor

@@            Coverage Diff            @@
##               4.22   #14006   +/-   ##
=========================================
  Coverage     17.86%   17.86%           
- Complexity    16037    16039    +2     
=========================================
  Files          5928     5928           
  Lines        534479   534496   +17     
  Branches      65410    65412    +2     
=========================================
+ Hits          95468    95479   +11     
- Misses       428173   428179    +6     
  Partials      10838    10838           

abh1sar requested review from shwstppr and weizhouapache and a lite review from Copilot August 29, 2026 16:15

Copilot started reviewing on behalf of abh1sar August 29, 2026 16:16

View session

Copilot AI reviewed Aug 29, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

shwstppr approved these changes Sep 9, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

shwstppr approved these changes Sep 14, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

weizhouapache approved these changes Sep 14, 2026

View reviewed changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

25 of 26 checks passed

Conflicts:
- LibvirtRestoreBackupCommandWrapperTest.java: apache#14006 appended three mount/unmount
  timeout tests at the end of the class, where the volume mapping tests end too;
  kept both.

…eouts

apache#14006 runs the mount, the umount and the rsync of a restore through
Script.executeCommandForExitValue(long, String...) so that they honour the
configured timeouts. The volume mapping tests still stubbed and verified the
overloads without a timeout. The two tests asserting where a backup is written
therefore failed, and the checks that a backup is never written into the wrong
volume passed without verifying anything, as nothing called that overload any
more. Stub and verify the timeout variants instead, as apache#14006 does for the
existing tests.

DaanHoogland deleted the nas-restore-timeout branch September 24, 2026 10:27

Resolves conflicts with the incremental NAS backup backport (apache#13877) and the
restore mount/unmount timeout fix (apache#14006). Both sets of zone settings and
command fields are kept; the agent wrappers carry both the mount timeout and
the LUKS key file, and the enhancement flags plus temporary passphrase file
move into upstream's new runBackupScript.

Compression and LUKS encryption rewrite each backup file with qemu-img
convert, which would flatten an incremental through its backing chain (or
leave an encrypted child whose parent needs its own secret). So a zone with
either enabled now takes full backups (decideChain returns legacy-full), and
nasbackup.sh refuses incremental mode combined with -c/-e as a backstop.
Covered by decideChainReturnsLegacyFullWhenCompressionOrEncryptionEnabled.

Claude-Session: https://claude.ai/code/session_011MtPDN3Yg3DR8wu4EN8nkp

Since apache#14006 mount/umount also go through Script.executeCommandForExitValue,
so the LUKS restore tests' call capture picked them up (4 calls, not 2).
Filter the capture to qemu-img command lines; the no-passphrase test now
asserts no qemu-img command ran instead of no command at all.

Claude-Session: https://claude.ai/code/session_011MtPDN3Yg3DR8wu4EN8nkp

…storeBackupCommandWrapper (apache#14006)

* kvm: detect mount failures and honour the configured timeouts on backup restore

Script.executeCommand returns null when the command fails, it does not throw,
so the try/catch around the mount and umount of the backup repository could
never fire and the return value was discarded. A repository that fails to mount
was therefore treated as mounted, and the restore carried on against an empty
directory until it failed later with a misleading "backup file not found". A
failed umount was ignored the same way, leaking the mount. Both now go through
executeCommandForExitValue and check the exit value.

The same refactor also dropped the timeouts. mountTimeout was still passed into
mountBackupDirectory but never used, and the rsync of the volume lost the
command timeout, so both fell back to the one hour default in Script instead of
the configured values. An unresponsive repository could hold a restore up for
an hour rather than failing after nas.backup.restore.mount.timeout seconds.

* kvm: clean up after a failed mount and bound the unmount of a backup repository

The directory created for the mount is removed by the caller in a finally block,
but that block is only reached once the mount has succeeded, so a repository
that cannot be mounted left an empty directory behind on every attempt. It is
now removed before the failure is reported, best effort and logged if it cannot
be.

The unmount ran without a timeout and so fell back to the one hour default in
Script. Unmounting a repository that has become unreachable blocks as easily as
mounting one, and this runs in the cleanup path of a restore, so it is now
bounded by the configured mount timeout like the mount itself.

* origin/main: (156 commits)
  Upgrade Netris SDK version to 1.1.0 since 24.0 (apache#14365)
  feature: instance boot group and readiness rules (apache#13907)
  dont show unathorized notification for prelogin (apache#14054)
  .github: Upgrade docker/login-action to fix ASF Allowlist failure (apache#14276)
  virtual router: Add route-maps to BGP peers for Routed Mode (apache#9964)
  Add pre-commit hook to validate dependabot.yml (apache#14107)
  Add the ASF Allowlist check workflow (apache#14113)
  Revert "VMware to KVM: cold (VDDK) and warm (CBT) migration into Ceph/RBD and Linstor, plus importVm adoption of existing RBD root volumes and Linstor root/data volumes (apache#13656)"
  VMware to KVM: cold (VDDK) and warm (CBT) migration into Ceph/RBD and Linstor, plus importVm adoption of existing RBD root volumes and Linstor root/data volumes (apache#13656)
  engine-schema: upgrade path for 24.0.0 (apache#14033)
  xenserver/xcpng: do not bypass secondary storage when copy volumes between pools (apache#13750)
  backport: Incremental NAS backup support for KVM (apache#13074) (apache#13877)
  storage: KVM - enable RBD/Ceph volume encryption support (apache#13556)
  backup: remove the powered-off precondition for all backup providers (apache#14094)
  Nas backup: Fix mount/unmount error handling and timeout in LibvirtRestoreBackupCommandWrapper (apache#14006)
  kvm: apply rbd_default_data_pool when creating volumes from templates on RBD (apache#13361)
  server: copy template details to vm instance details when import VM (apache#12793)
  marvin: use pycryptodome instead of PyCrypt (apache#13594)
  Packaging: accept normalized Marvin archive names (apache#13793)
  SharedFS: fix NPE when accessing systemvm templates for unsupported hypervisors (apache#13930)
  ...