Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Comments

Block use of internal and external snapshots on KVM#11039

Merged
DaanHoogland merged 4 commits intoapache:mainfrom
scclouds:avoid-data-loss
Nov 24, 2025
Merged

Block use of internal and external snapshots on KVM#11039
DaanHoogland merged 4 commits intoapache:mainfrom
scclouds:avoid-data-loss

Conversation

@JoaoJandre
Copy link
Contributor

Description

On KVM, there are two types of snapshots: internal and external. Most snapshot/backup solutions use external snapshots on ACS; save for disk-and-memory VM snapshots, which use internal snapshots (this is a limitation with KVM, as far as I know).

However, since internal snapshots are stored inside the VM's volume (hence the name), if an internal snapshot is taken after an external snapshot and the external snapshot is restored, the internal snapshot is lost.

Thus, this PR blocks the use of disk-and-memory VM snapshots alongside volume snapshots, NAS backups, and disk-only VM snapshots (at least the ones created using the default volume snapshot implementation).

I encourage maintainers of 3rd party storage providers to test if their implementation is compatible with disk-and-memory VM snapshots, if it is not it their simultaneous usage should be blocked.

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?

I created a VM and created a few disk-and-memory VM snapshots on it; then I tried to create NAS backups, volume snapshots and disk-only VM snapshots, all of them gave me an error, which is expected.

I validated that the opposite was also true for the aforementioned cases, e.g., create volume snapshot and try to create disk-and-memory VM snapshot.

I also validated that it was possible to create multiple NAS backups, disk-only VM snapshots and volume snapshots with no issues.

@JoaoJandre
Copy link
ContributorAuthor

@slavkap@rp- I think it would be interesting to validate if the implementations done for Storpool and Linstor are compatible with disk-and-memory VM snapshots.

@codecov
Copy link

codecovbot commentedJun 16, 2025•
edited
Loading

Codecov Report

❌ Patch coverage is0% with27 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.56%. Comparing base (6dc259c) to head (3095fb1).
⚠️ Report is 1 commits behind head on main.

Files with missing linesPatch %Lines
.../storage/vmsnapshot/DefaultVMSnapshotStrategy.java0.00%15 Missing⚠️
...rg/apache/cloudstack/backup/NASBackupProvider.java0.00%5 Missing and 1 partial⚠️
...a/org/apache/cloudstack/backup/BackupProvider.java0.00%3 Missing⚠️
...tack/storage/snapshot/DefaultSnapshotStrategy.java0.00%3 Missing⚠️
Additional details and impacted files
@@            Coverage Diff            @@##               main   #11039   +/-   ##=========================================  Coverage     17.55%   17.56%- Complexity    15535    15537    +2=========================================  Files          5911     5912    +1       Lines        529359   529383   +24       Branches      64655    64660    +5     =========================================+ Hits          92949    92980   +31+ Misses       425952   425942   -10- Partials      10458    10461    +3
FlagCoverage Δ
uitests3.58% <ø> (ø)
unittests18.63% <0.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.

@JoaoJandre
Copy link
ContributorAuthor

@blueorangutan package

@blueorangutan
Copy link

@JoaoJandre 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
Copy link

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

@blueorangutan
Copy link

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

@DaanHoogland
Copy link
Contributor

@JoaoJandre :

08:35:48 [ERROR] /jenkins/workspace/acs-centos8-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.20.2.0-SNAPSHOT/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/VMSnapshotStrategyKVMTest.java:32:8: Unused import - org.apache.cloudstack.backup.dao.BackupDao. [UnusedImports]
JoaoJandre reacted with confused emoji

@JoaoJandre
Copy link
ContributorAuthor

@blueorangutan package

@blueorangutan
Copy link

@JoaoJandre 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
Copy link

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

@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland 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
Copy link

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

@rp-
Copy link
Contributor

rp- commentedJul 3, 2025

Linstor does currently not support memory snapshots (we check and throw an error if selected).
So I guess we are currently not affected by any of this?

JoaoJandre reacted with thumbs up emoji

Copy link
Contributor

@slavkapslavkap left a comment

Choose a reason for hiding this comment

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

code LGTM
I haven't tested it with NFS, but the StorPool smoke tests are executed successfully

@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

TestResultTime (s)Test File

@weizhouapache
Copy link
Member

@JoaoJandre
this seems to be included in PR#10632
do you still want it in 4.20.2 ?

@JoaoJandre
Copy link
ContributorAuthor

@JoaoJandre this seems to be included in PR#10632 do you still want it in 4.20.2 ?

@weizhouapache PR#10632 blocks the usage of the feature introduced in#10632 and other incompatible features. This PR purposefully ignores#10632 and adds restrictions to avoid other interactions between internal and external snapshots; such as volume snapshot and disk-and-memory VM snapshot.

They are complementary. When merging this PR forward, care should be taken so that the validations of both PRs do not erase one another (I can make the merge forward if needed).

@weizhouapache
Copy link
Member

weizhouapache commentedAug 28, 2025•
edited
Loading

@JoaoJandre this seems to be included in PR#10632 do you still want it in 4.20.2 ?

@weizhouapache PR#10632 blocks the usage of the feature introduced in#10632 and other incompatible features. This PR purposefully ignores#10632 and adds restrictions to avoid other interactions between internal and external snapshots; such as volume snapshot and disk-and-memory VM snapshot.

They are complementary. When merging this PR forward, care should be taken so that the validations of both PRs do not erase one another (I can make the merge forward if needed).

ok@JoaoJandre
I think the best option might be re-target this PR to 4.22 which includes#10632 , to avoid re-work.

@JoaoJandre
Copy link
ContributorAuthor

@DaanHoogland I'm pretty sure I built this PR and it worked (the github build also worked); could we try again later?

DaanHoogland reacted with thumbs up emoji

@blueorangutan
Copy link

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

JoaoJandre reacted with hooray emoji

@github-actions
Copy link

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@JoaoJandre
Copy link
ContributorAuthor

@blueorangutan package

@blueorangutan
Copy link

@JoaoJandre 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
Copy link

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

JoaoJandre reacted with confused emoji

@JoaoJandre
Copy link
ContributorAuthor

@blueorangutan package

@blueorangutan
Copy link

@JoaoJandre 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
Copy link

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

JoaoJandre reacted with confused emoji

@JoaoJandre
Copy link
ContributorAuthor

@blueorangutan package

@blueorangutan
Copy link

@JoaoJandre 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
Copy link

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

@weizhouapache
Copy link
Member

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

TestResultTime (s)Test File

@DaanHoogland
Copy link
Contributor

@JoaoJandre , are we good to go with this? cc@weizhouapache

@JoaoJandre
Copy link
ContributorAuthor

I believe we have all the prerequisites to merge,@DaanHoogland. Shall we?

DaanHoogland reacted with thumbs up emoji

@DaanHooglandDaanHoogland changed the base branch frommain to4.22November 24, 2025 10:37
@DaanHooglandDaanHoogland changed the base branch from4.22 tomainNovember 24, 2025 10:38
@DaanHooglandDaanHoogland modified the milestones:4.22.1,4.23Nov 24, 2025
@DaanHooglandDaanHoogland merged commit8171d95 intoapache:mainNov 24, 2025
28 checks passed
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull requestDec 11, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@slavkapslavkapslavkap approved these changes

@weizhouapacheweizhouapacheweizhouapache approved these changes

@rp-rp-Awaiting requested review from rp-

@harikrishna-patnalaharikrishna-patnalaAwaiting requested review from harikrishna-patnala

Assignees

No one assigned

Projects

None yet

Milestone

4.23.0

Development

Successfully merging this pull request may close these issues.

7 participants

@JoaoJandre@blueorangutan@DaanHoogland@rp-@weizhouapache@slavkap@harikrishna-patnala
[8]ページ先頭
©2009-2026 Movatter.jp