vmware: do not tear down vm disks if deploy-as-is vm has vm snapshots#9243
vmware: do not tear down vm disks if deploy-as-is vm has vm snapshots#9243DaanHoogland merged 1 commit intoapache:4.19apache/cloudstack:4.19from weizhouapache:4.19-fix-vmware-deploy-as-is-vm-start-with-snapshotweizhouapache/cloudstack:4.19-fix-vmware-deploy-as-is-vm-start-with-snapshotCopy head branch name to clipboard
Conversation
|
@blueorangutan package |
|
@nvazquez @harikrishna-patnala |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #9243 +/- ##
============================================
- Coverage 14.96% 14.96% -0.01%
Complexity 11015 11015
============================================
Files 5377 5377
Lines 469605 469605
Branches 60258 58877 -1381
============================================
- Hits 70296 70292 -4
- Misses 391523 391528 +5
+ Partials 7786 7785 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
sureshanaparti
left a comment
There was a problem hiding this comment.
clgtm, not tested
|
@blueorangutan package |
|
@weizhouapache 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9924 |
| } | ||
| } | ||
| if (deployAsIs) { | ||
| if (deployAsIs && !vmMo.hasSnapshot()) { |
There was a problem hiding this comment.
This seems right to me as I see some other calls of tearDownVmDevices() method before this call which does handle snapshot existance.
https://github.com/shapeblue/cloudstack/blob/78e07cff62dd98d9b88852a47407e80fa57537d2/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java#L2826-L2834
private void tearDownVmDevices(VirtualMachineMO vmMo, boolean hasSnapshot, boolean deployAsIs) throws Exception {
if (deployAsIs) {
vmMo.tearDownDevices(new Class<?>[]{VirtualEthernetCard.class});
} else if (!hasSnapshot) {
vmMo.tearDownDevices(new Class<?>[]{VirtualDisk.class, VirtualEthernetCard.class});
} else {
vmMo.tearDownDevices(new Class<?>[]{VirtualEthernetCard.class});
}
}
There was a problem hiding this comment.
thanks @harikrishna-patnala
It looks consistent with this pr.
I got the error on vcenter: " Invalid configuration for device '0'. Cannot remove virtual disk from the virtual machine because it or one of its parent disks is part of a snapshot of the virtual machine."
it seems to be a known restriction that disk cannot be removed from vm with snapshot.
harikrishna-patnala
left a comment
There was a problem hiding this comment.
Code LGTM, needs testing though
nvazquez
left a comment
There was a problem hiding this comment.
Code LGTM - subject to testing
|
Thanks @harikrishna-patnala @nvazquez @blueorangutan test rocky8 vmware-80 |
|
@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + vmware-80) has been kicked to run smoke tests |
|
[SF] Trillian Build Failed (tid-10434) |
|
[SF] Trillian test result (tid-10436)
|
rajujith
left a comment
There was a problem hiding this comment.
LGTM.
Followed the repro steps in the corresponding issue and the PR fixes it. I can start an instance with data volume having instance snapshots.
Description
This PR fixes #9180
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?