You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by wi...@apache.org on 2013/02/25 15:44:57 UTC

[21/28] git commit: refs/heads/qemu-img - CLOUDSTACK-357 ISOs can be deleted while still attached to a running VM, and they subsequently cannot be detached from a running VM

CLOUDSTACK-357 ISOs can be deleted while still attached to a running VM, and they subsequently cannot be detached from a running VM

I made the changes to make sure that:
1. ISO will be deleted from the UI, but it is not deleted from the secondary storage as long as it is attached to a VM.
2. The storage cleanup thread will check whether the iso is attached to any vm, if not, it removes the ISO from the secondary storage.
3. Detach operation is now working which was failing before for the vms having attached iso(deleted).

Updated the patch for template sync during MS restart.

Manually tested the following:
setup: upload ISO1 and ISO 2
Attach ISO1 to VM1 and VM2
Attach ISO2 to VM3
set storage.cleanup.interval to 300

test cases:
1. delete ISO1 from UI, gets deleted
2. In VM Details of VM1 and VM2, can see detach ISO option
3. ISO1 exists in secondary storage
4. detach ISO1 from VM1, successful
5. ISO1 still exists in secondary storage.
6. Restart MS, template sync will not delete ISO1.
7. Detach ISO1 from VM2, successfull detached.
8. Wait for storage cleanup thread to execute, ISO1 gets deleted from Secondary storage.
9. Detach ISO2 from VM3
10.ISO2 exists in secondary storage, Delete ISO2 form UI, get deleted from secondary storage.


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

Branch: refs/heads/qemu-img
Commit: 4d573ddd1bcd4ab27edfb91791a830308236521b
Parents: fe9df21
Author: Deepti Dohare <de...@citrix.com>
Authored: Mon Feb 25 16:01:48 2013 +0530
Committer: Nitin Mehta <ni...@citrix.com>
Committed: Mon Feb 25 16:02:25 2013 +0530

----------------------------------------------------------------------
 .../cloud/storage/dao/VMTemplateHostDaoImpl.java   |    1 -
 .../storage/download/DownloadMonitorImpl.java      |   28 +++++---
 .../cloud/template/HyervisorTemplateAdapter.java   |   21 ++++--
 .../com/cloud/template/TemplateAdapterBase.java    |    2 +
 .../com/cloud/template/TemplateManagerImpl.java    |   57 ++++++++-------
 server/src/com/cloud/vm/dao/UserVmDao.java         |    2 +
 server/src/com/cloud/vm/dao/UserVmDaoImpl.java     |   35 ++++++---
 7 files changed, 89 insertions(+), 57 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d573ddd/server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java b/server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java
index 4d19bfc..7f35eab 100755
--- a/server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java
+++ b/server/src/com/cloud/storage/dao/VMTemplateHostDaoImpl.java
@@ -258,7 +258,6 @@ public class VMTemplateHostDaoImpl extends GenericDaoBase<VMTemplateHostVO, Long
         sc.setParameters("template_id", templateId);
         sc.setParameters("host_id", hostId);
         sc.setParameters("states", (Object[])states);
-        sc.setParameters("destroyed", false);
         return search(sc, null);
     }
 	   

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d573ddd/server/src/com/cloud/storage/download/DownloadMonitorImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/download/DownloadMonitorImpl.java b/server/src/com/cloud/storage/download/DownloadMonitorImpl.java
index e12bc32..1fd1996 100755
--- a/server/src/com/cloud/storage/download/DownloadMonitorImpl.java
+++ b/server/src/com/cloud/storage/download/DownloadMonitorImpl.java
@@ -108,8 +108,10 @@ import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.SecondaryStorageVm;
 import com.cloud.vm.SecondaryStorageVmVO;
 import com.cloud.vm.UserVmManager;
+import com.cloud.vm.UserVmVO;
 import com.cloud.vm.VirtualMachine.State;
 import com.cloud.vm.dao.SecondaryStorageVmDao;
+import com.cloud.vm.dao.UserVmDao;
 
 import edu.emory.mathcs.backport.java.util.Collections;
 
@@ -173,6 +175,8 @@ public class DownloadMonitorImpl extends ManagerBase implements  DownloadMonitor
     private SwiftDao _swiftDao;
     @Inject
     protected ResourceLimitService _resourceLimitMgr;
+    @Inject
+    protected UserVmDao _userVmDao;
 
 	private Boolean _sslCopy = new Boolean(false);
 	private String _copyAuthPasswd;
@@ -927,17 +931,21 @@ public class DownloadMonitorImpl extends ManagerBase implements  DownloadMonitor
 
         for (String uniqueName : templateInfos.keySet()) {
             TemplateInfo tInfo = templateInfos.get(uniqueName);
-            DeleteTemplateCommand dtCommand = new DeleteTemplateCommand(ssHost.getStorageUrl(), tInfo.getInstallPath());
-            try {
-	            _agentMgr.sendToSecStorage(ssHost, dtCommand, null);
-            } catch (AgentUnavailableException e) {
-                String err = "Failed to delete " + tInfo.getTemplateName() + " on secondary storage " + sserverId + " which isn't in the database";
-                s_logger.error(err);
-                return;
-            }
+            List<UserVmVO> userVmUsingIso = _userVmDao.listByIsoId(tInfo.getId());
+            //check if there is any Vm using this ISO.
+            if (userVmUsingIso == null || userVmUsingIso.isEmpty()) {
+                DeleteTemplateCommand dtCommand = new DeleteTemplateCommand(ssHost.getStorageUrl(), tInfo.getInstallPath());
+                try {
+                    _agentMgr.sendToSecStorage(ssHost, dtCommand, null);
+                } catch (AgentUnavailableException e) {
+                    String err = "Failed to delete " + tInfo.getTemplateName() + " on secondary storage " + sserverId + " which isn't in the database";
+                    s_logger.error(err);
+                    return;
+                }
 
-            String description = "Deleted template " + tInfo.getTemplateName() + " on secondary storage " + sserverId + " since it isn't in the database";
-            s_logger.info(description);
+                String description = "Deleted template " + tInfo.getTemplateName() + " on secondary storage " + sserverId + " since it isn't in the database";
+                s_logger.info(description);
+            }
         }
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d573ddd/server/src/com/cloud/template/HyervisorTemplateAdapter.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/template/HyervisorTemplateAdapter.java b/server/src/com/cloud/template/HyervisorTemplateAdapter.java
index c1177f4..ad41af5 100755
--- a/server/src/com/cloud/template/HyervisorTemplateAdapter.java
+++ b/server/src/com/cloud/template/HyervisorTemplateAdapter.java
@@ -63,6 +63,8 @@ import com.cloud.storage.secondary.SecondaryStorageVmManager;
 import com.cloud.user.Account;
 import com.cloud.utils.db.DB;
 import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.vm.UserVmVO;
+
 import org.apache.cloudstack.api.command.user.iso.DeleteIsoCmd;
 import org.apache.cloudstack.api.command.user.iso.RegisterIsoCmd;
 import org.apache.cloudstack.api.command.user.template.DeleteTemplateCmd;
@@ -86,7 +88,6 @@ public class HyervisorTemplateAdapter extends TemplateAdapterBase implements Tem
     @Inject ImageDataFactory imageFactory;
     @Inject TemplateManager templateMgr;
 
-
     @Override
     public String getName() {
         return TemplateAdapterType.Hypervisor.getName();
@@ -248,17 +249,21 @@ public class HyervisorTemplateAdapter extends TemplateAdapterBase implements Tem
                     templateHostVO.setDestroyed(true);
 					_tmpltHostDao.update(templateHostVO.getId(), templateHostVO);
                     String installPath = templateHostVO.getInstallPath();
-                    if (installPath != null) {
-                        Answer answer = _agentMgr.sendToSecStorage(secondaryStorageHost, new DeleteTemplateCommand(secondaryStorageHost.getStorageUrl(), installPath));
+                    List<UserVmVO> userVmUsingIso = _userVmDao.listByIsoId(templateId);
+                    //check if there is any VM using this ISO.
+                    if (userVmUsingIso == null || userVmUsingIso.isEmpty()) {
+                        if (installPath != null) {
+                            Answer answer = _agentMgr.sendToSecStorage(secondaryStorageHost, new DeleteTemplateCommand(secondaryStorageHost.getStorageUrl(), installPath));
 
-                        if (answer == null || !answer.getResult()) {
-                            s_logger.debug("Failed to delete " + templateHostVO + " due to " + ((answer == null) ? "answer is null" : answer.getDetails()));
+                            if (answer == null || !answer.getResult()) {
+                                s_logger.debug("Failed to delete " + templateHostVO + " due to " + ((answer == null) ? "answer is null" : answer.getDetails()));
+                            } else {
+                                _tmpltHostDao.remove(templateHostVO.getId());
+                                s_logger.debug("Deleted template at: " + installPath);
+                            }
                         } else {
                             _tmpltHostDao.remove(templateHostVO.getId());
-                            s_logger.debug("Deleted template at: " + installPath);
                         }
-                    } else {
-                        _tmpltHostDao.remove(templateHostVO.getId());
                     }
 					VMTemplateZoneVO templateZone = _tmpltZoneDao.findByZoneTemplate(sZoneId, templateId);
 					

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d573ddd/server/src/com/cloud/template/TemplateAdapterBase.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/template/TemplateAdapterBase.java b/server/src/com/cloud/template/TemplateAdapterBase.java
index 247ce63..1b11425 100755
--- a/server/src/com/cloud/template/TemplateAdapterBase.java
+++ b/server/src/com/cloud/template/TemplateAdapterBase.java
@@ -63,6 +63,7 @@ import com.cloud.utils.EnumUtils;
 import com.cloud.utils.component.AdapterBase;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.vm.UserVmVO;
+import com.cloud.vm.dao.UserVmDao;
 
 public abstract class TemplateAdapterBase extends AdapterBase implements TemplateAdapter {
 	private final static Logger s_logger = Logger.getLogger(TemplateAdapterBase.class);
@@ -77,6 +78,7 @@ public abstract class TemplateAdapterBase extends AdapterBase implements Templat
 	protected @Inject VMTemplateZoneDao _tmpltZoneDao;
 	protected @Inject UsageEventDao _usageEventDao;
 	protected @Inject HostDao _hostDao;
+	protected @Inject UserVmDao _userVmDao;
 	protected @Inject ResourceLimitService _resourceLimitMgr;
 	protected @Inject DataStoreManager storeMgr;
 	@Inject TemplateManager templateMgr;

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d573ddd/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 101c3d9..29659d3 100755
--- a/server/src/com/cloud/template/TemplateManagerImpl.java
+++ b/server/src/com/cloud/template/TemplateManagerImpl.java
@@ -1227,32 +1227,37 @@ public class TemplateManagerImpl extends ManagerBase implements TemplateManager,
     }
 
 	@Override
-    public boolean templateIsDeleteable(VMTemplateHostVO templateHostRef) {
-		VMTemplateVO template = _tmpltDao.findByIdIncludingRemoved(templateHostRef.getTemplateId());
-		long templateId = template.getId();
-		HostVO secondaryStorageHost = _hostDao.findById(templateHostRef.getHostId());
-		long zoneId = secondaryStorageHost.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;
-		}
-		
-		// 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;
+	public boolean templateIsDeleteable(VMTemplateHostVO templateHostRef) {
+	    VMTemplateVO template = _tmpltDao.findByIdIncludingRemoved(templateHostRef.getTemplateId());
+	    long templateId = template.getId();
+	    HostVO secondaryStorageHost = _hostDao.findById(templateHostRef.getHostId());
+	    long zoneId = secondaryStorageHost.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.
+	    if (!userVmUsingIso.isEmpty()) {
+	        s_logger.debug("ISO " + template.getName() + " in zone " + zone.getName() + " 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;
 	}
 
 	@Override

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d573ddd/server/src/com/cloud/vm/dao/UserVmDao.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/dao/UserVmDao.java b/server/src/com/cloud/vm/dao/UserVmDao.java
index 9fbcde3..81d13cd 100755
--- a/server/src/com/cloud/vm/dao/UserVmDao.java
+++ b/server/src/com/cloud/vm/dao/UserVmDao.java
@@ -70,5 +70,7 @@ public interface UserVmDao extends GenericDao<UserVmVO, Long> {
     public Long countAllocatedVMsForAccount(long accountId);
 
     Hashtable<Long, UserVmData> listVmDetails(Hashtable<Long, UserVmData> userVmData);
+
+    List<UserVmVO> listByIsoId(Long isoId);
    
 }

http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d573ddd/server/src/com/cloud/vm/dao/UserVmDaoImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/dao/UserVmDaoImpl.java b/server/src/com/cloud/vm/dao/UserVmDaoImpl.java
index f2fc10b..02604fe 100755
--- a/server/src/com/cloud/vm/dao/UserVmDaoImpl.java
+++ b/server/src/com/cloud/vm/dao/UserVmDaoImpl.java
@@ -72,10 +72,11 @@ public class UserVmDaoImpl extends GenericDaoBase<UserVmVO, Long> implements Use
     protected SearchBuilder<UserVmVO> DestroySearch;
     protected SearchBuilder<UserVmVO> AccountDataCenterVirtualSearch;
     protected GenericSearchBuilder<UserVmVO, Long> CountByAccountPod;
-    protected GenericSearchBuilder<UserVmVO, Long> CountByAccount;
-    protected GenericSearchBuilder<UserVmVO, Long> PodsHavingVmsForAccount;
-    
-    protected SearchBuilder<UserVmVO> UserVmSearch;
+    protected GenericSearchBuilder<UserVmVO, Long> CountByAccount;
+    protected GenericSearchBuilder<UserVmVO, Long> PodsHavingVmsForAccount;
+
+    protected SearchBuilder<UserVmVO> UserVmSearch;
+    protected SearchBuilder<UserVmVO> UserVmByIsoSearch;
     protected Attribute _updateTimeAttr;
     // ResourceTagsDaoImpl _tagsDao = ComponentLocator.inject(ResourceTagsDaoImpl.class);
     @Inject ResourceTagsDaoImpl _tagsDao;
@@ -194,7 +195,10 @@ public class UserVmDaoImpl extends GenericDaoBase<UserVmVO, Long> implements Use
         AccountDataCenterVirtualSearch.and("dc", AccountDataCenterVirtualSearch.entity().getDataCenterId(), SearchCriteria.Op.EQ);
         AccountDataCenterVirtualSearch.join("nicSearch", nicSearch, AccountDataCenterVirtualSearch.entity().getId(), nicSearch.entity().getInstanceId(), JoinBuilder.JoinType.INNER);
         AccountDataCenterVirtualSearch.done();
-       
+
+        UserVmByIsoSearch = createSearchBuilder();
+        UserVmByIsoSearch.and("isoId", UserVmByIsoSearch.entity().getIsoId(), SearchCriteria.Op.EQ);
+        UserVmByIsoSearch.done();
 
         _updateTimeAttr = _allAttributes.get("updateTime");
         assert _updateTimeAttr != null : "Couldn't get this updateTime attribute";
@@ -248,13 +252,20 @@ public class UserVmDaoImpl extends GenericDaoBase<UserVmVO, Long> implements Use
     public List<UserVmVO> listByHostId(Long id) {
         SearchCriteria<UserVmVO> sc = HostSearch.create();
         sc.setParameters("host", id);
-        
-        return listBy(sc);
-    }
-    
-    @Override
-    public List<UserVmVO> listUpByHostId(Long hostId) {
-        SearchCriteria<UserVmVO> sc = HostUpSearch.create();
+
+        return listBy(sc);
+    }
+
+    @Override
+    public List<UserVmVO> listByIsoId(Long isoId) {
+        SearchCriteria<UserVmVO> sc = UserVmByIsoSearch.create();
+        sc.setParameters("isoId", isoId);
+        return listBy(sc);
+    }
+
+    @Override
+    public List<UserVmVO> listUpByHostId(Long hostId) {
+        SearchCriteria<UserVmVO> sc = HostUpSearch.create();
         sc.setParameters("host", hostId);
         sc.setParameters("states", new Object[] {State.Destroyed, State.Stopped, State.Expunging});
         return listBy(sc);