Fix export snapshot and template to secondary storage to export only required disk #5510

harikrishna-patnala · 2021-09-24T08:32:59Z

Description

This PR fixes issue #5498 where multiple volumes of a VM are exported to secondary storage upon taking snapshot of a single volume in case of VMware

Fixes: #5498

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Created a VM with multiple disks
Created snapshot of one disk (also a recurring snapshot). (before creating a snapshot, add some data to the disk)
Check secondary storage snapshot folder to have only one disk
Created a volume from the snapshot above (to check subsequent operations on that snapshot are working fine)
Attached the above volume to a VM and made sure it is the proper disk by checking the data created before in Step 2

…e to export only one required disk

sureshanaparti · 2021-09-24T11:57:50Z

plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java

+        if (vmDisks.length != 1) {
+            String baseName = VmwareHelper.getDiskDeviceFileName(requiredDisk);
+            s_logger.info(String.format("Detaching all disks for the cloned VM: %s except disk with base name: %s, key=%d", clonedVm.getName(), baseName, requiredDisk.getKey()));
+            clonedVm.detachAllDisksExcept(VmwareHelper.getDiskDeviceFileName(requiredDisk), null);


Is it possible to include this process, as part of creating full clone with specific disk (which ensures it detaches/destroys the additional disks), instead checking this separately.

I'll try to keep it in same method. Thanks Suresh

Moved the process to virtualMachineMO class.

rohityadavcloud · 2021-09-24T12:03:26Z

@harikrishna-patnala does the sdk/api behave differently in different vmware versions?
@blueorangutan test centos7 vmware-67u3

harikrishna-patnala · 2021-09-24T12:14:09Z

@harikrishna-patnala does the sdk/api behave differently in different vmware versions?

Ideally it should not differ, I'll test all versions.

rohityadavcloud · 2021-09-27T09:20:42Z

This needs a manual test confirmation cc @borisstoyanov @vladimirpetrov @NuxRo thanks

vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java

borisstoyanov

LGTM, manually tested it.

blueorangutan · 2021-09-29T12:32:03Z

@sureshanaparti a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

sureshanaparti

code LGTM

blueorangutan · 2021-09-29T23:04:37Z

Trillian test result (tid-2239)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36756 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5510-t2239-vmware-67u3.zip
Smoke tests completed. 89 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_live_migrate_VM_with_two_data_disks	`Error`	61.81	test_vm_life_cycle.py

nvazquez · 2021-09-30T02:35:13Z

@blueorangutan test centos7 vmware-67u3

blueorangutan · 2021-09-30T02:36:03Z

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests

blueorangutan · 2021-09-30T13:59:58Z

Trillian test result (tid-2243)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 39958 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5510-t2243-vmware-67u3.zip
Smoke tests completed. 89 look OK, 1 have errors
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_live_migrate_VM_with_two_data_disks	`Error`	63.73	test_vm_life_cycle.py

nvazquez · 2021-09-30T14:01:34Z

Edit: smoke test failure also seen on the health checks PR, merging this PR - test needs fixing on stabilisation phase

…required disk (apache#5510) * Fix export snapshot and export template to secondary storage in VMware to export only one required disk * Move clone operation into virtual machine mo * Code refactored for readability * Added disk key check even for successful clone operation * Delete dettached disks from cloned VM and added few logs

…required disk (apache#5510) * Fix export snapshot and export template to secondary storage in VMware to export only one required disk * Move clone operation into virtual machine mo * Code refactored for readability * Added disk key check even for successful clone operation * Delete dettached disks from cloned VM and added few logs (cherry picked from commit df0c004) Signed-off-by: Rohit Yadav <[email protected]>

Fix export snapshot and export template to secondary storage in VMwar…

e4727f7

…e to export only one required disk

harikrishna-patnala added this to the 4.16.0.0 milestone Sep 24, 2021

harikrishna-patnala changed the title ~~Fix export snapshot and export template to secondary storage in VMwar…~~ Fix export snapshot and template to secondary storage to export only required disk. Sep 24, 2021

harikrishna-patnala changed the title ~~Fix export snapshot and template to secondary storage to export only required disk.~~ Fix export snapshot and template to secondary storage to export only required disk Sep 24, 2021

apache deleted a comment from blueorangutan Sep 24, 2021

rohityadavcloud added Severity:Critical Critical bug component:vmware labels Sep 24, 2021

rohityadavcloud requested review from nvazquez, shwstppr, sureshanaparti, davidjumani and weizhouapache September 24, 2021 11:49

rohityadavcloud added the status:needs-testing label Sep 24, 2021

sureshanaparti reviewed Sep 24, 2021

View reviewed changes

weizhouapache closed this Sep 24, 2021

weizhouapache reopened this Sep 24, 2021

rohityadavcloud approved these changes Sep 27, 2021

View reviewed changes

Move clone operation into virtual machine mo

874b5a3

harikrishna-patnala force-pushed the VMwareExportSnapshotFix branch from ea5999b to 874b5a3 Compare September 27, 2021 10:53

borisstoyanov self-assigned this Sep 27, 2021

GutoVeronezi reviewed Sep 27, 2021

View reviewed changes

vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java Outdated Show resolved Hide resolved

vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java Outdated Show resolved Hide resolved

sureshanaparti reviewed Sep 28, 2021

View reviewed changes

vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java Outdated Show resolved Hide resolved

borisstoyanov approved these changes Sep 28, 2021

View reviewed changes

borisstoyanov removed their assignment Sep 28, 2021

nvazquez removed the status:needs-testing label Sep 28, 2021

harikrishna-patnala added 2 commits September 29, 2021 08:48

Code refactored for readability

4e78a33

Added disk key check even for successful clone operation

1ee3594

sureshanaparti approved these changes Sep 29, 2021

View reviewed changes

nvazquez approved these changes Sep 30, 2021

View reviewed changes

nvazquez merged commit df0c004 into main Sep 30, 2021

apache deleted a comment from blueorangutan Jan 11, 2022

apache deleted a comment from rohityadavcloud Jan 11, 2022

apache deleted a comment from blueorangutan Jan 11, 2022

apache deleted a comment from rohityadavcloud Jan 11, 2022

apache deleted a comment from blueorangutan Jan 11, 2022

apache deleted a comment from harikrishna-patnala Jan 11, 2022

apache deleted a comment from blueorangutan Jan 11, 2022

apache deleted a comment from harikrishna-patnala Jan 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix export snapshot and template to secondary storage to export only required disk #5510

Fix export snapshot and template to secondary storage to export only required disk #5510

harikrishna-patnala commented Sep 24, 2021 •

edited

Loading

sureshanaparti Sep 24, 2021

harikrishna-patnala Sep 24, 2021

harikrishna-patnala Sep 27, 2021

rohityadavcloud commented Sep 24, 2021

harikrishna-patnala commented Sep 24, 2021

rohityadavcloud commented Sep 27, 2021

borisstoyanov left a comment

blueorangutan commented Sep 29, 2021

sureshanaparti left a comment

blueorangutan commented Sep 29, 2021

nvazquez commented Sep 30, 2021

blueorangutan commented Sep 30, 2021

blueorangutan commented Sep 30, 2021

nvazquez commented Sep 30, 2021 •

edited

Loading

Fix export snapshot and template to secondary storage to export only required disk #5510

Fix export snapshot and template to secondary storage to export only required disk #5510

Conversation

harikrishna-patnala commented Sep 24, 2021 • edited Loading

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

sureshanaparti Sep 24, 2021

Choose a reason for hiding this comment

harikrishna-patnala Sep 24, 2021

Choose a reason for hiding this comment

harikrishna-patnala Sep 27, 2021

Choose a reason for hiding this comment

rohityadavcloud commented Sep 24, 2021

harikrishna-patnala commented Sep 24, 2021

rohityadavcloud commented Sep 27, 2021

borisstoyanov left a comment

Choose a reason for hiding this comment

blueorangutan commented Sep 29, 2021

sureshanaparti left a comment

Choose a reason for hiding this comment

blueorangutan commented Sep 29, 2021

nvazquez commented Sep 30, 2021

blueorangutan commented Sep 30, 2021

blueorangutan commented Sep 30, 2021

nvazquez commented Sep 30, 2021 • edited Loading

harikrishna-patnala commented Sep 24, 2021 •

edited

Loading

nvazquez commented Sep 30, 2021 •

edited

Loading