Updated Branches: refs/heads/object_store 3c7fb2f79 -> c614c6a42
CLOUDSTACK-2674: Secondary Storage garbage collector failed with NPE in case of S3 storage provider. Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/c614c6a4 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/c614c6a4 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/c614c6a4 Branch: refs/heads/object_store Commit: c614c6a424f33f582e6ce4a0e7bf4d72f5606bda Parents: 3c7fb2f Author: Min Chen <[email protected]> Authored: Fri May 24 11:10:45 2013 -0700 Committer: Min Chen <[email protected]> Committed: Fri May 24 11:10:45 2013 -0700 ---------------------------------------------------------------------- .../storage/image/TemplateServiceImpl.java | 7 ++- .../src/com/cloud/storage/StorageManagerImpl.java | 2 +- server/src/com/cloud/template/TemplateManager.java | 2 +- .../com/cloud/template/TemplateManagerImpl.java | 35 +++------------ 4 files changed, 13 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c614c6a4/engine/storage/image/src/org/apache/cloudstack/storage/image/TemplateServiceImpl.java ---------------------------------------------------------------------- diff --git a/engine/storage/image/src/org/apache/cloudstack/storage/image/TemplateServiceImpl.java b/engine/storage/image/src/org/apache/cloudstack/storage/image/TemplateServiceImpl.java index eb6e44c..fd34971 100644 --- a/engine/storage/image/src/org/apache/cloudstack/storage/image/TemplateServiceImpl.java +++ b/engine/storage/image/src/org/apache/cloudstack/storage/image/TemplateServiceImpl.java @@ -87,6 +87,7 @@ import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.download.DownloadMonitor; import com.cloud.storage.template.TemplateConstants; import com.cloud.storage.template.TemplateProp; +import com.cloud.template.TemplateManager; import com.cloud.user.AccountManager; import com.cloud.user.ResourceLimitService; import com.cloud.utils.UriUtils; @@ -138,6 +139,8 @@ public class TemplateServiceImpl implements TemplateService { EndPointSelector _epSelector; @Inject ImageDataManager imageMgr; + @Inject + TemplateManager _tmpltMgr; class TemplateOpContext<T> extends AsyncRpcConext<T> { @@ -430,9 +433,7 @@ public class TemplateServiceImpl implements TemplateService { for (String uniqueName : templateInfos.keySet()) { TemplateProp tInfo = templateInfos.get(uniqueName); - List<UserVmJoinVO> userVmUsingIso = _userVmJoinDao.listActiveByIsoId(tInfo.getId()); - //check if there is any Vm using this ISO. - if (userVmUsingIso == null || userVmUsingIso.isEmpty()) { + if (_tmpltMgr.templateIsDeleteable(tInfo.getId())) { //TODO: we cannot directly call deleteTemplateSync here to reuse delete logic since in this case, our db does not have this template at all. VMTemplateVO template = _templateDao.findById(tInfo.getId()); DeleteTemplateCommand dtCommand = new DeleteTemplateCommand(store.getTO(), tInfo.getInstallPath(), null, null); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c614c6a4/server/src/com/cloud/storage/StorageManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index 0313b49..cbbbe94 100755 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -1133,7 +1133,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C s_logger.debug("Secondary storage garbage collector found " + destroyedTemplateStoreVOs.size() + " templates to cleanup on secondary storage host: " + store.getName()); for (TemplateDataStoreVO destroyedTemplateStoreVO : destroyedTemplateStoreVOs) { - if (!_tmpltMgr.templateIsDeleteable(destroyedTemplateStoreVO)) { + if (!_tmpltMgr.templateIsDeleteable(destroyedTemplateStoreVO.getTemplateId())) { if (s_logger.isDebugEnabled()) { s_logger.debug("Not deleting template at: " + destroyedTemplateStoreVO); } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c614c6a4/server/src/com/cloud/template/TemplateManager.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/template/TemplateManager.java b/server/src/com/cloud/template/TemplateManager.java index 202feef..af71d30 100755 --- a/server/src/com/cloud/template/TemplateManager.java +++ b/server/src/com/cloud/template/TemplateManager.java @@ -92,7 +92,7 @@ public interface TemplateManager extends TemplateApiService{ boolean templateIsDeleteable(VMTemplateHostVO templateHostRef); - boolean templateIsDeleteable(TemplateDataStoreVO templateStoreRef); + boolean templateIsDeleteable(long templateId); Pair<String, String> getAbsoluteIsoPath(long templateId, long dataCenterId); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c614c6a4/server/src/com/cloud/template/TemplateManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/template/TemplateManagerImpl.java b/server/src/com/cloud/template/TemplateManagerImpl.java index eaf9d25..c828773 100755 --- a/server/src/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/com/cloud/template/TemplateManagerImpl.java @@ -945,35 +945,15 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, } @Override - public boolean templateIsDeleteable(TemplateDataStoreVO templateStoreRef) { - VMTemplateVO template = _tmpltDao.findByIdIncludingRemoved(templateStoreRef.getTemplateId()); - long templateId = template.getId(); - ImageStoreVO imageStore = _imageStoreDao.findById(templateStoreRef.getDataStoreId()); - long zoneId = imageStore.getDataCenterId(); - DataCenterVO zone = _dcDao.findById(zoneId); - - // Check if there are VMs running in the template host ref's zone that use the template - List<VMInstanceVO> nonExpungedVms = _vmInstanceDao.listNonExpungedByZoneAndTemplate(zoneId, templateId); - - if (!nonExpungedVms.isEmpty()) { - s_logger.debug("Template " + template.getName() + " in zone " + zone.getName() + " is not deleteable because there are non-expunged VMs deployed from this template."); - return false; - } - List<UserVmVO> userVmUsingIso = _userVmDao.listByIsoId(templateId); - //check if there is any VM using this ISO. + public boolean templateIsDeleteable(long templateId) { + List<UserVmJoinVO> userVmUsingIso = _userVmJoinDao.listActiveByIsoId(templateId); + //check if there is any Vm using this ISO. We only need to check the case where templateId is an ISO since + // VM can be launched from ISO in secondary storage, while template will always be copied to + // primary storage before deploying VM. if (!userVmUsingIso.isEmpty()) { - s_logger.debug("ISO " + template.getName() + " in zone " + zone.getName() + " is not deleteable because it is attached to " + userVmUsingIso.size() + " VMs"); + s_logger.debug("ISO " + templateId + " is not deleteable because it is attached to " + userVmUsingIso.size() + " VMs"); return false; } - // Check if there are any snapshots for the template in the template host ref's zone - List<VolumeVO> volumes = _volumeDao.findByTemplateAndZone(templateId, zoneId); - for (VolumeVO volume : volumes) { - List<SnapshotVO> snapshots = _snapshotDao.listByVolumeIdVersion(volume.getId(), "2.1"); - if (!snapshots.isEmpty()) { - s_logger.debug("Template " + template.getName() + " in zone " + zone.getName() + " is not deleteable because there are 2.1 snapshots using this template."); - return false; - } - } return true; } @@ -1153,9 +1133,8 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager, throw new InvalidParameterValueException("Please specify a valid iso."); } - List<UserVmJoinVO> userVmUsingIso = _userVmJoinDao.listActiveByIsoId(templateId); // check if there is any VM using this ISO. - if (userVmUsingIso != null && !userVmUsingIso.isEmpty()) { + if (!templateIsDeleteable(templateId)) { throw new InvalidParameterValueException("Unable to delete iso, as it's used by other vms"); }
