You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ra...@apache.org on 2017/02/08 06:58:50 UTC

[1/2] git commit: updated refs/heads/master to e303baa

Repository: cloudstack
Updated Branches:
  refs/heads/master 202b92f24 -> e303baa1b


CLOUDSTACK-9738: [Vmware] Optimize vm expunge process for instances with vm snapshots


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/6ce6cf67
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/6ce6cf67
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/6ce6cf67

Branch: refs/heads/master
Commit: 6ce6cf67f024e8e31d2ae2ab60fe1a758e42fcc3
Parents: 8e069ed
Author: nvazquez <ni...@gmail.com>
Authored: Mon Jan 9 17:08:32 2017 -0300
Committer: nvazquez <ni...@gmail.com>
Committed: Mon Feb 6 23:39:01 2017 -0300

----------------------------------------------------------------------
 api/src/com/cloud/vm/UserVmService.java         |  6 ++--
 .../cloud/vm/snapshot/VMSnapshotService.java    |  7 +++++
 .../src/com/cloud/vm/VirtualMachineManager.java |  2 +-
 .../cloud/entity/api/VirtualMachineEntity.java  |  3 +-
 .../api/storage/VMSnapshotStrategy.java         |  8 ++++++
 .../com/cloud/vm/VirtualMachineManagerImpl.java | 30 ++++++++++++++++----
 .../cloud/entity/api/VMEntityManager.java       |  2 +-
 .../cloud/entity/api/VMEntityManagerImpl.java   |  4 +--
 .../entity/api/VirtualMachineEntityImpl.java    |  4 +--
 .../vmsnapshot/DefaultVMSnapshotStrategy.java   | 11 +++++++
 .../resource/VmwareStorageProcessor.java        |  3 --
 .../cloud/ha/HighAvailabilityManagerImpl.java   |  2 +-
 .../cloud/network/as/AutoScaleManagerImpl.java  |  2 +-
 .../com/cloud/resource/ResourceManagerImpl.java |  2 +-
 .../src/com/cloud/user/AccountManagerImpl.java  |  2 +-
 server/src/com/cloud/vm/UserVmManagerImpl.java  |  8 ++++--
 .../vm/snapshot/VMSnapshotManagerImpl.java      | 21 ++++++++++++++
 ...AccountManagerImplVolumeDeleteEventTest.java |  3 +-
 18 files changed, 93 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/api/src/com/cloud/vm/UserVmService.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/vm/UserVmService.java b/api/src/com/cloud/vm/UserVmService.java
index 54206ed..db9ded8 100644
--- a/api/src/com/cloud/vm/UserVmService.java
+++ b/api/src/com/cloud/vm/UserVmService.java
@@ -75,14 +75,14 @@ public interface UserVmService {
     /**
      * Destroys one virtual machine
      *
-     * @param userId
-     *            the id of the user performing the action
      * @param vmId
      *            the id of the virtual machine.
+     * @param expunge
+     *            indicates if vm should be expunged
      * @throws ConcurrentOperationException
      * @throws ResourceUnavailableException
      */
-    UserVm destroyVm(long vmId) throws ResourceUnavailableException, ConcurrentOperationException;
+    UserVm destroyVm(long vmId, boolean expunge) throws ResourceUnavailableException, ConcurrentOperationException;
 
     /**
      * Resets the password of a virtual machine.

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
----------------------------------------------------------------------
diff --git a/api/src/com/cloud/vm/snapshot/VMSnapshotService.java b/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
index 0d4d22c..e376265 100644
--- a/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
+++ b/api/src/com/cloud/vm/snapshot/VMSnapshotService.java
@@ -46,4 +46,11 @@ public interface VMSnapshotService {
         ConcurrentOperationException;
 
     VirtualMachine getVMBySnapshotId(Long id);
+
+    /**
+     * Delete vm snapshots only from database. Introduced as a Vmware optimization in which vm snapshots are deleted when
+     * the vm gets deleted on hypervisor (no need to delete each vm snapshot before deleting vm, just mark them as deleted on DB)
+     * @param id vm id
+     */
+    boolean deleteVMSnapshotsFromDB(Long vmId);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/api/src/com/cloud/vm/VirtualMachineManager.java
----------------------------------------------------------------------
diff --git a/engine/api/src/com/cloud/vm/VirtualMachineManager.java b/engine/api/src/com/cloud/vm/VirtualMachineManager.java
index b205415..4b61caa 100644
--- a/engine/api/src/com/cloud/vm/VirtualMachineManager.java
+++ b/engine/api/src/com/cloud/vm/VirtualMachineManager.java
@@ -110,7 +110,7 @@ public interface VirtualMachineManager extends Manager {
 
     void advanceExpunge(String vmUuid) throws ResourceUnavailableException, OperationTimedoutException, ConcurrentOperationException;
 
-    void destroy(String vmUuid) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException;
+    void destroy(String vmUuid, boolean expunge) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException;
 
     void migrateAway(String vmUuid, long hostId) throws InsufficientServerCapacityException;
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java b/engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java
index 1c3af62..de91ea7 100644
--- a/engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java
+++ b/engine/api/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntity.java
@@ -129,8 +129,9 @@ public interface VirtualMachineEntity extends CloudStackEntity {
 
     /**
      * Destroys the VM.
+     * @param expunge indicates if vm should be expunged
      */
-    boolean destroy(String caller) throws AgentUnavailableException, CloudException, ConcurrentOperationException;
+    boolean destroy(String caller, boolean expunge) throws AgentUnavailableException, CloudException, ConcurrentOperationException;
 
     /**
      * Duplicate this VM in the database so that it will start new

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
index 239ba87..38f633a 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VMSnapshotStrategy.java
@@ -28,4 +28,12 @@ public interface VMSnapshotStrategy {
     boolean revertVMSnapshot(VMSnapshot vmSnapshot);
 
     StrategyPriority canHandle(VMSnapshot vmSnapshot);
+
+    /**
+     * Delete vm snapshot only from database. Introduced as a Vmware optimization in which vm snapshots are deleted when
+     * the vm gets deleted on hypervisor (no need to delete each vm snapshot before deleting vm, just mark them as deleted on DB)
+     * @param vmSnapshot vm snapshot to be marked as deleted.
+     * @return true if vm snapshot removed from DB, false if not.
+     */
+    boolean deleteVMSnapshotFromDB(VMSnapshot vmSnapshot);
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
old mode 100644
new mode 100755
index b1c69b2..d19aaf3
--- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
+++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -1679,7 +1679,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
     }
 
     @Override
-    public void destroy(final String vmUuid) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException {
+    public void destroy(final String vmUuid, final boolean expunge) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException {
         VMInstanceVO vm = _vmDao.findByUuid(vmUuid);
         if (vm == null || vm.getState() == State.Destroyed || vm.getState() == State.Expunging || vm.getRemoved() != null) {
             if (s_logger.isDebugEnabled()) {
@@ -1689,15 +1689,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         }
 
         if (s_logger.isDebugEnabled()) {
-            s_logger.debug("Destroying vm " + vm);
+            s_logger.debug("Destroying vm " + vm + ", expunge flag " + (expunge ? "on" : "off"));
         }
 
         advanceStop(vmUuid, VmDestroyForcestop.value());
 
-        if (!_vmSnapshotMgr.deleteAllVMSnapshots(vm.getId(), null)) {
-            s_logger.debug("Unable to delete all snapshots for " + vm);
-            throw new CloudRuntimeException("Unable to delete vm snapshots for " + vm);
-        }
+        deleteVMSnapshots(vm, expunge);
 
         // reload the vm object from db
         vm = _vmDao.findByUuid(vmUuid);
@@ -1712,6 +1709,27 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         }
     }
 
+    /**
+     * Delete vm snapshots depending on vm's hypervisor type. For Vmware, vm snapshots removal is delegated to vm cleanup thread
+     * to reduce tasks sent to hypervisor (one tasks to delete vm snapshots and vm itself
+     * instead of one task for each vm snapshot plus another for the vm)
+     * @param vm vm
+     * @param expunge indicates if vm should be expunged
+     */
+    private void deleteVMSnapshots(VMInstanceVO vm, boolean expunge) {
+        if (! vm.getHypervisorType().equals(HypervisorType.VMware)) {
+            if (!_vmSnapshotMgr.deleteAllVMSnapshots(vm.getId(), null)) {
+                s_logger.debug("Unable to delete all snapshots for " + vm);
+                throw new CloudRuntimeException("Unable to delete vm snapshots for " + vm);
+            }
+        }
+        else {
+            if (expunge) {
+                _vmSnapshotMgr.deleteVMSnapshotsFromDB(vm.getId());
+            }
+        }
+    }
+
     protected boolean checkVmOnHost(final VirtualMachine vm, final long hostId) throws AgentUnavailableException, OperationTimedoutException {
         final Answer answer = _agentMgr.send(hostId, new CheckVirtualMachineCommand(vm.getInstanceName()));
         if (answer == null || !answer.getResult()) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManager.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManager.java b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManager.java
index 0c357ca..7e3edc6 100644
--- a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManager.java
+++ b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManager.java
@@ -46,5 +46,5 @@ public interface VMEntityManager {
 
     boolean stopvirtualmachineforced(VMEntityVO vmEntityVO, String caller) throws ResourceUnavailableException;
 
-    boolean destroyVirtualMachine(VMEntityVO vmEntityVO, String caller) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException;
+    boolean destroyVirtualMachine(VMEntityVO vmEntityVO, String caller, boolean expunge) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException;
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java
index a944b93..1911645 100644
--- a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java
+++ b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java
@@ -261,10 +261,10 @@ public class VMEntityManagerImpl implements VMEntityManager {
     }
 
     @Override
-    public boolean destroyVirtualMachine(VMEntityVO vmEntityVO, String caller) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException {
+    public boolean destroyVirtualMachine(VMEntityVO vmEntityVO, String caller, boolean expunge) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException {
 
         VMInstanceVO vm = _vmDao.findByUuid(vmEntityVO.getUuid());
-        _itMgr.destroy(vm.getUuid());
+        _itMgr.destroy(vm.getUuid(), expunge);
         return true;
     }
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java
index 2f4de36..7a8f624 100644
--- a/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java
+++ b/engine/orchestration/src/org/apache/cloudstack/engine/cloud/entity/api/VirtualMachineEntityImpl.java
@@ -229,8 +229,8 @@ public class VirtualMachineEntityImpl implements VirtualMachineEntity {
     }
 
     @Override
-    public boolean destroy(String caller) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException {
-        return manager.destroyVirtualMachine(this.vmEntityVO, caller);
+    public boolean destroy(String caller, boolean expunge) throws AgentUnavailableException, OperationTimedoutException, ConcurrentOperationException {
+        return manager.destroyVirtualMachine(this.vmEntityVO, caller, expunge);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
----------------------------------------------------------------------
diff --git a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
index 13fd54c..71a5e10 100644
--- a/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
+++ b/engine/storage/snapshot/src/org/apache/cloudstack/storage/vmsnapshot/DefaultVMSnapshotStrategy.java
@@ -392,4 +392,15 @@ public class DefaultVMSnapshotStrategy extends ManagerBase implements VMSnapshot
     public StrategyPriority canHandle(VMSnapshot vmSnapshot) {
         return StrategyPriority.DEFAULT;
     }
+
+    @Override
+    public boolean deleteVMSnapshotFromDB(VMSnapshot vmSnapshot) {
+        try {
+            vmSnapshotHelper.vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.ExpungeRequested);
+        } catch (NoTransitionException e) {
+            s_logger.debug("Failed to change vm snapshot state with event ExpungeRequested");
+            throw new CloudRuntimeException("Failed to change vm snapshot state with event ExpungeRequested: " + e.getMessage());
+        }
+        return vmSnapshotDao.remove(vmSnapshot.getId());
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
index 8ce65c4..e790d47 100644
--- a/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
+++ b/plugins/hypervisors/vmware/src/com/cloud/storage/resource/VmwareStorageProcessor.java
@@ -1679,9 +1679,6 @@ public class VmwareStorageProcessor implements StorageProcessor {
                             s_logger.info("Destroy root volume and VM itself. vmName " + vmName);
                         }
 
-                        // Remove all snapshots to consolidate disks for removal
-                        vmMo.removeAllSnapshots();
-
                         VirtualMachineDiskInfo diskInfo = null;
                         if (vol.getChainInfo() != null)
                             diskInfo = _gson.fromJson(vol.getChainInfo(), VirtualMachineDiskInfo.class);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java b/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java
index 53d79f7..fe6b717 100644
--- a/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java
+++ b/server/src/com/cloud/ha/HighAvailabilityManagerImpl.java
@@ -674,7 +674,7 @@ public class HighAvailabilityManagerImpl extends ManagerBase implements HighAvai
             }
 
             if (vm.getHostId() != null) {
-                _itMgr.destroy(vm.getUuid());
+                _itMgr.destroy(vm.getUuid(), false);
                 s_logger.info("Successfully destroy " + vm);
                 return null;
             } else {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/server/src/com/cloud/network/as/AutoScaleManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/as/AutoScaleManagerImpl.java b/server/src/com/cloud/network/as/AutoScaleManagerImpl.java
index 657d4d4..d911360 100644
--- a/server/src/com/cloud/network/as/AutoScaleManagerImpl.java
+++ b/server/src/com/cloud/network/as/AutoScaleManagerImpl.java
@@ -1514,7 +1514,7 @@ public class AutoScaleManagerImpl<Type> extends ManagerBase implements AutoScale
                     public void run() {
                         try {
 
-                            _userVmManager.destroyVm(vmId);
+                            _userVmManager.destroyVm(vmId, false);
 
                         } catch (ResourceUnavailableException e) {
                             e.printStackTrace();

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/server/src/com/cloud/resource/ResourceManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/resource/ResourceManagerImpl.java b/server/src/com/cloud/resource/ResourceManagerImpl.java
index 2bb1596..2efce06 100644
--- a/server/src/com/cloud/resource/ResourceManagerImpl.java
+++ b/server/src/com/cloud/resource/ResourceManagerImpl.java
@@ -2165,7 +2165,7 @@ public class ResourceManagerImpl extends ManagerBase implements ResourceManager,
                 final List<VMInstanceVO> vmsOnLocalStorage = _storageMgr.listByStoragePool(storagePool.getId());
                 for (final VMInstanceVO vm : vmsOnLocalStorage) {
                     try {
-                        _vmMgr.destroy(vm.getUuid());
+                        _vmMgr.destroy(vm.getUuid(), false);
                     } catch (final Exception e) {
                         final String errorMsg = "There was an error Destory the vm: " + vm + " as a part of hostDelete id=" + host.getId();
                         s_logger.debug(errorMsg, e);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/server/src/com/cloud/user/AccountManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
index c7a6ef6..1a3607f 100644
--- a/server/src/com/cloud/user/AccountManagerImpl.java
+++ b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -759,7 +759,7 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
             for (UserVmVO vm : vms) {
                 if (vm.getState() != VirtualMachine.State.Destroyed && vm.getState() != VirtualMachine.State.Expunging) {
                     try {
-                        _vmMgr.destroyVm(vm.getId());
+                        _vmMgr.destroyVm(vm.getId(), false);
                     } catch (Exception e) {
                         e.printStackTrace();
                         s_logger.warn("Failed destroying instance " + vm.getUuid() + " as part of account deletion.");

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/server/src/com/cloud/vm/UserVmManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java
old mode 100644
new mode 100755
index ec7a6f6..dedeab1
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -2627,7 +2627,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             throw new PermissionDeniedException("Parameter " + ApiConstants.EXPUNGE + " can be passed by Admin only. Or when the allow.user.expunge.recover.vm key is set.");
         }
 
-        UserVm destroyedVm = destroyVm(vmId);
+        UserVm destroyedVm = destroyVm(vmId, expunge);
         if (expunge) {
             UserVmVO vm = _vmDao.findById(vmId);
             if (!expunge(vm, ctx.getCallingUserId(), ctx.getCallingAccount())) {
@@ -4114,7 +4114,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
     }
 
     @Override
-    public UserVm destroyVm(long vmId) throws ResourceUnavailableException, ConcurrentOperationException {
+    public UserVm destroyVm(long vmId, boolean expunge) throws ResourceUnavailableException, ConcurrentOperationException {
         // Account caller = CallContext.current().getCallingAccount();
         // Long userId = CallContext.current().getCallingUserId();
         Long userId = 2L;
@@ -4136,7 +4136,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
         try {
             VirtualMachineEntity vmEntity = _orchSrvc.getVirtualMachine(vm.getUuid());
-            status = vmEntity.destroy(Long.toString(userId));
+            status = vmEntity.destroy(Long.toString(userId), expunge);
         } catch (CloudException e) {
             CloudRuntimeException ex = new CloudRuntimeException("Unable to destroy with specified vmId", e);
             ex.addProxyObject(vm.getUuid(), "vmId");
@@ -4318,6 +4318,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             throw new PermissionDeniedException("Expunging a vm can only be done by an Admin. Or when the allow.user.expunge.recover.vm key is set.");
         }
 
+        _vmSnapshotMgr.deleteVMSnapshotsFromDB(vmId);
+
         boolean status;
 
         status = expunge(vm, userId, caller);

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
index d155186..5f9115f 100644
--- a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
+++ b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java
@@ -1133,4 +1133,25 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme
         }
         return result;
     }
+
+    @Override
+    public boolean deleteVMSnapshotsFromDB(Long vmId) {
+        List<VMSnapshotVO> listVmSnapshots = _vmSnapshotDao.findByVm(vmId);
+        if (listVmSnapshots == null || listVmSnapshots.isEmpty()) {
+            return true;
+        }
+        for (VMSnapshotVO snapshot : listVmSnapshots) {
+            try {
+                VMSnapshotStrategy strategy = findVMSnapshotStrategy(snapshot);
+                if (! strategy.deleteVMSnapshotFromDB(snapshot)) {
+                    s_logger.error("Couldn't delete vm snapshot with id " + snapshot.getId());
+                    return false;
+                }
+            }
+            catch (CloudRuntimeException e) {
+                s_logger.error("Couldn't delete vm snapshot due to: " + e.getMessage());
+            }
+        }
+        return true;
+    }
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/6ce6cf67/server/test/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
----------------------------------------------------------------------
diff --git a/server/test/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java b/server/test/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
index 43c8cfc..2397713 100644
--- a/server/test/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
+++ b/server/test/com/cloud/user/AccountManagerImplVolumeDeleteEventTest.java
@@ -21,6 +21,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.anyLong;
 import static org.mockito.Mockito.anyString;
+import static org.mockito.Mockito.anyBoolean;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
@@ -118,7 +119,7 @@ public class AccountManagerImplVolumeDeleteEventTest extends AccountManagetImplT
         VirtualMachineEntity vmEntity = mock(VirtualMachineEntity.class);
 
         when(_orchSrvc.getVirtualMachine(anyString())).thenReturn(vmEntity);
-        when(vmEntity.destroy(anyString())).thenReturn(true);
+        when(vmEntity.destroy(anyString(), anyBoolean())).thenReturn(true);
 
         Mockito.doReturn(vm).when(_vmDao).findById(anyLong());
 


[2/2] git commit: updated refs/heads/master to e303baa

Posted by ra...@apache.org.
Merge pull request #1905 from nvazquez/expungeVmRefactor

CLOUDSTACK-9738: [Vmware] Optimize vm expunge process for instances with vm snapshots## Description
It was noticed that expunging instances with many vm snapshots took a look of time, as hypervisor received as many tasks as vm snapshots instance had, apart from the delete vm task. We propose a way to optimize this process for instances with vm snapshots by sending only one delete task to hypervisor, which will delete vm and its snapshots

## Use cases

1. deleteVMsnapohsot-> no changes to current behavior
2. destroyVM with expunge=false ->  no actions to VMsnaphsot is performed at the moment. When VM cleanup thread is executed it will perform the same sequence as (3). If instance is recovered before expunged by the cleanup thread it will remain intact with VMSnapshot chain present
3. destroyVM with expunge=true:
   * Vmsnaphsot is marked with removed timestamp and state = Expunging in DB
   * VM is deleted in HW

* pr/1905:
  CLOUDSTACK-9738: [Vmware] Optimize vm expunge process for instances with vm snapshots

Signed-off-by: Rajani Karuturi <ra...@accelerite.com>


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/e303baa1
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/e303baa1
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/e303baa1

Branch: refs/heads/master
Commit: e303baa1bf10e464d55b078d39cf3f42de1dc2c0
Parents: 202b92f 6ce6cf6
Author: Rajani Karuturi <ra...@accelerite.com>
Authored: Wed Feb 8 12:28:08 2017 +0530
Committer: Rajani Karuturi <ra...@accelerite.com>
Committed: Wed Feb 8 12:28:09 2017 +0530

----------------------------------------------------------------------
 api/src/com/cloud/vm/UserVmService.java         |  6 ++--
 .../cloud/vm/snapshot/VMSnapshotService.java    |  7 +++++
 .../src/com/cloud/vm/VirtualMachineManager.java |  2 +-
 .../cloud/entity/api/VirtualMachineEntity.java  |  3 +-
 .../api/storage/VMSnapshotStrategy.java         |  8 ++++++
 .../com/cloud/vm/VirtualMachineManagerImpl.java | 30 ++++++++++++++++----
 .../cloud/entity/api/VMEntityManager.java       |  2 +-
 .../cloud/entity/api/VMEntityManagerImpl.java   |  4 +--
 .../entity/api/VirtualMachineEntityImpl.java    |  4 +--
 .../vmsnapshot/DefaultVMSnapshotStrategy.java   | 11 +++++++
 .../resource/VmwareStorageProcessor.java        |  3 --
 .../cloud/ha/HighAvailabilityManagerImpl.java   |  2 +-
 .../cloud/network/as/AutoScaleManagerImpl.java  |  2 +-
 .../com/cloud/resource/ResourceManagerImpl.java |  2 +-
 .../src/com/cloud/user/AccountManagerImpl.java  |  2 +-
 server/src/com/cloud/vm/UserVmManagerImpl.java  |  8 ++++--
 .../vm/snapshot/VMSnapshotManagerImpl.java      | 21 ++++++++++++++
 ...AccountManagerImplVolumeDeleteEventTest.java |  3 +-
 18 files changed, 93 insertions(+), 27 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e303baa1/server/src/com/cloud/vm/UserVmManagerImpl.java
----------------------------------------------------------------------
diff --cc server/src/com/cloud/vm/UserVmManagerImpl.java
index 9d31d37,dedeab1..de4eaf2
mode 100644,100755..100755
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java