You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by mc...@apache.org on 2013/07/24 00:06:43 UTC
git commit: updated refs/heads/master to 956e830
Updated Branches:
refs/heads/master 529ac6f12 -> 956e8301c
CLOUDSTACK-3716: State of expunged volumes are not consistent in volumes
table and volume_store_ref.
Conflicts:
server/src/com/cloud/storage/VolumeManagerImpl.java
Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/956e8301
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/956e8301
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/956e8301
Branch: refs/heads/master
Commit: 956e8301c200ca5e4584e88ccb05e4c6d6ec350c
Parents: 529ac6f
Author: Min Chen <mi...@citrix.com>
Authored: Tue Jul 23 15:06:02 2013 -0700
Committer: Min Chen <mi...@citrix.com>
Committed: Tue Jul 23 15:06:34 2013 -0700
----------------------------------------------------------------------
.../api/storage/VolumeDataFactory.java | 4 +
.../storage/volume/VolumeDataFactoryImpl.java | 20 ++++
.../cloudstack/storage/volume/VolumeObject.java | 8 +-
.../storage/volume/VolumeServiceImpl.java | 36 +++++-
.../com/cloud/storage/VolumeManagerImpl.java | 117 ++++++++++---------
5 files changed, 126 insertions(+), 59 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/956e8301/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeDataFactory.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeDataFactory.java b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeDataFactory.java
index fc65a32..99e3b59 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeDataFactory.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/VolumeDataFactory.java
@@ -18,10 +18,14 @@
*/
package org.apache.cloudstack.engine.subsystem.api.storage;
+import com.cloud.storage.DataStoreRole;
+
public interface VolumeDataFactory {
VolumeInfo getVolume(long volumeId, DataStore store);
VolumeInfo getVolume(DataObject volume, DataStore store);
+ VolumeInfo getVolume(long volumeId, DataStoreRole storeRole);
+
VolumeInfo getVolume(long volumeId);
}
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/956e8301/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java
index 8d0a5a8..2512c49 100644
--- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java
+++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeDataFactoryImpl.java
@@ -52,6 +52,26 @@ public class VolumeDataFactoryImpl implements VolumeDataFactory {
}
@Override
+ public VolumeInfo getVolume(long volumeId, DataStoreRole storeRole) {
+ VolumeVO volumeVO = volumeDao.findById(volumeId);
+ VolumeObject vol = null;
+ if (storeRole == DataStoreRole.Image) {
+ VolumeDataStoreVO volumeStore = volumeStoreDao.findByVolume(volumeId);
+ if (volumeStore != null) {
+ DataStore store = this.storeMgr.getDataStore(volumeStore.getDataStoreId(), DataStoreRole.Image);
+ vol = VolumeObject.getVolumeObject(store, volumeVO);
+ }
+ } else {
+ // Primary data store
+ if (volumeVO.getPoolId() != null) {
+ DataStore store = this.storeMgr.getDataStore(volumeVO.getPoolId(), DataStoreRole.Primary);
+ vol = VolumeObject.getVolumeObject(store, volumeVO);
+ }
+ }
+ return vol;
+ }
+
+ @Override
public VolumeInfo getVolume(long volumeId) {
VolumeVO volumeVO = volumeDao.findById(volumeId);
VolumeObject vol = null;
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/956e8301/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
----------------------------------------------------------------------
diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
index 55fc3a6..c247f18 100644
--- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
+++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeObject.java
@@ -109,7 +109,7 @@ public class VolumeObject implements VolumeInfo {
@Override
public String get_iScsiName() {
- return volumeVO.get_iScsiName();
+ return volumeVO.get_iScsiName();
}
public void setSize(Long size) {
@@ -254,9 +254,8 @@ public class VolumeObject implements VolumeInfo {
}
if (this.dataStore.getRole() == DataStoreRole.Image) {
objectInStoreMgr.update(this, event);
- if (this.volumeVO.getState() == Volume.State.Migrating
- || this.volumeVO.getState() == Volume.State.Copying
- || this.volumeVO.getState() == Volume.State.Uploaded) {
+ if (this.volumeVO.getState() == Volume.State.Migrating || this.volumeVO.getState() == Volume.State.Copying || this.volumeVO.getState() == Volume.State.Uploaded
+ || this.volumeVO.getState() == Volume.State.Expunged) {
return;
}
if (event == ObjectInDataStoreStateMachine.Event.CreateOnlyRequested) {
@@ -513,6 +512,7 @@ public class VolumeObject implements VolumeInfo {
}
+ @Override
public void incRefCount() {
if (this.dataStore == null) {
return;
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/956e8301/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
index 5919c27..cc73b47 100644
--- a/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
+++ b/engine/storage/volume/src/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java
@@ -75,6 +75,7 @@ import com.cloud.storage.StoragePool;
import com.cloud.storage.VMTemplateStoragePoolVO;
import com.cloud.storage.VMTemplateStorageResourceAssoc;
import com.cloud.storage.VMTemplateStorageResourceAssoc.Status;
+import com.cloud.storage.Volume.State;
import com.cloud.storage.Volume;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.VMTemplatePoolDao;
@@ -210,13 +211,34 @@ public class VolumeServiceImpl implements VolumeService {
}
}
+ // check if a volume is expunged on both primary and secondary
+ private boolean canVolumeBeRemoved(long volumeId) {
+ VolumeVO vol = volDao.findById(volumeId);
+ if (vol == null) {
+ // already removed from volumes table
+ return false;
+ }
+ VolumeDataStoreVO volumeStore = _volumeStoreDao.findByVolume(volumeId);
+ if (vol.getState() == State.Expunged && volumeStore == null) {
+ // volume is expunged from primary, as well as on secondary
+ return true;
+ } else {
+ return false;
+ }
+
+ }
+
@DB
@Override
public AsyncCallFuture<VolumeApiResult> expungeVolumeAsync(VolumeInfo volume) {
AsyncCallFuture<VolumeApiResult> future = new AsyncCallFuture<VolumeApiResult>();
VolumeApiResult result = new VolumeApiResult(volume);
if (volume.getDataStore() == null) {
- volDao.remove(volume.getId());
+ s_logger.info("Expunge volume with no data store specified");
+ if (canVolumeBeRemoved(volume.getId())) {
+ s_logger.info("Volume " + volume.getId() + " is not referred anywhere, remove it from volumes table");
+ volDao.remove(volume.getId());
+ }
future.complete(result);
return future;
}
@@ -245,7 +267,11 @@ public class VolumeServiceImpl implements VolumeService {
}
VolumeObject vo = (VolumeObject) volume;
- volume.processEvent(Event.ExpungeRequested);
+ if (volume.getDataStore().getRole() == DataStoreRole.Image) {
+ volume.processEvent(Event.DestroyRequested);
+ } else if (volume.getDataStore().getRole() == DataStoreRole.Primary) {
+ volume.processEvent(Event.ExpungeRequested);
+ }
DeleteVolumeContext<VolumeApiResult> context = new DeleteVolumeContext<VolumeApiResult>(null, vo, future);
AsyncCallbackDispatcher<VolumeServiceImpl, CommandResult> caller = AsyncCallbackDispatcher.create(this);
@@ -255,6 +281,7 @@ public class VolumeServiceImpl implements VolumeService {
return future;
}
+
public Void deleteVolumeCallback(AsyncCallbackDispatcher<VolumeServiceImpl, CommandResult> callback,
DeleteVolumeContext<VolumeApiResult> context) {
CommandResult result = callback.getResult();
@@ -262,7 +289,10 @@ public class VolumeServiceImpl implements VolumeService {
VolumeApiResult apiResult = new VolumeApiResult(vo);
if (result.isSuccess()) {
vo.processEvent(Event.OperationSuccessed);
- volDao.remove(vo.getId());
+ if (canVolumeBeRemoved(vo.getId())) {
+ s_logger.info("Volume " + vo.getId() + " is not referred anywhere, remove it from volumes table");
+ volDao.remove(vo.getId());
+ }
} else {
vo.processEvent(Event.OperationFailed);
apiResult.setResult(result.getResult());
http://git-wip-us.apache.org/repos/asf/cloudstack/blob/956e8301/server/src/com/cloud/storage/VolumeManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/storage/VolumeManagerImpl.java b/server/src/com/cloud/storage/VolumeManagerImpl.java
index a77efbd..f9e04f6 100644
--- a/server/src/com/cloud/storage/VolumeManagerImpl.java
+++ b/server/src/com/cloud/storage/VolumeManagerImpl.java
@@ -31,6 +31,10 @@ import java.util.concurrent.ExecutionException;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
+import org.apache.commons.lang.StringUtils;
+import org.apache.log4j.Logger;
+import org.springframework.stereotype.Component;
+
import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.command.user.volume.AttachVolumeCmd;
import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
@@ -69,9 +73,7 @@ import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.VolumeDataStoreVO;
import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity;
-import org.apache.commons.lang.StringUtils;
-import org.apache.log4j.Logger;
-import org.springframework.stereotype.Component;
+
import com.cloud.agent.AgentManager;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.storage.CreateVolumeOVAAnswer;
@@ -349,7 +351,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
public VolumeInfo moveVolume(VolumeInfo volume, long destPoolDcId,
Long destPoolPodId, Long destPoolClusterId,
HypervisorType dataDiskHyperType)
- throws ConcurrentOperationException {
+ throws ConcurrentOperationException {
// Find a destination storage pool with the specified criteria
DiskOfferingVO diskOffering = _diskOfferingDao.findById(volume
@@ -411,7 +413,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
private boolean validateVolume(Account caller, long ownerId, Long zoneId,
String volumeName, String url, String format)
- throws ResourceAllocationException {
+ throws ResourceAllocationException {
// permission check
_accountMgr.checkAccess(caller, null, true,
@@ -480,17 +482,17 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
&& !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")))) {
+ || (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());
+ + " is an invalid for the format "
+ + format.toLowerCase());
}
UriUtils.validateUrl(url);
@@ -905,26 +907,26 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
if (isCustomizedIops != null) {
if (isCustomizedIops) {
- minIops = cmd.getMinIops();
- maxIops = cmd.getMaxIops();
-
- if (minIops == null && maxIops == null) {
- minIops = 0L;
- maxIops = 0L;
- }
- else {
+ minIops = cmd.getMinIops();
+ maxIops = cmd.getMaxIops();
+
+ if (minIops == null && maxIops == null) {
+ minIops = 0L;
+ maxIops = 0L;
+ }
+ else {
if (minIops == null || minIops <= 0) {
throw new InvalidParameterValueException("The min IOPS must be greater than 0.");
}
- if (maxIops == null) {
- maxIops = 0L;
- }
+ if (maxIops == null) {
+ maxIops = 0L;
+ }
- if (minIops > maxIops) {
- throw new InvalidParameterValueException("The min IOPS must be less than or equal to the max IOPS.");
- }
- }
+ if (minIops > maxIops) {
+ throw new InvalidParameterValueException("The min IOPS must be less than or equal to the max IOPS.");
+ }
+ }
}
else {
minIops = diskOffering.getMinIops();
@@ -933,10 +935,10 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
}
if (!validateVolumeSizeRange(size)) {// convert size from mb to gb
- // for validation
+ // for validation
throw new InvalidParameterValueException(
"Invalid size for custom volume creation: " + size
- + " ,max volume size is:" + _maxVolumeSizeInGb);
+ + " ,max volume size is:" + _maxVolumeSizeInGb);
}
} else { // create volume from snapshot
Long snapshotId = cmd.getSnapshotId();
@@ -957,7 +959,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
diskOffering = _diskOfferingDao.findById(diskOfferingId);
zoneId = snapshotCheck.getDataCenterId();
size = snapshotCheck.getSize(); // ; disk offering is used for tags
- // purposes
+ // purposes
// check snapshot permissions
_accountMgr.checkAccess(caller, null, true, snapshotCheck);
@@ -1311,7 +1313,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
_accountMgr.checkAccess(caller, null, true, volume);
- if (volume.getInstanceId() != null) {
+ if (volume.getInstanceId() != null) {
throw new InvalidParameterValueException(
"Please specify a volume that is not attached to any VM.");
}
@@ -1355,9 +1357,20 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
_usageEventDao.persist(usageEvent);
}
}
- AsyncCallFuture<VolumeApiResult> future = volService.expungeVolumeAsync(volFactory.getVolume(volume.getId()));
- future.get();
-
+ // expunge volume from primary if volume is on primary
+ VolumeInfo volOnPrimary = volFactory.getVolume(volume.getId(), DataStoreRole.Primary);
+ if (volOnPrimary != null) {
+ s_logger.info("Expunging volume " + volume.getId() + " from primary data store");
+ AsyncCallFuture<VolumeApiResult> future = volService.expungeVolumeAsync(volOnPrimary);
+ future.get();
+ }
+ // expunge volume from secondary if volume is on image store
+ VolumeInfo volOnSecondary = volFactory.getVolume(volume.getId(), DataStoreRole.Image);
+ if (volOnSecondary != null) {
+ s_logger.info("Expunging volume " + volume.getId() + " from secondary data store");
+ AsyncCallFuture<VolumeApiResult> future2 = volService.expungeVolumeAsync(volOnSecondary);
+ future2.get();
+ }
} catch (Exception e) {
s_logger.warn("Failed to expunge volume:", e);
return false;
@@ -1644,7 +1657,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
volumeToAttach = _volsDao.findById(volumeToAttach.getId());
if (volumeToAttachStoragePool.isManaged() &&
- volumeToAttach.getPath() == null) {
+ volumeToAttach.getPath() == null) {
volumeToAttach.setPath(answer.getDisk().getVdiUuid());
_volsDao.update(volumeToAttach.getId(), volumeToAttach);
@@ -1656,8 +1669,8 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
// insert record for disk I/O statistics
VmDiskStatisticsVO diskstats = _vmDiskStatsDao.findBy(vm.getAccountId(), vm.getDataCenterId(),vm.getId(), volumeToAttach.getId());
if (diskstats == null) {
- diskstats = new VmDiskStatisticsVO(vm.getAccountId(), vm.getDataCenterId(),vm.getId(), volumeToAttach.getId());
- _vmDiskStatsDao.persist(diskstats);
+ diskstats = new VmDiskStatisticsVO(vm.getAccountId(), vm.getDataCenterId(),vm.getId(), volumeToAttach.getId());
+ _vmDiskStatsDao.persist(diskstats);
}
return _volsDao.findById(volumeToAttach.getId());
@@ -2030,7 +2043,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
@DB
protected VolumeVO switchVolume(VolumeVO existingVolume,
VirtualMachineProfile vm)
- throws StorageUnavailableException {
+ throws StorageUnavailableException {
Transaction txn = Transaction.currentTxn();
Long templateIdToUse = null;
@@ -2086,7 +2099,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
for (VolumeVO vol : volumesForVm) {
if (vol.getVolumeType().equals(Type.ROOT)) {
// Destroy volume if not already destroyed
- boolean volumeAlreadyDestroyed = (vol.getState() == Volume.State.Destroy ||
+ boolean volumeAlreadyDestroyed = (vol.getState() == Volume.State.Destroy ||
vol.getState() == Volume.State.Expunged ||
vol.getState() == Volume.State.Expunging);
if (!volumeAlreadyDestroyed) {
@@ -2337,14 +2350,14 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
MIGRATE
}
private static class VolumeTask {
- final VolumeTaskType type;
- final StoragePoolVO pool;
- final VolumeVO volume;
- VolumeTask(VolumeTaskType type, VolumeVO volume, StoragePoolVO pool) {
- this.type = type;
- this.pool = pool;
- this.volume = volume;
- }
+ final VolumeTaskType type;
+ final StoragePoolVO pool;
+ final VolumeVO volume;
+ VolumeTask(VolumeTaskType type, VolumeVO volume, StoragePoolVO pool) {
+ this.type = type;
+ this.pool = pool;
+ this.volume = volume;
+ }
}
private List<VolumeTask> getTasks(List<VolumeVO> vols, Map<Volume, StoragePool> destVols) throws StorageUnavailableException {
@@ -2445,7 +2458,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
DataStore destPool = null;
if (recreate
&& (dest.getStorageForDisks() == null || dest
- .getStorageForDisks().get(vol) == null)) {
+ .getStorageForDisks().get(vol) == null)) {
destPool = dataStoreMgr.getDataStore(vol.getPoolId(), DataStoreRole.Primary);
s_logger.debug("existing pool: " + destPool.getId());
} else {
@@ -2602,8 +2615,8 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager {
.getValue(Config.CustomDiskOfferingMinSize.toString());
_customDiskOfferingMinSize = NumbersUtil.parseInt(
_customDiskOfferingMinSizeStr, Integer
- .parseInt(Config.CustomDiskOfferingMinSize
- .getDefaultValue()));
+ .parseInt(Config.CustomDiskOfferingMinSize
+ .getDefaultValue()));
String maxVolumeSizeInGbString = _configDao
.getValue("storage.max.volume.size");