| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
@DaanHoogland @wilderrodrigues Please review the changes. |
Sorry, something went wrong.
|
@borisstoyanov Can you please kick off VMware trillian tests |
Sorry, something went wrong.
|
@borisstoyanov a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
Sorry, something went wrong.
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-705 |
Sorry, something went wrong.
|
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Sorry, something went wrong.
|
@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Sorry, something went wrong.
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.
Sorry, something went wrong.
| 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 ?
Sorry, something went wrong.
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?
Sorry, something went wrong.
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.
Sorry, something went wrong.
| } | ||
| } | ||
| 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.
Sorry, something went wrong.
There was a problem hiding this comment.
@DaanHoogland moved to a private method removeSnapshot()
Sorry, something went wrong.
|
Trillian test result (tid-1075)
|
Sorry, something went wrong.
|
@DaanHoogland Can you check the changes. |
Sorry, something went wrong.
|
@DaanHoogland Can you check and confirm the changes. |
Sorry, something went wrong.
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: |
Sorry, something went wrong.
…lts in disks to be left behind
|
ping @sureshanaparti can you fix the conflicts? |
Sorry, something went wrong.
|
@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. |
Sorry, something went wrong.
|
@rhtyd @PaulAngus @andrijapanicsb is this still relevant? |
Sorry, something went wrong.
|
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 main. If you wish kindly submit a new PR. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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