Skip to content

Commit 838d017

Browse files
nitinmehnitinmeh
committed
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.
1 parent 562dd29 commit 838d017

6 files changed

Lines changed: 36 additions & 52 deletions

File tree

‎api/src/com/cloud/agent/api/storage/DownloadCommand.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ public DownloadCommand(String secUrl, Volume volume, Long maxDownloadSizeInBytes
135135
//this.hvm = volume.isRequiresHvm();
136136
this.checksum = checkSum;
137137
this.id = volume.getId();
138-
//this.description = volume.get;
139138
this.setSecUrl(secUrl);
140139
this.maxDownloadSizeInBytes = maxDownloadSizeInBytes;
141140
this.resourceType = ResourceType.VOLUME;

‎api/src/com/cloud/storage/Volume.java‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ enum State {
3232
Snapshotting("There is a snapshot created on this volume, not backed up to secondary storage yet"),
3333
Expunging("The volume is being expunging"),
3434
Destroy("The volume is destroyed, and can't be recovered."),
35-
Uploading ("The volume upload is in progress"),
36-
Uploaded ("The volume is uploaded and present on secondary storage"),
37-
UploadError ("The volume couldnt be uploaded");
35+
UploadOp ("The volume upload operation is in progress");
3836

3937
String _description;
4038

@@ -59,15 +57,9 @@ public String getDescription() {
5957
s_fsm.addTransition(Creating, Event.OperationSucceeded, Ready);
6058
s_fsm.addTransition(Creating, Event.DestroyRequested, Destroy);
6159
s_fsm.addTransition(Creating, Event.CreateRequested, Creating);
62-
s_fsm.addTransition(Allocated, Event.UploadRequested, Uploading);
63-
s_fsm.addTransition(Uploading, Event.UploadSucceeded, Uploaded);
64-
s_fsm.addTransition(Uploading, Event.OperationFailed, UploadError);
65-
s_fsm.addTransition(UploadError, Event.DestroyRequested, Destroy);
66-
s_fsm.addTransition(Uploaded, Event.UploadSucceeded, Uploaded);
67-
s_fsm.addTransition(Uploaded, Event.CopyRequested, Creating);
68-
s_fsm.addTransition(Uploaded, Event.DestroyRequested, Destroy);
60+
s_fsm.addTransition(Allocated, Event.UploadRequested, UploadOp);
6961
s_fsm.addTransition(Creating, Event.CopySucceeded, Ready);
70-
s_fsm.addTransition(Creating, Event.CopyFailed, Uploaded);
62+
s_fsm.addTransition(UploadOp, Event.CopySucceeded, Ready);
7163
s_fsm.addTransition(Ready, Event.DestroyRequested, Destroy);
7264
s_fsm.addTransition(Destroy, Event.ExpungingRequested, Expunging);
7365
s_fsm.addTransition(Ready, Event.SnapshotRequested, Snapshotting);
@@ -89,7 +81,6 @@ enum Event {
8981
OperationSucceeded,
9082
OperationRetry,
9183
UploadRequested,
92-
UploadSucceeded,
9384
MigrationRequested,
9485
SnapshotRequested,
9586
DestroyRequested,

‎server/src/com/cloud/api/ApiResponseHelper.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1019,7 +1019,7 @@ public VolumeResponse createVolumeResponse(Volume volume) {
10191019

10201020
volResponse.setCreated(volume.getCreated());
10211021
volResponse.setState(volume.getState().toString());
1022-
if(volume.getState() == Volume.State.Uploading || volume.getState() == Volume.State.Uploaded){
1022+
if(volume.getState() == Volume.State.UploadOp){
10231023
com.cloud.storage.VolumeHostVO volumeHostRef = ApiDBUtils.findVolumeHostRef(volume.getId(), volume.getDataCenterId());
10241024
volResponse.setSize(volumeHostRef.getSize());
10251025
volResponse.setCreated(volumeHostRef.getCreated());
@@ -1031,12 +1031,19 @@ public VolumeResponse createVolumeResponse(Volume volume) {
10311031
} else {
10321032
volumeStatus = volumeHostRef.getDownloadPercent() + "% Uploaded";
10331033
}
1034+
volResponse.setState("Uploading");
10341035
} else {
10351036
volumeStatus = volumeHostRef.getErrorString();
1037+
if(volumeHostRef.getDownloadState() == VMTemplateHostVO.Status.NOT_DOWNLOADED){
1038+
volResponse.setState("UploadNotStarted");
1039+
}else {
1040+
volResponse.setState("UploadError");
1041+
}
10361042
}
10371043
volResponse.setStatus(volumeStatus);
10381044
} else if (volumeHostRef.getDownloadState() == VMTemplateHostVO.Status.DOWNLOADED) {
10391045
volResponse.setStatus("Upload Complete");
1046+
volResponse.setState("Uploaded");
10401047
} else {
10411048
volResponse.setStatus("Successfully Installed");
10421049
}
@@ -1047,7 +1054,7 @@ public VolumeResponse createVolumeResponse(Volume volume) {
10471054
String storageType;
10481055
try {
10491056
if (volume.getPoolId() == null) {
1050-
if (volume.getState() == Volume.State.Allocated || volume.getState() == Volume.State.Uploaded || volume.getState() == Volume.State.Uploading) {
1057+
if (volume.getState() == Volume.State.Allocated || volume.getState() == Volume.State.UploadOp) {
10511058
/* set it as shared, so the UI can attach it to VM */
10521059
storageType = "shared";
10531060
} else {

‎server/src/com/cloud/storage/StorageManagerImpl.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -724,8 +724,7 @@ public VolumeVO copyVolumeFromSecToPrimary(VolumeVO volume, VMInstanceVO vm, VMT
724724
// Find a suitable storage to create volume on
725725
StoragePoolVO destPool = findStoragePool(dskCh, dc, pod, clusterId, vm, avoidPools);
726726

727-
// Copy the volume from secondary storage to the destination storage pool
728-
stateTransitTo(volume, Event.CopyRequested);
727+
// Copy the volume from secondary storage to the destination storage pool
729728
VolumeHostVO volumeHostVO = _volumeHostDao.findByVolumeId(volume.getId());
730729
HostVO secStorage = _hostDao.findById(volumeHostVO.getHostId());
731730
String secondaryStorageURL = secStorage.getStorageUrl();
@@ -1754,10 +1753,10 @@ private boolean validateVolume(Account caller, long ownerId, Long zoneId, String
17541753
throw new InvalidParameterValueException("Please specify a valid " + format.toLowerCase());
17551754
}
17561755

1757-
if ((format.equalsIgnoreCase("vhd") && (!url.toLowerCase().endsWith("vhd") && !url.toLowerCase().endsWith("vhd.zip") && !url.toLowerCase().endsWith("vhd.bz2") && !url.toLowerCase().endsWith("vhd.gz") ))
1758-
|| (format.equalsIgnoreCase("qcow2") && (!url.toLowerCase().endsWith("qcow2") && !url.toLowerCase().endsWith("qcow2.zip") && !url.toLowerCase().endsWith("qcow2.bz2") && !url.toLowerCase().endsWith("qcow2.gz") ))
1759-
|| (format.equalsIgnoreCase("ova") && (!url.toLowerCase().endsWith("ova") && !url.toLowerCase().endsWith("ova.zip") && !url.toLowerCase().endsWith("ova.bz2") && !url.toLowerCase().endsWith("ova.gz")))
1760-
|| (format.equalsIgnoreCase("raw") && (!url.toLowerCase().endsWith("img") && !url.toLowerCase().endsWith("raw")))) {
1756+
if ((format.equalsIgnoreCase("vhd") && (!url.toLowerCase().endsWith(".vhd") && !url.toLowerCase().endsWith("vhd.zip") && !url.toLowerCase().endsWith("vhd.bz2") && !url.toLowerCase().endsWith("vhd.gz") ))
1757+
|| (format.equalsIgnoreCase("qcow2") && (!url.toLowerCase().endsWith(".qcow2") && !url.toLowerCase().endsWith("qcow2.zip") && !url.toLowerCase().endsWith("qcow2.bz2") && !url.toLowerCase().endsWith("qcow2.gz") ))
1758+
|| (format.equalsIgnoreCase("ova") && (!url.toLowerCase().endsWith(".ova") && !url.toLowerCase().endsWith("ova.zip") && !url.toLowerCase().endsWith("ova.bz2") && !url.toLowerCase().endsWith("ova.gz")))
1759+
|| (format.equalsIgnoreCase("raw") && (!url.toLowerCase().endsWith(".img") && !url.toLowerCase().endsWith("raw")))) {
17611760
throw new InvalidParameterValueException("Please specify a valid URL. URL:" + url + " is an invalid for the format " + format.toLowerCase());
17621761
}
17631762
validateUrl(url);

‎server/src/com/cloud/storage/download/DownloadMonitorImpl.java‎

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -542,13 +542,6 @@ public void handleDownloadEvent(HostVO host, VolumeVO volume, Status dnldStatus)
542542
txn.start();
543543

544544
if (dnldStatus == Status.DOWNLOADED) {
545-
546-
// Do the volume state transition
547-
try {
548-
_storageMgr.stateTransitTo(volume, Event.UploadSucceeded);
549-
} catch (NoTransitionException e) {
550-
e.printStackTrace();
551-
}
552545

553546
//Create usage event
554547
long size = -1;
@@ -563,13 +556,7 @@ public void handleDownloadEvent(HostVO host, VolumeVO volume, Status dnldStatus)
563556
UsageEventVO usageEvent = new UsageEventVO(eventType, volume.getAccountId(), host.getDataCenterId(), volume.getId(), volume.getName(), null, 0l , size);
564557
_usageEventDao.persist(usageEvent);
565558
}
566-
}else if (dnldStatus == Status.DOWNLOAD_ERROR){
567-
//Transistion the volume state
568-
try {
569-
_storageMgr.stateTransitTo(volume, Volume.Event.OperationFailed);
570-
} catch (NoTransitionException e) {
571-
throw new CloudRuntimeException("Unable to update the failure on a volume: " + volume, e);
572-
}
559+
}else if (dnldStatus == Status.DOWNLOAD_ERROR || dnldStatus == Status.ABANDONED || dnldStatus == Status.UNKNOWN){
573560
//Decrement the volume count
574561
_resourceLimitMgr.decrementResourceCount(volume.getAccountId(), com.cloud.configuration.Resource.ResourceType.volume);
575562
}
@@ -737,27 +724,25 @@ public void handleVolumeSync(HostVO ssHost) {
737724
toBeDownloaded.add(volumeHost);
738725
}
739726

740-
} else {
727+
} else { // Put them in right status
741728
volumeHost.setDownloadPercent(100);
742729
volumeHost.setDownloadState(Status.DOWNLOADED);
743730
volumeHost.setInstallPath(volInfo.getInstallPath());
744731
volumeHost.setSize(volInfo.getSize());
745732
volumeHost.setPhysicalSize(volInfo.getPhysicalSize());
746-
volumeHost.setLastUpdated(new Date());
747-
if (volume.getState() == Volume.State.Uploading){
748-
try {
749-
_storageMgr.stateTransitTo(volume, Event.UploadSucceeded);
750-
} catch (NoTransitionException e) {
751-
e.printStackTrace();
752-
}
753-
}
754-
733+
volumeHost.setLastUpdated(new Date());
755734
_volumeHostDao.update(volumeHost.getId(), volumeHost);
756735
}
736+
continue;
757737
}
738+
// Volume is not on secondary but we should download.
739+
if (volumeHost.getDownloadState() != Status.DOWNLOADED) {
740+
s_logger.info("Volume Sync did not find " + volume.getName() + " ready on server " + sserverId + ", will request download to start/resume shortly");
741+
toBeDownloaded.add(volumeHost);
742+
}
758743
}
759744

760-
//Download volumes which havent been downloaded yet.
745+
//Download volumes which haven't been downloaded yet.
761746
if (toBeDownloaded.size() > 0) {
762747
for (VolumeHostVO volumeHost : toBeDownloaded) {
763748
if (volumeHost.getDownloadUrl() == null) { // If url is null we can't initiate the download

‎server/src/com/cloud/vm/UserVmManagerImpl.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -556,16 +556,19 @@ public Volume attachVolumeToVM(AttachVolumeCmd command) {
556556
boolean volumeOnSec = false;
557557
VolumeHostVO volHostVO = _volumeHostDao.findByVolumeId(volume.getId());
558558
if (volHostVO != null){
559-
volumeOnSec = true;
559+
volumeOnSec = true;
560+
if( !(volHostVO.getDownloadState() == Status.DOWNLOADED) ){
561+
throw new InvalidParameterValueException("Volume is not uploaded yet. Please try this operation once the volume is uploaded");
562+
}
560563
}
561564

562-
// Check that the volume is stored on shared storage
563-
if (!(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Uploaded.equals(volume.getState())) && !_storageMgr.volumeOnSharedStoragePool(volume)) {
565+
//If the volume is Ready, check that the volume is stored on shared storage
566+
if (!(Volume.State.Allocated.equals(volume.getState()) || Volume.State.UploadOp.equals(volume.getState())) && !_storageMgr.volumeOnSharedStoragePool(volume)) {
564567
throw new InvalidParameterValueException("Please specify a volume that has been created on a shared storage pool.");
565568
}
566569

567-
if ( !(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Ready.equals(volume.getState()) || Volume.State.Uploaded.equals(volume.getState())) ) {
568-
throw new InvalidParameterValueException("Volume state must be in Allocated, Ready or Uploaded state");
570+
if ( !(Volume.State.Allocated.equals(volume.getState()) || Volume.State.Ready.equals(volume.getState())) ) {
571+
throw new InvalidParameterValueException("Volume state must be in Allocated or Ready state");
569572
}
570573

571574
VolumeVO rootVolumeOfVm = null;
@@ -578,7 +581,7 @@ public Volume attachVolumeToVM(AttachVolumeCmd command) {
578581

579582
HypervisorType rootDiskHyperType = _volsDao.getHypervisorType(rootVolumeOfVm.getId());
580583

581-
if (volume.getState().equals(Volume.State.Allocated) || volume.getState().equals(Volume.State.Uploaded)) {
584+
if (volume.getState().equals(Volume.State.Allocated) || volume.getState().equals(Volume.State.UploadOp)) {
582585
/* Need to create the volume */
583586
VMTemplateVO rootDiskTmplt = _templateDao.findById(vm.getTemplateId());
584587
DataCenterVO dcVO = _dcDao.findById(vm.getDataCenterIdToDeployIn());

0 commit comments

Comments
 (0)