From 365ac7501f08c0f10c90f029f4af972f2b2538d0 Mon Sep 17 00:00:00 2001 From: Kris McQueen Date: Thu, 21 Oct 2010 15:14:51 -0700 Subject: [PATCH] bug 6672: Fix up volume commands. For createVolume, the command is supposed to create a database object, but not assign it to a storage pool until the user first attaches it to a vm instance. That allows the volume to start off hypervisor agnostic. For attachVolume, detachVolume, and listVolumes, the responses had minor issues (sourceType could be null, for example) in either the response generation, or the response handling by the UI. status 6672: resolved fixed --- .../cloud/api/commands/CreateVolumeCmd.java | 6 +-- .../cloud/api/commands/ListVolumesCmd.java | 4 +- .../src/com/cloud/storage/StorageManager.java | 2 +- .../com/cloud/storage/StorageManagerImpl.java | 40 ++++++++++++++----- .../src/com/cloud/vm/UserVmManagerImpl.java | 2 +- ui/scripts/cloud.core.storage.js | 9 +++-- 6 files changed, 42 insertions(+), 21 deletions(-) diff --git a/server/src/com/cloud/api/commands/CreateVolumeCmd.java b/server/src/com/cloud/api/commands/CreateVolumeCmd.java index 6e61ede0ce2..31ec30f5c70 100644 --- a/server/src/com/cloud/api/commands/CreateVolumeCmd.java +++ b/server/src/com/cloud/api/commands/CreateVolumeCmd.java @@ -32,8 +32,8 @@ import com.cloud.storage.VolumeVO; import com.cloud.user.Account; import com.cloud.user.UserContext; -@Implementation(createMethod="createVolumeDB", method="createVolume", manager=Manager.StorageManager, description="Creates a disk volume from a disk offering. " + - "This disk volume must still be attached to a virtual machine to make use of it.") +@Implementation(createMethod="allocVolume", method="createVolume", manager=Manager.StorageManager, description="Creates a disk volume from a disk offering. " + + "This disk volume must still be attached to a virtual machine to make use of it.") public class CreateVolumeCmd extends BaseAsyncCreateCmd { public static final Logger s_logger = Logger.getLogger(CreateVolumeCmd.class.getName()); private static final String s_name = "createvolumeresponse"; @@ -109,7 +109,7 @@ public class CreateVolumeCmd extends BaseAsyncCreateCmd { public static String getResultObjectName() { return "volume"; } - + @Override public long getAccountId() { Account account = (Account)UserContext.current().getAccount(); diff --git a/server/src/com/cloud/api/commands/ListVolumesCmd.java b/server/src/com/cloud/api/commands/ListVolumesCmd.java index ecc9e6ba9ba..12f6bbb2493 100755 --- a/server/src/com/cloud/api/commands/ListVolumesCmd.java +++ b/server/src/com/cloud/api/commands/ListVolumesCmd.java @@ -202,7 +202,9 @@ public class ListVolumesCmd extends BaseListCmd { String poolName = (poolId == null) ? "none" : ApiDBUtils.findStoragePoolById(poolId).getName(); volResponse.setStoragePoolName(poolName); volResponse.setSourceId(volume.getSourceId()); - volResponse.setSourceType(volume.getSourceType().toString()); + if (volume.getSourceType() != null) { + volResponse.setSourceType(volume.getSourceType().toString()); + } volResponse.setHypervisor(ApiDBUtils.getVolumeHyperType(volume.getId()).toString()); volResponse.setResponseName("volume"); diff --git a/server/src/com/cloud/storage/StorageManager.java b/server/src/com/cloud/storage/StorageManager.java index 2f9692b3079..95e3bbcdc8a 100755 --- a/server/src/com/cloud/storage/StorageManager.java +++ b/server/src/com/cloud/storage/StorageManager.java @@ -214,7 +214,7 @@ public interface StorageManager extends Manager { * @throws InvalidParameterValueException * @throws PermissionDeniedException */ - VolumeVO createVolumeDB(CreateVolumeCmd cmd) throws InvalidParameterValueException, PermissionDeniedException, ResourceAllocationException; + VolumeVO allocVolume(CreateVolumeCmd cmd) throws InvalidParameterValueException, PermissionDeniedException, ResourceAllocationException; /** * Creates the volume based on the given criteria diff --git a/server/src/com/cloud/storage/StorageManagerImpl.java b/server/src/com/cloud/storage/StorageManagerImpl.java index ccc7f5cfd77..8e4b680ab54 100755 --- a/server/src/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/com/cloud/storage/StorageManagerImpl.java @@ -73,6 +73,7 @@ import com.cloud.api.commands.PreparePrimaryStorageForMaintenanceCmd; import com.cloud.api.commands.UpdateStoragePoolCmd; import com.cloud.async.AsyncInstanceCreateStatus; import com.cloud.async.AsyncJobManager; +import com.cloud.async.executor.VolumeOperationParam.VolumeOp; import com.cloud.capacity.CapacityVO; import com.cloud.capacity.dao.CapacityDao; import com.cloud.configuration.Config; @@ -592,6 +593,7 @@ public class StorageManagerImpl implements StorageManager { createdVolume.setFolder(volumeFolder); createdVolume.setPath(volumeUUID); createdVolume.setDomainId(account.getDomainId()); + createdVolume.setState(Volume.State.Ready); } else { createdVolume.setStatus(AsyncInstanceCreateStatus.Corrupted); createdVolume.setDestroyed(true); @@ -1633,7 +1635,7 @@ public class StorageManagerImpl implements StorageManager { /*Just allocate a volume in the database, don't send the createvolume cmd to hypervisor. The volume will be finally created only when it's attached to a VM.*/ @Override - public VolumeVO createVolumeDB(CreateVolumeCmd cmd) throws InvalidParameterValueException, PermissionDeniedException, ResourceAllocationException { + public VolumeVO allocVolume(CreateVolumeCmd cmd) throws InvalidParameterValueException, PermissionDeniedException, ResourceAllocationException { // FIXME: some of the scheduled event stuff might be missing here... Account account = (Account)UserContext.current().getAccount(); String accountName = cmd.getAccountName(); @@ -1758,6 +1760,7 @@ public class StorageManagerImpl implements StorageManager { volume.setUpdated(new Date()); volume.setStatus(AsyncInstanceCreateStatus.Creating); volume.setDomainId((account == null) ? Domain.ROOT_DOMAIN : account.getDomainId()); + volume.setState(Volume.State.Allocated); volume = _volsDao.persist(volume); return volume; @@ -1766,7 +1769,7 @@ public class StorageManagerImpl implements StorageManager { @Override @DB public VolumeVO createVolume(CreateVolumeCmd cmd) { VolumeVO volume = _volsDao.findById(cmd.getId()); - VolumeVO createdVolume = null; +// VolumeVO createdVolume = null; Long userId = UserContext.current().getUserId(); if (cmd.getSnapshotId() != null) { @@ -1774,46 +1777,59 @@ public class StorageManagerImpl implements StorageManager { } else { DataCenterVO dc = _dcDao.findById(cmd.getZoneId()); DiskOfferingVO diskOffering = _diskOfferingDao.findById(cmd.getDiskOfferingId()); - long size = diskOffering.getDiskSize(); + long sizeMB = diskOffering.getDiskSize(); +/* VMTemplateVO template = _templateDao.findById(volume.getTemplateId()); + long size = diskOffering.getDiskSize(); try { List poolsToAvoid = new ArrayList(); Set podsToAvoid = new HashSet(); Pair pod = null; + HypervisorType hypervisorType = ((template == null) ? HypervisorType.None : template.getHypervisorType()); while ((pod = _agentMgr.findPod(null, null, dc, volume.getAccountId(), podsToAvoid)) != null) { - if ((createdVolume = createVolume(volume, null, null, dc, pod.first(), null, null, diskOffering, poolsToAvoid, size, template.getHypervisorType())) != null) { + if ((createdVolume = createVolume(volume, null, null, dc, pod.first(), null, null, diskOffering, poolsToAvoid, size, hypervisorType)) != null) { break; } else { podsToAvoid.add(pod.first().getId()); } } +*/ // Create an event EventVO event = new EventVO(); event.setAccountId(volume.getAccountId()); event.setUserId(userId); event.setType(EventTypes.EVENT_VOLUME_CREATE); event.setStartId(cmd.getStartEventId()); - +/* Transaction txn = Transaction.currentTxn(); txn.start(); if (createdVolume != null) { +*/ // Increment the number of volumes - _accountMgr.incrementResourceCount(createdVolume.getAccountId(), ResourceType.volume); +// _accountMgr.incrementResourceCount(createdVolume.getAccountId(), ResourceType.volume); + _accountMgr.incrementResourceCount(volume.getAccountId(), ResourceType.volume); + VolumeVO volForUpdate = _volsDao.createForUpdate(); + volForUpdate.setStatus(AsyncInstanceCreateStatus.Created); + _volsDao.update(volume.getId(), volForUpdate); // Set event parameters - long sizeMB = createdVolume.getSize() / (1024 * 1024); - StoragePoolVO pool = _storagePoolDao.findById(createdVolume.getPoolId()); - String eventParams = "id=" + createdVolume.getId() + "\ndoId=" + diskOffering.getId() + "\ntId=" + -1 + "\ndcId=" + dc.getId() + "\nsize=" + sizeMB; +// long sizeMB = createdVolume.getSize() / (1024 * 1024); +// StoragePoolVO pool = _storagePoolDao.findById(createdVolume.getPoolId()); +// String eventParams = "id=" + createdVolume.getId() + "\ndoId=" + diskOffering.getId() + "\ntId=" + -1 + "\ndcId=" + dc.getId() + "\nsize=" + sizeMB; + String eventParams = "id=" + volume.getId() + "\ndoId=" + diskOffering.getId() + "\ntId=" + -1 + "\ndcId=" + dc.getId() + "\nsize=" + sizeMB; event.setLevel(EventVO.LEVEL_INFO); - event.setDescription("Created volume: " + createdVolume.getName() + " with size: " + sizeMB + " MB in pool: " + pool.getName()); +// event.setDescription("Created volume: " + createdVolume.getName() + " with size: " + sizeMB + " MB in pool: " + pool.getName()); +// event.setDescription("Created volume: " + createdVolume.getName() + " with size: " + sizeMB + " MB"); + event.setDescription("Created volume: " + volume.getName() + " with size: " + sizeMB + " MB"); event.setParameters(eventParams); event.setState(EventState.Completed); _eventDao.persist(event); +/* } else { event.setDescription("Unable to create a volume for " + volume); event.setLevel(EventVO.LEVEL_ERROR); @@ -1825,9 +1841,11 @@ public class StorageManagerImpl implements StorageManager { } catch (Exception e) { s_logger.error("Unhandled exception while creating volume " + volume.getName(), e); } +*/ } - return createdVolume; +// return createdVolume; + return _volsDao.findById(volume.getId()); } @Override diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java index 40d0b5cf7cb..ef185a54af9 100644 --- a/server/src/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/com/cloud/vm/UserVmManagerImpl.java @@ -371,7 +371,7 @@ public class UserVmManagerImpl implements UserVmManager { } // Check that the volume is stored on shared storage - if (!_storageMgr.volumeOnSharedStoragePool(volume)) { + if (!Volume.State.Allocated.equals(volume.getState()) && !_storageMgr.volumeOnSharedStoragePool(volume)) { throw new InvalidParameterValueException("Please specify a volume that has been created on a shared storage pool."); } diff --git a/ui/scripts/cloud.core.storage.js b/ui/scripts/cloud.core.storage.js index 9eb2aaa02b3..580b03046d8 100644 --- a/ui/scripts/cloud.core.storage.js +++ b/ui/scripts/cloud.core.storage.js @@ -166,7 +166,7 @@ function showStorageTab(domainId, targetTab) { $("body").stopTime(timerKey); if (result.jobstatus == 1) { // Succeeded - volumeJSONToTemplate(result.volume[0], template); + volumeJSONToTemplate(result.jobresult.createvolumeresponse, template); changeGridRowsTotal(submenuContent.find("#grid_rows_total"), 1); loadingImg.hide(); @@ -1103,15 +1103,16 @@ function showStorageTab(domainId, targetTab) { $("body").stopTime(timerKey); if (result.jobstatus == 1) { // Succeeded - if (result.virtualmachine[0].vmstate == "Stopped") { + var virtualmachine = result.jobresult.attachvolumeresponse; + if (virtualmachine.vmstate == "Stopped") { template.find("#volume_action_attach_span, #volume_action_delete_span").hide(); template.find("#volume_action_detach_span, #volume_action_create_template_span").show(); } else { template.find("#volume_action_attach_span, #volume_action_delete_span, #volume_action_create_template_span").hide(); template.find("#volume_action_detach_span").show(); } - template.find("#volume_vmname").text(getVmName(result.virtualmachine[0].vmname, result.virtualmachine[0].vmdisplayname) + " (" + result.virtualmachine[0].vmstate + ")"); - template.data("vmid", virtualMachineId).data("vmname", getVmName(result.virtualmachine[0].vmname, result.virtualmachine[0].vmdisplayname)); + template.find("#volume_vmname").text(getVmName(virtualmachine.vmname, virtualmachine.vmdisplayname) + " (" + virtualmachine.vmstate + ")"); + template.data("vmid", virtualMachineId).data("vmname", getVmName(virtualmachine.vmname, virtualmachine.vmdisplayname)); loadingImg.hide(); rowContainer.show(); } else if (result.jobstatus == 2) {