Convert command's timeout for snapshots commands#13210
Conversation
Codecov Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Looks good (did not test).
@erikbocks does this apply for 4.20/4.22? If yes, can you rebase?
|
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. |
c60546b to
efea579
Compare
|
@blueorangutan package |
|
@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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 17972 |
|
could this be combined with #13212 ? |
|
@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. |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-16174) |
|
@blueorangutan package |
|
@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. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 18068 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
56907a0 to
6dc2f8e
Compare
|
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. |
Description
The #9659 PR introduced the
commands.timeoutglobal configuration for granular command timeout definition. If a operator wishes to increase snapshot related timeouts, he could increase theCreateObjectCommandtimeout in thecommands.timeoutconfiguration. The defined timeouts are set in seconds.However, normal and incremental snapshots creation flows use
qemu-imgscript to execute some of the necessary operations. These scripts accept timeouts as milliseconds, but receive them as seconds from theCreateObjectCommand. This leads to incorrect timeouts.Therefore, this PR converts the
CreateObjectCommandseconds to milliseconds before passing them toqemu-imgscripts.Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
First, I set
commands.timeoutconfiguration value toCreateObjectCommand=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
Failure log
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-imgscript 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.