[GitHub] mike-tutkowski commented on a change in pull request #2090: CLOUDSTACK-8415 [VMware] SSVM shutdown during snapshot operation results in disks to be left behind

[gi...@git.apache.org]

mike-tutkowski commented on a change in pull request #2090: CLOUDSTACK-8415 [VMware] SSVM shutdown during snapshot operation results in disks to be left behind
URL: https://github.com/apache/cloudstack/pull/2090#discussion_r116519545
 
 

 ##########
 File path: server/src/com/cloud/storage/StorageManagerImpl.java
 ##########
 @@ -1104,14 +1107,61 @@ public void cleanupStorage(boolean recurring) {
                     }
 
                     // remove snapshots in Error state
-                    List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);
+                    List<SnapshotVO> snapshots = _snapshotDao.listAllByStatusIncludingRemoved(Snapshot.State.Error);
                     for (SnapshotVO snapshotVO : snapshots) {
                         try {
                             List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.findBySnapshotId(snapshotVO.getId());
+                            boolean isVMware = snapshotVO.getHypervisorType().equals(HypervisorType.VMware);
 
 Review comment:
   Why 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?
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services