From c463057c82656a4d7564fc9205bb79517317c629 Mon Sep 17 00:00:00 2001 From: Mike Tutkowski Date: Wed, 10 May 2017 21:01:27 -0600 Subject: [PATCH] Fix for CLOUDSTACK-9660 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A root volume can be replaced by a different root volume without the VM it belongs to being expunged. From dev@: For example: Let’s say we have a system VM running on NFS primary storage. We then put this primary storage into maintenance mode, which creates the system VM (with the same name) on a different primary storage (we do not create a new row in the cloud.vm_instance table for this VM). While this VM works, the original root disk of the system VM remains on the original primary storage and is not destroyed by the code in StorageManagerImpl.cleanupStorage(boolean) in 4.10 because 4.10 (as shown above) only asks for non-root volumes to consider for deletion. In the 4.9 version of the code, the original root disk is cleaned up in StorageManagerImpl.cleanupStorage(boolean). The problem with 4.10 relying on a root disk always being deleted when the VM it belongs to is deleted is that in a situation like this that the system VM doesn’t get deleted at this point – it gets a new root disk that’s hosted by a different primary storage (so now it’s original root disk is stranded). --- engine/schema/src/com/cloud/storage/dao/VolumeDao.java | 2 +- engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java | 4 +--- server/src/com/cloud/storage/StorageManagerImpl.java | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/engine/schema/src/com/cloud/storage/dao/VolumeDao.java b/engine/schema/src/com/cloud/storage/dao/VolumeDao.java index f2d5fc735207..a05dc1f560da 100644 --- a/engine/schema/src/com/cloud/storage/dao/VolumeDao.java +++ b/engine/schema/src/com/cloud/storage/dao/VolumeDao.java @@ -80,7 +80,7 @@ public interface VolumeDao extends GenericDao, StateDao listVolumesToBeDestroyed(); - List listNonRootVolumesToBeDestroyed(Date date); + List listVolumesToBeDestroyed(Date date); ImageFormat getImageFormat(Long volumeId); diff --git a/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java b/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java index 4f5b613ddd15..6ed556eabece 100644 --- a/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java +++ b/engine/schema/src/com/cloud/storage/dao/VolumeDaoImpl.java @@ -325,7 +325,6 @@ public VolumeDaoImpl() { AllFieldsSearch.and("deviceId", AllFieldsSearch.entity().getDeviceId(), Op.EQ); AllFieldsSearch.and("poolId", AllFieldsSearch.entity().getPoolId(), Op.EQ); AllFieldsSearch.and("vType", AllFieldsSearch.entity().getVolumeType(), Op.EQ); - AllFieldsSearch.and("notVolumeType", AllFieldsSearch.entity().getVolumeType(), Op.NEQ); AllFieldsSearch.and("id", AllFieldsSearch.entity().getId(), Op.EQ); AllFieldsSearch.and("destroyed", AllFieldsSearch.entity().getState(), Op.EQ); AllFieldsSearch.and("notDestroyed", AllFieldsSearch.entity().getState(), Op.NEQ); @@ -482,10 +481,9 @@ public List listVolumesToBeDestroyed() { } @Override - public List listNonRootVolumesToBeDestroyed(Date date) { + public List listVolumesToBeDestroyed(Date date) { SearchCriteria sc = AllFieldsSearch.create(); sc.setParameters("state", Volume.State.Destroy); - sc.setParameters("notVolumeType", Volume.Type.ROOT.toString()); sc.setParameters("updateTime", date); return listBy(sc); diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 10ca42473683..8f5b053d3840 100644 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -1080,8 +1080,7 @@ public void cleanupStorage(boolean recurring) { cleanupSecondaryStorage(recurring); - // ROOT volumes will be destroyed as part of VM cleanup - List vols = _volsDao.listNonRootVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long) StorageCleanupDelay.value() << 10))); + List vols = _volsDao.listVolumesToBeDestroyed(new Date(System.currentTimeMillis() - ((long) StorageCleanupDelay.value() << 10))); for (VolumeVO vol : vols) { try { // If this fails, just log a warning. It's ideal if we clean up the host-side clustered file