cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mc...@apache.org
Subject git commit: updated refs/heads/4.2 to e13959a
Date Tue, 23 Jul 2013 21:58:13 GMT
Updated Branches:
  refs/heads/4.2 24af02e5b -> e13959afc


CLOUDSTACK-3716: State of expunged volumes are not consistent in volumes
table and volume_store_ref.


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

Branch: refs/heads/4.2
Commit: e13959afc3cccba2d2aec0ec74206deb74def6f9
Parents: 24af02e
Author: Min Chen <min.chen@citrix.com>
Authored: Tue Jul 23 14:52:39 2013 -0700
Committer: Min Chen <min.chen@citrix.com>
Committed: Tue Jul 23 14:56:15 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    | 107 ++++++++++---------
 5 files changed, 120 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/e13959af/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/e13959af/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/e13959af/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/e13959af/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 183d27e..48b660b 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;
@@ -211,13 +212,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;
         }
@@ -246,7 +268,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);
@@ -256,6 +282,7 @@ public class VolumeServiceImpl implements VolumeService {
         return future;
     }
 
+
     public Void deleteVolumeCallback(AsyncCallbackDispatcher<VolumeServiceImpl, CommandResult>
callback,
             DeleteVolumeContext<VolumeApiResult> context) {
         CommandResult result = callback.getResult();
@@ -263,7 +290,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/e13959af/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 1c133cf..016fa9e 100644
--- a/server/src/com/cloud/storage/VolumeManagerImpl.java
+++ b/server/src/com/cloud/storage/VolumeManagerImpl.java
@@ -349,7 +349,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 +411,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 +480,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);
 
@@ -908,26 +908,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();
@@ -936,10 +936,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();
@@ -960,7 +960,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);
@@ -1314,7 +1314,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.");
         }
@@ -1358,9 +1358,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;
@@ -1647,7 +1658,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);
@@ -1659,8 +1670,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());
@@ -2033,7 +2044,7 @@ public class VolumeManagerImpl extends ManagerBase implements VolumeManager
{
     @DB
     protected VolumeVO switchVolume(VolumeVO existingVolume,
             VirtualMachineProfile<? extends VirtualMachine> vm)
-            throws StorageUnavailableException {
+                    throws StorageUnavailableException {
         Transaction txn = Transaction.currentTxn();
 
         Long templateIdToUse = null;
@@ -2340,14 +2351,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 {
@@ -2448,7 +2459,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 {
@@ -2605,8 +2616,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");


Mime
View raw message