From 838d01754b8ea3d2261bb17b2a2abfaac4873965 Mon Sep 17 00:00:00 2001
From: Nitin Mehta <nitin.mehta@citrix.com>
Date: Fri, 11 May 2012 11:38:07 +0530
Subject: [PATCH] bug CS-14785: Improvements to volumes sync so that it can
 recover from any error. Introduced new state UploadOp which signifies that
 the volume os on secondary and the status would be picked up from volume host
 ref table.

---
 .../agent/api/storage/DownloadCommand.java    |  1 -
 api/src/com/cloud/storage/Volume.java         | 15 ++------
 .../src/com/cloud/api/ApiResponseHelper.java  | 11 ++++--
 .../com/cloud/storage/StorageManagerImpl.java | 11 +++---
 .../storage/download/DownloadMonitorImpl.java | 35 ++++++-------------
 .../src/com/cloud/vm/UserVmManagerImpl.java   | 15 ++++----
 6 files changed, 36 insertions(+), 52 deletions(-)

diff --git a/api/src/com/cloud/agent/api/storage/DownloadCommand.java b/api/src/com/cloud/agent/api/storage/DownloadCommand.java
index cef9920412d..bc58c20bc91 100644
--- a/api/src/com/cloud/agent/api/storage/DownloadCommand.java
+++ b/api/src/com/cloud/agent/api/storage/DownloadCommand.java
@@ -135,7 +135,6 @@ public class DownloadCommand extends AbstractDownloadCommand {
 	    //this.hvm = volume.isRequiresHvm();
 	    this.checksum = checkSum;
 	    this.id = volume.getId();
-	    //this.description = volume.get;
 	    this.setSecUrl(secUrl);
 	    this.maxDownloadSizeInBytes = maxDownloadSizeInBytes;
 	    this.resourceType = ResourceType.VOLUME;
diff --git a/api/src/com/cloud/storage/Volume.java b/api/src/com/cloud/storage/Volume.java
index f31454e66c2..1c43ee85001 100755
--- a/api/src/com/cloud/storage/Volume.java
+++ b/api/src/com/cloud/storage/Volume.java
@@ -32,9 +32,7 @@ public interface Volume extends ControlledEntity, BasedOn, StateObject<Volume.St
         Snapshotting("There is a snapshot created on this volume, not backed up to secondary storage yet"),
         Expunging("The volume is being expunging"),
         Destroy("The volume is destroyed, and can't be recovered."),        
-        Uploading ("The volume upload is in progress"),
-        Uploaded ("The volume is uploaded and present on secondary storage"),
-        UploadError ("The volume couldnt be uploaded");
+        UploadOp ("The volume upload operation is in progress");            
 
         String _description;
 
@@ -59,15 +57,9 @@ public interface Volume extends ControlledEntity, BasedOn, StateObject<Volume.St
             s_fsm.addTransition(Creating, Event.OperationSucceeded, Ready);
             s_fsm.addTransition(Creating, Event.DestroyRequested, Destroy);
             s_fsm.addTransition(Creating, Event.CreateRequested, Creating);            
-            s_fsm.addTransition(Allocated, Event.UploadRequested, Uploading);
-            s_fsm.addTransition(Uploading, Event.UploadSucceeded, Uploaded);
-            s_fsm.addTransition(Uploading, Event.OperationFailed, UploadError);
-            s_fsm.addTransition(UploadError, Event.DestroyRequested, Destroy);
-            s_fsm.addTransition(Uploaded, Event.UploadSucceeded, Uploaded);
-            s_fsm.addTransition(Uploaded, Event.CopyRequested, Creating);
-            s_fsm.addTransition(Uploaded, Event.DestroyRequested, Destroy);
+            s_fsm.addTransition(Allocated, Event.UploadRequested, UploadOp);         
             s_fsm.addTransition(Creating, Event.CopySucceeded, Ready);
-            s_fsm.addTransition(Creating, Event.CopyFailed, Uploaded);
+            s_fsm.addTransition(UploadOp, Event.CopySucceeded, Ready);
             s_fsm.addTransition(Ready, Event.DestroyRequested, Destroy);
             s_fsm.addTransition(Destroy, Event.ExpungingRequested, Expunging);
             s_fsm.addTransition(Ready, Event.SnapshotRequested, Snapshotting);
@@ -89,7 +81,6 @@ public interface Volume extends ControlledEntity, BasedOn, StateObject<Volume.St
         OperationSucceeded,
         OperationRetry,
         UploadRequested,
-        UploadSucceeded,
         MigrationRequested,
         SnapshotRequested,
         DestroyRequested,
diff --git a/server/src/com/cloud/api/ApiResponseHelper.java b/server/src/com/cloud/api/ApiResponseHelper.java
index 6376c063aca..9cc1a8844d7 100755
--- a/server/src/com/cloud/api/ApiResponseHelper.java
+++ b/server/src/com/cloud/api/ApiResponseHelper.java
@@ -1019,7 +1019,7 @@ public class ApiResponseHelper implements ResponseGenerator {
 
         volResponse.setCreated(volume.getCreated());
         volResponse.setState(volume.getState().toString());
-        if(volume.getState() == Volume.State.Uploading || volume.getState() == Volume.State.Uploaded){
+        if(volume.getState() == Volume.State.UploadOp){
         	com.cloud.storage.VolumeHostVO volumeHostRef = ApiDBUtils.findVolumeHostRef(volume.getId(), volume.getDataCenterId());
             volResponse.setSize(volumeHostRef.getSize());
             volResponse.setCreated(volumeHostRef.getCreated());
@@ -1031,12 +1031,19 @@ public class ApiResponseHelper implements ResponseGenerator {
                     } else {
                         volumeStatus = volumeHostRef.getDownloadPercent() + "% Uploaded";
                     }
+                    volResponse.setState("Uploading");
                 } else {
                     volumeStatus = volumeHostRef.getErrorString();
+                    if(volumeHostRef.getDownloadState() == VMTemplateHostVO.Status.NOT_DOWNLOADED){
+                    	volResponse.setState("UploadNotStarted");
+                    }else {
+                    	volResponse.setState("UploadError");
+                    }
                 }
                 volResponse.setStatus(volumeStatus);
             } else if (volumeHostRef.getDownloadState() == VMTemplateHostVO.Status.DOWNLOADED) {
             	volResponse.setStatus("Upload Complete");
+            	volResponse.setState("Uploaded");
             } else {
             	volResponse.setStatus("Successfully Installed");
             }            
@@ -1047,7 +1054,7 @@ public class ApiResponseHelper implements ResponseGenerator {
         String storageType;
         try {
             if (volume.getPoolId() == null) {
-                if (volume.getState() == Volume.State.Allocated || volume.getState() == Volume.State.Uploaded || volume.getState() == Volume.State.Uploading) {
+                if (volume.getState() == Volume.State.Allocated || volume.getState() == Volume.State.UploadOp) {
                     /* set it as shared, so the UI can attach it to VM */
                     storageType = "shared";
                 } else {
diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java
index 87012fbb8d5..739f1252c3a 100755
--- a/server/src/com/cloud/storage/StorageManagerImpl.java
+++ b/server/src/com/cloud/storage/StorageManagerImpl.java
@@ -724,8 +724,7 @@ public class StorageManagerImpl implements StorageManager, Manager, ClusterManag
     	// Find a suitable storage to create volume on 
     	StoragePoolVO destPool = findStoragePool(dskCh, dc, pod, clusterId, vm, avoidPools);
     	
-    	// Copy the volume from secondary storage to the destination storage pool
-    	stateTransitTo(volume, Event.CopyRequested);    	
+    	// Copy the volume from secondary storage to the destination storage pool    	  	
     	VolumeHostVO volumeHostVO = _volumeHostDao.findByVolumeId(volume.getId());
     	HostVO secStorage = _hostDao.findById(volumeHostVO.getHostId());
     	String secondaryStorageURL = secStorage.getStorageUrl();
@@ -1754,10 +1753,10 @@ public class StorageManagerImpl implements StorageManager, Manager, ClusterManag
 		        throw new InvalidParameterValueException("Please specify a valid " + format.toLowerCase());
 		    }
 			
-			if ((format.equalsIgnoreCase("vhd") && (!url.toLowerCase().endsWith("vhd") && !url.toLowerCase().endsWith("vhd.zip") && !url.toLowerCase().endsWith("vhd.bz2") && !url.toLowerCase().endsWith("vhd.gz") ))
-				|| (format.equalsIgnoreCase("qcow2") && (!url.toLowerCase().endsWith("qcow2") && !url.toLowerCase().endsWith("qcow2.zip") && !url.toLowerCase().endsWith("qcow2.bz2") && !url.toLowerCase().endsWith("qcow2.gz") ))
-				|| (format.equalsIgnoreCase("ova") && (!url.toLowerCase().endsWith("ova") && !url.toLowerCase().endsWith("ova.zip") && !url.toLowerCase().endsWith("ova.bz2") && !url.toLowerCase().endsWith("ova.gz")))
-				|| (format.equalsIgnoreCase("raw") && (!url.toLowerCase().endsWith("img") && !url.toLowerCase().endsWith("raw")))) {
+			if ((format.equalsIgnoreCase("vhd") && (!url.toLowerCase().endsWith(".vhd") && !url.toLowerCase().endsWith("vhd.zip") && !url.toLowerCase().endsWith("vhd.bz2") && !url.toLowerCase().endsWith("vhd.gz") ))
+				|| (format.equalsIgnoreCase("qcow2") && (!url.toLowerCase().endsWith(".qcow2") && !url.toLowerCase().endsWith("qcow2.zip") && !url.toLowerCase().endsWith("qcow2.bz2") && !url.toLowerCase().endsWith("qcow2.gz") ))
+				|| (format.equalsIgnoreCase("ova") && (!url.toLowerCase().endsWith(".ova") && !url.toLowerCase().endsWith("ova.zip") && !url.toLowerCase().endsWith("ova.bz2") && !url.toLowerCase().endsWith("ova.gz")))
+				|| (format.equalsIgnoreCase("raw") && (!url.toLowerCase().endsWith(".img") && !url.toLowerCase().endsWith("raw")))) {
 		        throw new InvalidParameterValueException("Please specify a valid URL. URL:" + url + " is an invalid for the format " + format.toLowerCase());
 			}
         validateUrl(url);
diff --git a/server/src/com/cloud/storage/download/DownloadMonitorImpl.java b/server/src/com/cloud/storage/download/DownloadMonitorImpl.java
index fb42f4fffbb..03142a90a27 100755
--- a/server/src/com/cloud/storage/download/DownloadMonitorImpl.java
+++ b/server/src/com/cloud/storage/download/DownloadMonitorImpl.java
@@ -542,13 +542,6 @@ public class DownloadMonitorImpl implements  DownloadMonitor {
         txn.start();		
 
         if (dnldStatus == Status.DOWNLOADED) {
-        	
-        	// Do the volume state transition
-			try {
-				_storageMgr.stateTransitTo(volume, Event.UploadSucceeded);			
-			} catch (NoTransitionException e) {
-				e.printStackTrace();
-			}
 			
 			//Create usage event
             long size = -1;
@@ -563,13 +556,7 @@ public class DownloadMonitorImpl implements  DownloadMonitor {
                UsageEventVO usageEvent = new UsageEventVO(eventType, volume.getAccountId(), host.getDataCenterId(), volume.getId(), volume.getName(), null, 0l , size);
                _usageEventDao.persist(usageEvent);
             }
-        }else if (dnldStatus == Status.DOWNLOAD_ERROR){
-        	//Transistion the volume state
-        	try {
-                _storageMgr.stateTransitTo(volume, Volume.Event.OperationFailed);
-            } catch (NoTransitionException e) {
-                throw new CloudRuntimeException("Unable to update the failure on a volume: " + volume, e);
-            }
+        }else if (dnldStatus == Status.DOWNLOAD_ERROR || dnldStatus == Status.ABANDONED || dnldStatus == Status.UNKNOWN){
             //Decrement the volume count
         	_resourceLimitMgr.decrementResourceCount(volume.getAccountId(), com.cloud.configuration.Resource.ResourceType.volume);
         }
@@ -737,27 +724,25 @@ public class DownloadMonitorImpl implements  DownloadMonitor {
                         toBeDownloaded.add(volumeHost);
                     }
 
-                } else {
+                } else { // Put them in right status
                 	volumeHost.setDownloadPercent(100);
                 	volumeHost.setDownloadState(Status.DOWNLOADED);
                 	volumeHost.setInstallPath(volInfo.getInstallPath());
                 	volumeHost.setSize(volInfo.getSize());
                 	volumeHost.setPhysicalSize(volInfo.getPhysicalSize());
-                	volumeHost.setLastUpdated(new Date());
-                	if (volume.getState() == Volume.State.Uploading){
-                		try {
-                			_storageMgr.stateTransitTo(volume, Event.UploadSucceeded);			
-                		} catch (NoTransitionException e) {
-                			e.printStackTrace();
-                		}
-                	}
-                
+                	volumeHost.setLastUpdated(new Date());               
                 	_volumeHostDao.update(volumeHost.getId(), volumeHost);
                 }
+                continue;
         	}
+        	// Volume is not on secondary but we should download.
+        	if (volumeHost.getDownloadState() != Status.DOWNLOADED) {
+                s_logger.info("Volume Sync did not find " + volume.getName() + " ready on server " + sserverId + ", will request download to start/resume shortly");
+                toBeDownloaded.add(volumeHost);
+            } 
         }
         
-        //Download volumes which havent been downloaded yet.
+        //Download volumes which haven't been downloaded yet.
         if (toBeDownloaded.size() > 0) {
             for (VolumeHostVO volumeHost : toBeDownloaded) {
                 if (volumeHost.getDownloadUrl() == null) { // If url is null we can't initiate the download
diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java
index ee29d41e692..c7c28471cdb 100755
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -556,16 +556,19 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
         boolean volumeOnSec = false;
         VolumeHostVO  volHostVO = _volumeHostDao.findByVolumeId(volume.getId());
         if (volHostVO != null){
-        	volumeOnSec = true;       	
+        	volumeOnSec = true;
+        	if( !(volHostVO.getDownloadState() == Status.DOWNLOADED) ){
+        		throw new InvalidParameterValueException("Volume is not uploaded yet. Please try this operation once the volume is uploaded");	
+        	}
         }
 
-        // Check that the volume is stored on shared storage
-        if (!(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Uploaded.equals(volume.getState())) && !_storageMgr.volumeOnSharedStoragePool(volume)) {
+        //If the volume is Ready, check that the volume is stored on shared storage
+        if (!(Volume.State.Allocated.equals(volume.getState()) || Volume.State.UploadOp.equals(volume.getState())) && !_storageMgr.volumeOnSharedStoragePool(volume)) {
             throw new InvalidParameterValueException("Please specify a volume that has been created on a shared storage pool.");
         }
 
-        if ( !(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Ready.equals(volume.getState()) || Volume.State.Uploaded.equals(volume.getState())) ) {
-            throw new InvalidParameterValueException("Volume state must be in Allocated, Ready or Uploaded state");
+        if ( !(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Ready.equals(volume.getState())) ) {
+            throw new InvalidParameterValueException("Volume state must be in Allocated or Ready state");
         }
 
         VolumeVO rootVolumeOfVm = null;
@@ -578,7 +581,7 @@ public class UserVmManagerImpl implements UserVmManager, UserVmService, Manager
 
         HypervisorType rootDiskHyperType = _volsDao.getHypervisorType(rootVolumeOfVm.getId());
 
-        if (volume.getState().equals(Volume.State.Allocated) || volume.getState().equals(Volume.State.Uploaded)) {
+        if (volume.getState().equals(Volume.State.Allocated) || volume.getState().equals(Volume.State.UploadOp)) {
             /* Need to create the volume */
             VMTemplateVO rootDiskTmplt = _templateDao.findById(vm.getTemplateId());
             DataCenterVO dcVO = _dcDao.findById(vm.getDataCenterIdToDeployIn());