FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Convert command's timeout for snapshots commands by erikbocks · Pull Request #13210 · apache/cloudstack · GitHub

  1. apache
  2. cloudstack

Repository navigation

Convert command's timeout for snapshots commands - #13210

Merged
winterhazel merged 3 commits into
apache:4.20from
scclouds:fix-snapshot-timeout
Jun 1, 2026
ConversationCommits (3)ChecksFiles changed
Merged

Convert command's timeout for snapshots commands#13210
winterhazel merged 3 commits into
apache:4.20from
scclouds:fix-snapshot-timeout

Conversation

erikbocks commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Description

The #9659 PR introduced the commands.timeout global configuration for granular command timeout definition. If a operator wishes to increase snapshot related timeouts, he could increase the CreateObjectCommand timeout in the commands.timeout configuration. The defined timeouts are set in seconds.

However, normal and incremental snapshots creation flows use qemu-img script to execute some of the necessary operations. These scripts accept timeouts as milliseconds, but receive them as seconds from the CreateObjectCommand. This leads to incorrect timeouts.

Therefore, this PR converts the CreateObjectCommand seconds to milliseconds before passing them to qemu-img scripts.

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

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

First, I set commands.timeout configuration value to CreateObjectCommand=300. This defines this command timeout to 5 minutes. Then, I tried to create a full volume snapshot. The process failed due to a timeout.

Command timeout log 
2026-05-21 15:04:44,122 DEBUG [c.c.a.m.ClusteredAgentManagerImpl] (Work-Job-Executor-13:[ctx-fe95241a, job-77/job-78, ctx-11fcb589]) (logid:d462b71a) Wait time setting on org.apache.cloudstack.storage.command.CreateObjectCommand is 300 seconds
Failure log 
2026-05-21 15:04:56,776 DEBUG [utils.script.Script] (AgentRequest-Handler-1:[]) (logid:d462b71a) Executing command [qemu-img convert -O qcow2 -U --image-opts driver=qcow2,file.filename=/mnt/4244ccc6-8f06-3f9c-87bc-a7ba8c1caae9/5331f95a-ec63-4a79-a28f-7642ed095875 /mnt/507fca2c-a424-3ffc-b5f6-f7fe9e7c17e7/snapshots/2/4/2ad278b7-e1fa-4331-ae36-4ba67db4097b ].
2026-05-21 15:04:57,080 WARN  [utils.script.Script] (AgentRequest-Handler-1:[]) (logid:d462b71a) Process [14516] for command [qemu-img convert -O qcow2 -U --image-opts driver=qcow2,file.filename=/mnt/4244ccc6-8f06-3f9c-87bc-a7ba8c1caae9/5331f95a-ec63-4a79-a28f-7642ed095875 /mnt/507fca2c-a424-3ffc-b5f6-f7fe9e7c17e7/snapshots/2/4/2ad278b7-e1fa-4331-ae36-4ba67db4097b ] timed out. Output is [].

I installed the packages with the timeout conversion to my local environment, then tried to create another snapshot. Using a debug breakpoint, I validated that the qemu-img script instance had the converted timeout. The new snapshot was created successfully,

When trying to create a incremental snapshot, the same error occurred. Then, I installed the packages with the necessary changes and tried to create another incremental snapshot. This process also used debug breakpoints to validate the scripts timeout.

codecov Bot commented May 21, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 16.25%. Comparing base (c267ad3) to head (6dc2f8e).
⚠️ Report is 3 commits behind head on 4.20.

Files with missing lines Patch % Lines
...java/org/apache/cloudstack/utils/qemu/QemuImg.java 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##               4.20   #13210      +/-   ##
============================================
- Coverage     16.26%   16.25%   -0.02%     
  Complexity    13433    13433              
============================================
  Files          5665     5666       +1     
  Lines        500572   500899     +327     
  Branches      60792    60884      +92     
============================================
+ Hits          81417    81419       +2     
- Misses       410047   410373     +326     
+ Partials       9108     9107       -1
Flag Coverage Δ
uitests 4.14% <ø> (-0.01%) ⬇️
unittests 17.10% <50.00%> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 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.

winterhazel approved these changes May 21, 2026
View reviewed changes

winterhazel left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Looks good (did not test).

@erikbocks does this apply for 4.20/4.22? If yes, can you rebase?

erikbocks commented May 21, 2026

Copy link
Copy Markdown
Collaborator Author

@winterhazel

This change can be applied to both 4.20 and 4.22. However, the other changes can only be applied to the 4.22 branch, as they are dependent on code introduced in #9270.

I will put the 4.22 changes to another PR, and rebase this one to the 4.20 branch.

erikbocks force-pushed the fix-snapshot-timeout branch from c60546b to efea579 Compare May 21, 2026 19:39
erikbocks changed the base branch from main to 4.20 May 21, 2026 19:39

blueorangutan commented May 21, 2026

Copy link
Copy Markdown

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

DaanHoogland commented May 22, 2026

Copy link
Copy Markdown
Contributor

could this be combined with #13212 ?

erikbocks commented May 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@DaanHoogland, the reason why the PRs are separated is explained at #13210 (comment) and at #13212 PR's description. As the #13212 changes are dependent on changes introduced after 4.21, but the change in this PR is preset at both branches, it was better to separate them to include this fix in the next 4.20 minor.

DaanHoogland commented May 26, 2026

Copy link
Copy Markdown
Contributor

@blueorangutan test

DaanHoogland added this to the 4.20.4 milestone May 26, 2026

blueorangutan commented May 26, 2026

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

blueorangutan commented May 26, 2026

Copy link
Copy Markdown

[SF] Trillian Build Failed (tid-16174)

winterhazel commented May 28, 2026

Copy link
Copy Markdown
Member

@blueorangutan package

blueorangutan commented May 28, 2026

Copy link
Copy Markdown

@winterhazel a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan commented May 28, 2026

Copy link
Copy Markdown

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

DaanHoogland commented May 28, 2026

Copy link
Copy Markdown
Contributor

@blueorangutan test

blueorangutan commented May 28, 2026

Copy link
Copy Markdown

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

erikbocks force-pushed the fix-snapshot-timeout branch from 56907a0 to 6dc2f8e Compare May 28, 2026 14:25

erikbocks commented May 28, 2026

Copy link
Copy Markdown
Collaborator Author

@DaanHoogland @winterhazel

I validated that QemuImg's behavior using longs was kept by taking a snapshot with the timeout configured as 2147483 (the closest number to Java's integer overflow). During the tests, I tried to set my snapshot timeout to 2200000 (to cause an Integer overflow), and observed that the command failed to be sent because they use integers internally. Not related to the PR, but pointing it out.

blueorangutan commented May 28, 2026

Copy link
Copy Markdown

[SF] Trillian test result (tid-16208)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 51092 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr13210-t16208-kvm-ol8.zip
Smoke tests completed. 141 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

DaanHoogland approved these changes May 29, 2026
View reviewed changes

DaanHoogland 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

clgtm

winterhazel commented Jun 1, 2026

Copy link
Copy Markdown
Member

Tested manually. The timeout is being defined correctly now.

winterhazel merged commit a3970bb into apache:4.20 Jun 1, 2026
48 of 50 checks passed
This was referenced Jun 1, 2026
Closed
Closed
Closed
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Sep 11, 2026
Fix convert command's timeout for snapshot commands (apache#13210)
6509ba3 
(cherry picked from commit a3970bb)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

winterhazel winterhazel approved these changes
DaanHoogland DaanHoogland approved these changes

Assignees

No one assigned

Labels

component:kvm

Projects

None yet

Milestone

4.20.4

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL