Uh oh!
There was an error while loading. Please reload this page.
CLOUDSTACK-8415 [VMware] SSVM shutdown during snapshot operation results in disks to be left behind - #2090
CLOUDSTACK-8415 [VMware] SSVM shutdown during snapshot operation results in disks to be left behind#2090sureshanaparti wants to merge 1 commit into
Conversation
sureshanaparti
commented
May 9, 2017
@DaanHoogland@wilderrodrigues Please review the changes. |
sureshanaparti
commented
May 10, 2017
@borisstoyanov Can you please kick off VMware trillian tests |
borisstoyanov
commented
May 10, 2017
@blueorangutan package |
blueorangutan
commented
May 10, 2017
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
blueorangutan
commented
May 10, 2017
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-705 |
borisstoyanov
commented
May 10, 2017
@blueorangutan test |
blueorangutan
commented
May 10, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
borisstoyanov
commented
May 11, 2017
@blueorangutan test |
blueorangutan
commented
May 11, 2017
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
DaanHoogland
left a comment
There was a problem hiding this comment.
the functionality is good but shouldn't be in here. It is vmware specific and we should move away from closely tighing storage to hypervisors.
| for (SnapshotVO snapshotVO : snapshots) { | ||
| try { | ||
| List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.findBySnapshotId(snapshotVO.getId()); | ||
| boolean isVMware = snapshotVO.getHypervisorType().equals(HypervisorType.VMware); |
There was a problem hiding this comment.
please try to move this logic into the vmware plugin. this code should remain generic.
opinions @mike-tutkowski@swill @rhtyd ?
There was a problem hiding this comment.
While I believe your comment makes sense, @DaanHoogland, it appears this file has hypervisor-specific logic sprinkled all around it.
Perhaps it would be better to allow this here and for you to write up a bug noting that it would be ideal for this file to be hypervisor agnostic?
There was a problem hiding this comment.
@DaanHoogland@mike-tutkowski Partial disks are left behind in VMware env when SSVM is shutdown/destroyed. So, DeleteSnapshotsDirCommand for Image store and DeleteVMSnapshotCommand for Primary store (to remove worker VM snapshot) has been triggered in VMware env here to cleanup the partial disks created in secondary storage and VM snapshots in primary respectively. The VMware storage manager would cleanup the worker VM snapshot from DeleteVMSnapshotCommand. No VMware specific code here, except sending these commands for cleanup.
| } | ||
| } | ||
| if (removeSnapshot) { | ||
| _snapshotDao.expunge(snapshotVO.getId()); |
There was a problem hiding this comment.
this method is already to long.
please factor out the new part into a seperate method.
There was a problem hiding this comment.
@DaanHoogland moved to a private method removeSnapshot()
blueorangutan
commented
May 11, 2017
Trillian test result (tid-1075)
|
sureshanaparti
commented
May 19, 2017
@DaanHoogland Can you check the changes. |
sureshanaparti
commented
May 30, 2017
@DaanHoogland Can you check and confirm the changes. |
cloudmonger
commented
Jun 14, 2017
ACS CI BVT RunSumarry: Link to logs Folder (search by build_no):https://www.dropbox.com/sh/r2si930m8xxzavs/AAAzNrnoF1fC3auFrvsKo_8-a?dl=0 Failed tests:
Skipped tests: Passed test suits: |
sureshanaparti
commented
Jun 15, 2017
ping @DaanHoogland |
…lts in disks to be left behind
yadvr
commented
Apr 24, 2018
ping @sureshanaparti can you fix the conflicts? |
DaanHoogland
commented
Apr 28, 2018
@sureshanaparti sorry for totally missing this. Yes this looks like what I asked for. please rebase as @rhtyd asked so we can test and merge. |
DaanHoogland
commented
Mar 6, 2020
@rhtyd @PaulAngus@andrijapanicsb is this still relevant? |
yadvr
commented
Jun 17, 2021
Hi, the PR got auto-closed due to no known source repository or some other Github-specific issue during the recent renaming of the default branch to |
Types of changes
In VMware environment, SSVM shutdown during snapshot operation leaves behind partial, unusable disks in secondary storage and VM snapshots.
Fix: Enhance storage garbage collector to delete the partial disks created in secondary storage and VM snapshots while expunging 'ERROR' snapshot.
This closes#540