-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Update error message when no snapshot strategy is found #11455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11455 +/- ##
============================================
- Coverage 17.36% 17.36% -0.01%
+ Complexity 15237 15236 -1
============================================
Files 5886 5886
Lines 525650 525653 +3
Branches 64157 64157
============================================
Hits 91260 91260
- Misses 424094 424097 +3
Partials 10296 10296
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:
|
sureshanaparti
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
|
@blueorangutan package |
|
@sureshanaparti 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. |
| _snapshotDao.remove(snapshotId); | ||
| logger.debug("No strategy found for creation of snapshot [{}], removing its record from the database.", snapshot); | ||
| throw new CloudRuntimeException(String.format("Can't find snapshot strategy to deal with snapshot:%s", snapshot.getSnapshotVO())); | ||
| throw new UnsupportedOperationException(String.format("Unable to find a snapshot strategy to create snapshot [%s] of volume [%s]. Please check the logs.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoaoJandre
thanks for the PR
this is not user-friendly I think, only administrator can access the check the logs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weizhouapache How would you change the message so that it is still generic (this is an error when no snapshot strategy is found; thus this message will be shown for other situations as well) and does not reveal the infrastructure information to the end user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think more details but does not reveal the critical information would be better
for example, when the management server log says
2025-08-15 05:58:04,557 WARN [o.a.c.s.s.DefaultSnapshotStrategy] (Work-Job-Executor-6:[ctx-bcc76f85, job-179/job-180, ctx-5bbd5fed]) (logid:2d361233) VM [b50bdd89-1b6b-4ef3-bf85-c97d8cfa01a2] already has KVM File-Based storage VM snapshots. These VM snapshots and volume snapshots are not supported together for KVM. As restoring volume snapshots will erase the VM snapshots and cause data loss.
end users could get an error when create a volume snapshot, like
Volume snapshot is unsupported as this VM has a disk-only VM snapshots
btw: would it be possible to check the condition in a ealier stage ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoaoJandre please raise separate PR if error message can be improved. thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weizhouapache We currently have 5 snapshot strategies. This error is thrown when no snapshot strategy supports the snapshot being taken. Thus, the message cannot be specific to a single strategy.
Depending on the storage/strategy being used, it might be possible to create volume snapshots and disk-only VM snapshots.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JoaoJandre
ok, ignore it if it is difficult to improve
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14638 |

Description
This PR updates the error message shown when no snapshot strategy is found.
Fixes #11453
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
How Has This Been Tested?
I created a disk-only VM snapshot using the KvmFileBasedStorageVmSnapshotStrategy and then tried to create a volume snapshot. I got the following error, as expected: