From ac01241bf425d7c5c59dcc85182f198df9d5362b Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 10 Jul 2019 13:37:02 +0530 Subject: [PATCH 1/7] engine, server, services: fix for respecting secondary storage threshold limit ImageStoreProviderManager will return a null value when there is no image store available with free space and satisfies threshold limit. Consumers have been refactored to accept null value and throw exception or log error if needed. Signed-off-by: Abhishek Kumar --- .../motion/AncientDataMotionStrategy.java | 2 +- ...vmNonManagedStorageDataMotionStrategy.java | 28 ++++++++++--------- .../ImageStoreProviderManagerImpl.java | 28 +++++++++++++------ .../element/ConfigDriveNetworkElement.java | 15 ++++++++-- .../java/com/cloud/server/StatsCollector.java | 9 ++++++ .../cloud/storage/VolumeApiServiceImpl.java | 3 ++ .../storage/upload/UploadMonitorImpl.java | 4 +++ .../SecondaryStorageManagerImpl.java | 5 +++- 8 files changed, 68 insertions(+), 26 deletions(-) diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 7b526458835b..f4b03ecbd5cc 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -329,7 +329,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) // need to find a nfs or cifs image store, assuming that can't copy volume // directly to s3 ImageStoreEntity imageStore = (ImageStoreEntity)dataStoreMgr.getImageStore(destScope.getScopeId()); - if (!imageStore.getProtocol().equalsIgnoreCase("nfs") && !imageStore.getProtocol().equalsIgnoreCase("cifs")) { + if (imageStore == null || !imageStore.getProtocol().equalsIgnoreCase("nfs") && !imageStore.getProtocol().equalsIgnoreCase("cifs")) { s_logger.debug("can't find a nfs (or cifs) image store to satisfy the need for a staging store"); return null; } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java index e42715a1e6dd..fa2156e65a0c 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java @@ -24,18 +24,17 @@ import javax.inject.Inject; -import com.cloud.storage.ScopeType; -import com.cloud.storage.Storage; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; -import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.storage.command.CopyCommand; import org.apache.cloudstack.storage.datastore.DataStoreManagerImpl; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.TemplateObjectTO; +import org.apache.commons.collections.MapUtils; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; @@ -47,6 +46,8 @@ import com.cloud.host.Host; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.DataStoreRole; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; @@ -56,7 +57,6 @@ import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachineManager; -import org.apache.commons.collections.MapUtils; /** * Extends {@link StorageSystemDataMotionStrategy}, allowing KVM hosts to migrate VMs with the ROOT volume on a non managed local storage pool. @@ -198,18 +198,20 @@ protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolum VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId()); if (sourceVolumeTemplateStoragePoolVO == null && destStoragePool.getPoolType() == StoragePoolType.Filesystem) { DataStore sourceTemplateDataStore = dataStoreManagerImpl.getImageStore(srcVolumeInfo.getDataCenterId()); - TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore); - TemplateObjectTO sourceTemplate = new TemplateObjectTO(sourceTemplateInfo); + if (sourceTemplateDataStore != null) { + TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore); + TemplateObjectTO sourceTemplate = new TemplateObjectTO(sourceTemplateInfo); - LOGGER.debug(String.format("Could not find template [id=%s, name=%s] on the storage pool [id=%s]; copying the template to the target storage pool.", - srcVolumeInfo.getTemplateId(), sourceTemplateInfo.getName(), destDataStore.getId())); + LOGGER.debug(String.format("Could not find template [id=%s, name=%s] on the storage pool [id=%s]; copying the template to the target storage pool.", + srcVolumeInfo.getTemplateId(), sourceTemplateInfo.getName(), destDataStore.getId())); - TemplateInfo destTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), destDataStore); - final TemplateObjectTO destTemplate = new TemplateObjectTO(destTemplateInfo); - Answer copyCommandAnswer = sendCopyCommand(destHost, sourceTemplate, destTemplate, destDataStore); + TemplateInfo destTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), destDataStore); + final TemplateObjectTO destTemplate = new TemplateObjectTO(destTemplateInfo); + Answer copyCommandAnswer = sendCopyCommand(destHost, sourceTemplate, destTemplate, destDataStore); - if (copyCommandAnswer != null && copyCommandAnswer.getResult()) { - updateTemplateReferenceIfSuccessfulCopy(srcVolumeInfo, srcStoragePool, destTemplateInfo, destDataStore); + if (copyCommandAnswer != null && copyCommandAnswer.getResult()) { + updateTemplateReferenceIfSuccessfulCopy(srcVolumeInfo, srcStoragePool, destTemplateInfo, destDataStore); + } } } } diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java index cb9a97e59657..2a6b577d95c9 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java @@ -19,7 +19,7 @@ package org.apache.cloudstack.storage.image.manager; import java.util.ArrayList; -import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.Iterator; import java.util.List; @@ -28,9 +28,6 @@ import javax.annotation.PostConstruct; import javax.inject.Inject; -import org.apache.log4j.Logger; -import org.springframework.stereotype.Component; - import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreProviderManager; import org.apache.cloudstack.engine.subsystem.api.storage.ImageStoreProvider; @@ -42,6 +39,8 @@ import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; import org.apache.cloudstack.storage.image.datastore.ImageStoreProviderManager; import org.apache.cloudstack.storage.image.store.ImageStoreImpl; +import org.apache.log4j.Logger; +import org.springframework.stereotype.Component; import com.cloud.server.StatsCollector; import com.cloud.storage.ScopeType; @@ -146,17 +145,30 @@ public List listImageCacheStores(Scope scope) { @Override public DataStore getImageStore(List imageStores) { if (imageStores.size() > 1) { - Collections.shuffle(imageStores); // Randomize image store list. + imageStores.sort(new Comparator() { // Sort data stores based on free capacity + @Override + public int compare(DataStore store1, DataStore store2) { + return Long.compare(_statsCollector.imageStoreCurrentFreeCapacity(store1), + _statsCollector.imageStoreCurrentFreeCapacity(store2)); + } + }); Iterator i = imageStores.iterator(); - DataStore imageStore = null; while(i.hasNext()) { - imageStore = i.next(); + DataStore imageStore = i.next(); // Return image store if used percentage is less then threshold value i.e. 90%. if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { return imageStore; } } + } else if (imageStores.size() == 1) { + if (_statsCollector.imageStoreHasEnoughCapacity(imageStores.get(0))) { + return imageStores.get(0); + } } - return imageStores.get(0); + + // No store with space found + s_logger.error(String.format("Can't find an image storage in zone with less than %d usage", + Math.round(_statsCollector.getImageStoreCapacityThreshold()*100))); + return null; } } diff --git a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java index 76e4fc03ce7b..c4cb939572fc 100644 --- a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java +++ b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java @@ -23,7 +23,6 @@ import javax.inject.Inject; -import com.cloud.storage.StoragePool; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; @@ -59,6 +58,7 @@ import com.cloud.service.dao.ServiceOfferingDao; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage; +import com.cloud.storage.StoragePool; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.GuestOSCategoryDao; @@ -308,7 +308,12 @@ public boolean prepareMigration(NicProfile nic, Network network, VirtualMachineP if (nic.isDefaultNic() && _networkModel.getUserDataUpdateProvider(network).getProvider().equals(Provider.ConfigDrive)) { LOG.trace(String.format("[prepareMigration] for vm: %s", vm.getInstanceName())); final DataStore dataStore = findDataStore(vm, dest); - addConfigDriveDisk(vm, dataStore); + + try { + addConfigDriveDisk(vm, dataStore); + } catch (ResourceUnavailableException e) { + LOG.error("Failed to add config disk drive due to: ", e); + } return false; } else return true; @@ -473,7 +478,7 @@ private boolean deleteConfigDriveIso(final VirtualMachine vm) throws ResourceUna return true; } - private void addConfigDriveDisk(final VirtualMachineProfile profile, final DataStore dataStore) { + private void addConfigDriveDisk(final VirtualMachineProfile profile, final DataStore dataStore) throws ResourceUnavailableException { boolean isoAvailable = false; final String isoPath = ConfigDrive.createConfigDrivePath(profile.getInstanceName()); for (DiskTO dataTo : profile.getDisks()) { @@ -484,6 +489,10 @@ private void addConfigDriveDisk(final VirtualMachineProfile profile, final DataS } if (!isoAvailable) { TemplateObjectTO dataTO = new TemplateObjectTO(); + if (dataStore == null) { + throw new ResourceUnavailableException("Config drive disk add failed, datastore not available", + ConfigDriveNetworkElement.class, 0L); + } dataTO.setDataStore(dataStore.getTO()); dataTO.setUuid(profile.getUuid()); dataTO.setPath(isoPath); diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index b81507ad52bf..c6a5dab073aa 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1369,6 +1369,11 @@ public boolean imageStoreHasEnoughCapacity(DataStore imageStore) { return false; } + public long imageStoreCurrentFreeCapacity(DataStore imageStore) { + StorageStats imageStoreStats = _storageStats.get(imageStore.getId()); + return imageStoreStats != null ? Math.max(0, imageStoreStats.getCapacityBytes() - imageStoreStats.getByteUsed()) : 0; + } + /** * Sends VMs metrics to the configured graphite host. */ @@ -1574,4 +1579,8 @@ public String getConfigComponentName() { public ConfigKey[] getConfigKeys() { return new ConfigKey[] {vmDiskStatsInterval, vmDiskStatsIntervalMin, vmNetworkStatsInterval, vmNetworkStatsIntervalMin, StatsTimeout, statsOutputUri}; } + + public double getImageStoreCapacityThreshold() { + return _imageStoreCapacityThreshold; + } } diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 2022d5b5be13..cfcf124763b4 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2677,6 +2677,9 @@ private String orchestrateExtractVolume(long volumeId, long zoneId) { } // perform extraction ImageStoreEntity secStore = (ImageStoreEntity)dataStoreMgr.getImageStore(zoneId); + if (secStore == null) { + throw new InvalidParameterValueException(String.format("Secondary storage to satisfy storage needs cannot be found for zone: %d", zoneId)); + } String value = _configDao.getValue(Config.CopyVolumeWait.toString()); NumbersUtil.parseInt(value, Integer.parseInt(Config.CopyVolumeWait.getDefaultValue())); diff --git a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java index e8f2980a082c..4a4f285aea8c 100644 --- a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java +++ b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java @@ -177,6 +177,10 @@ public Long extractTemplate(VMTemplateVO template, String url, TemplateDataStore Type type = (template.getFormat() == ImageFormat.ISO) ? Type.ISO : Type.TEMPLATE; DataStore secStore = storeMgr.getImageStore(dataCenterId); + if(secStore == null) { + s_logger.error("Unable to extract template, secondary storage to satisfy storage needs cannot be found!"); + return null; + } UploadVO uploadTemplateObj = new UploadVO(secStore.getId(), template.getId(), new Date(), Upload.Status.NOT_UPLOADED, type, url, Mode.FTP_UPLOAD); _uploadDao.persist(uploadTemplateObj); diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 1d3eba835cec..e5a8d99592a4 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -1118,7 +1118,10 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl vm.setDetails(details); DataStore secStore = _dataStoreMgr.getImageStore(dest.getDataCenter().getId()); - assert (secStore != null); + if (secStore != null) { + s_logger.error(String.format("Unable to finalize virtual machine profile as no secondary storage available to satisfy storage needs for zone: %s", dest.getDataCenter().getUuid())); + return false; + } StringBuilder buf = profile.getBootArgsBuilder(); buf.append(" template=domP type=secstorage"); From dec660200bcad211cc8c52879ce8e163a54eb952 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Thu, 11 Jul 2019 11:15:16 +0530 Subject: [PATCH 2/7] services: fix for incorrect null check Signed-off-by: Abhishek Kumar --- .../secondarystorage/SecondaryStorageManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index e5a8d99592a4..9259ba298b61 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -1118,7 +1118,7 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl vm.setDetails(details); DataStore secStore = _dataStoreMgr.getImageStore(dest.getDataCenter().getId()); - if (secStore != null) { + if (secStore == null) { s_logger.error(String.format("Unable to finalize virtual machine profile as no secondary storage available to satisfy storage needs for zone: %s", dest.getDataCenter().getUuid())); return false; } From 410c1bd1e91369d7aaa18d661cc29cd1e6a04187 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Tue, 16 Jul 2019 13:16:41 +0530 Subject: [PATCH 3/7] engine: refactor retrieving image store based on read or write use Signed-off-by: Abhishek Kumar --- .../api/storage/DataStoreManager.java | 4 +++- .../StorageCacheRandomAllocator.java | 4 ++-- .../motion/AncientDataMotionStrategy.java | 2 +- ...vmNonManagedStorageDataMotionStrategy.java | 2 +- ...NonManagedStorageSystemDataMotionTest.java | 22 +++++++++---------- .../ImageStoreProviderManagerImpl.java | 16 +++++++++----- .../storage/snapshot/SnapshotServiceImpl.java | 4 ++-- .../datastore/DataStoreManagerImpl.java | 15 ++++++++++--- .../datastore/ImageStoreProviderManager.java | 4 +++- .../hyperv/manager/HypervManagerImpl.java | 2 +- .../vmware/manager/VmwareManagerImpl.java | 4 ++-- .../element/ConfigDriveNetworkElement.java | 4 ++-- .../cloud/storage/VolumeApiServiceImpl.java | 2 +- .../storage/upload/UploadMonitorImpl.java | 2 +- .../template/HypervisorTemplateAdapter.java | 4 ++-- .../cloud/template/TemplateManagerImpl.java | 8 +++---- .../ConfigDriveNetworkElementTest.java | 2 +- .../SecondaryStorageManagerImpl.java | 4 ++-- 18 files changed, 62 insertions(+), 43 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java index 5ebef031c5c6..145678e8f61d 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java @@ -33,7 +33,9 @@ public interface DataStoreManager { List getImageStoresByScope(ZoneScope scope); - DataStore getImageStore(long zoneId); + DataStore getImageStoreForRead(long zoneId); + + DataStore getImageStoreForWrite(long zoneId); List getImageCacheStores(Scope scope); diff --git a/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java index c9832bf11696..86a281e1be4f 100644 --- a/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java +++ b/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java @@ -62,7 +62,7 @@ public DataStore getCacheStore(Scope scope) { return null; } - return imageStoreMgr.getImageStore(cacheStores); + return imageStoreMgr.getImageStoreForWrite(cacheStores); } @Override @@ -88,6 +88,6 @@ public DataStore getCacheStore(DataObject data, Scope scope) { } } } - return imageStoreMgr.getImageStore(cacheStores); + return imageStoreMgr.getImageStoreForWrite(cacheStores); } } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index f4b03ecbd5cc..60887638ad12 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -328,7 +328,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) if (cacheStore == null) { // need to find a nfs or cifs image store, assuming that can't copy volume // directly to s3 - ImageStoreEntity imageStore = (ImageStoreEntity)dataStoreMgr.getImageStore(destScope.getScopeId()); + ImageStoreEntity imageStore = (ImageStoreEntity)dataStoreMgr.getImageStoreForWrite(destScope.getScopeId()); if (imageStore == null || !imageStore.getProtocol().equalsIgnoreCase("nfs") && !imageStore.getProtocol().equalsIgnoreCase("cifs")) { s_logger.debug("can't find a nfs (or cifs) image store to satisfy the need for a staging store"); return null; diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java index fa2156e65a0c..30b6f1bf1119 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java @@ -197,7 +197,7 @@ protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolum Host destHost) { VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId()); if (sourceVolumeTemplateStoragePoolVO == null && destStoragePool.getPoolType() == StoragePoolType.Filesystem) { - DataStore sourceTemplateDataStore = dataStoreManagerImpl.getImageStore(srcVolumeInfo.getDataCenterId()); + DataStore sourceTemplateDataStore = dataStoreManagerImpl.getImageStoreForRead(srcVolumeInfo.getDataCenterId()); if (sourceTemplateDataStore != null) { TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore); TemplateObjectTO sourceTemplate = new TemplateObjectTO(sourceTemplateInfo); diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java index 5b8d3aff2b8b..c7b802385064 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java @@ -18,13 +18,12 @@ */ package org.apache.cloudstack.storage.motion; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.when; + import java.util.HashMap; import java.util.Map; -import com.cloud.host.Host; -import com.cloud.hypervisor.Hypervisor; -import com.cloud.storage.ScopeType; -import com.cloud.storage.Storage; import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; @@ -58,21 +57,22 @@ import com.cloud.exception.AgentUnavailableException; import com.cloud.exception.CloudException; import com.cloud.exception.OperationTimedoutException; +import com.cloud.host.Host; import com.cloud.host.HostVO; +import com.cloud.hypervisor.Hypervisor; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.DataStoreRole; -import com.cloud.storage.StoragePool; -import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.ScopeType; +import com.cloud.storage.Storage; import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.storage.StoragePool; +import com.cloud.storage.VMTemplateStoragePoolVO; import com.cloud.storage.dao.DiskOfferingDao; import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.VirtualMachineManager; -import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.when; - @RunWith(MockitoJUnitRunner.class) public class KvmNonManagedStorageSystemDataMotionTest { @@ -353,7 +353,7 @@ private void configureAndTestcopyTemplateToTargetStorageIfNeeded(VMTemplateStora Mockito.when(sourceTemplateInfo.getHypervisorType()).thenReturn(HypervisorType.KVM); Mockito.when(vmTemplatePoolDao.findByPoolTemplate(Mockito.anyLong(), Mockito.anyLong())).thenReturn(vmTemplateStoragePoolVO); - Mockito.when(dataStoreManagerImpl.getImageStore(Mockito.anyLong())).thenReturn(sourceTemplateDataStore); + Mockito.when(dataStoreManagerImpl.getImageStoreForRead(Mockito.anyLong())).thenReturn(sourceTemplateDataStore); Mockito.when(templateDataFactory.getTemplate(Mockito.anyLong(), Mockito.eq(sourceTemplateDataStore))).thenReturn(sourceTemplateInfo); Mockito.when(templateDataFactory.getTemplate(Mockito.anyLong(), Mockito.eq(destDataStore))).thenReturn(sourceTemplateInfo); kvmNonManagedStorageDataMotionStrategy.copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, srcStoragePool, destDataStore, destStoragePool, destHost); @@ -362,7 +362,7 @@ private void configureAndTestcopyTemplateToTargetStorageIfNeeded(VMTemplateStora InOrder verifyInOrder = Mockito.inOrder(vmTemplatePoolDao, dataStoreManagerImpl, templateDataFactory, kvmNonManagedStorageDataMotionStrategy); verifyInOrder.verify(vmTemplatePoolDao, Mockito.times(1)).findByPoolTemplate(Mockito.anyLong(), Mockito.anyLong()); - verifyInOrder.verify(dataStoreManagerImpl, Mockito.times(times)).getImageStore(Mockito.anyLong()); + verifyInOrder.verify(dataStoreManagerImpl, Mockito.times(times)).getImageStoreForRead(Mockito.anyLong()); verifyInOrder.verify(templateDataFactory, Mockito.times(times)).getTemplate(Mockito.anyLong(), Mockito.eq(sourceTemplateDataStore)); verifyInOrder.verify(templateDataFactory, Mockito.times(times)).getTemplate(Mockito.anyLong(), Mockito.eq(destDataStore)); verifyInOrder.verify(kvmNonManagedStorageDataMotionStrategy, Mockito.times(times)).sendCopyCommand(Mockito.eq(destHost), Mockito.any(TemplateObjectTO.class), diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java index 2a6b577d95c9..064948a17f2c 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java @@ -19,9 +19,9 @@ package org.apache.cloudstack.storage.image.manager; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; @@ -143,7 +143,15 @@ public List listImageCacheStores(Scope scope) { } @Override - public DataStore getImageStore(List imageStores) { + public DataStore getImageStoreForRead(List imageStores) { + if (imageStores.size() > 1) { + Collections.shuffle(imageStores); + } + return imageStores.get(0); + } + + @Override + public DataStore getImageStoreForWrite(List imageStores) { if (imageStores.size() > 1) { imageStores.sort(new Comparator() { // Sort data stores based on free capacity @Override @@ -152,9 +160,7 @@ public int compare(DataStore store1, DataStore store2) { _statsCollector.imageStoreCurrentFreeCapacity(store2)); } }); - Iterator i = imageStores.iterator(); - while(i.hasNext()) { - DataStore imageStore = i.next(); + for (DataStore imageStore : imageStores) { // Return image store if used percentage is less then threshold value i.e. 90%. if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { return imageStore; diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java index 9c5137096770..9f085f4893e8 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java @@ -240,7 +240,7 @@ private DataStore findSnapshotImageStore(SnapshotInfo snapshot) { fullSnapshot = snapshotFullBackup; } if (fullSnapshot) { - return dataStoreMgr.getImageStore(snapshot.getDataCenterId()); + return dataStoreMgr.getImageStoreForWrite(snapshot.getDataCenterId()); } else { SnapshotInfo parentSnapshot = snapshot.getParent(); // Note that DataStore information in parentSnapshot is for primary @@ -251,7 +251,7 @@ private DataStore findSnapshotImageStore(SnapshotInfo snapshot) { parentSnapshotOnBackupStore = _snapshotStoreDao.findBySnapshot(parentSnapshot.getId(), DataStoreRole.Image); } if (parentSnapshotOnBackupStore == null) { - return dataStoreMgr.getImageStore(snapshot.getDataCenterId()); + return dataStoreMgr.getImageStoreForWrite(snapshot.getDataCenterId()); } return dataStoreMgr.getDataStore(parentSnapshotOnBackupStore.getDataStoreId(), parentSnapshotOnBackupStore.getRole()); } diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java index f64037619a8a..66c4a1ff1437 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java @@ -73,12 +73,21 @@ public List getImageStoresByScope(ZoneScope scope) { } @Override - public DataStore getImageStore(long zoneId) { + public DataStore getImageStoreForRead(long zoneId) { List stores = getImageStoresByScope(new ZoneScope(zoneId)); if (stores == null || stores.size() == 0) { return null; } - return imageDataStoreMgr.getImageStore(stores); + return imageDataStoreMgr.getImageStoreForRead(stores); + } + + @Override + public DataStore getImageStoreForWrite(long zoneId) { + List stores = getImageStoresByScope(new ZoneScope(zoneId)); + if (stores == null || stores.size() == 0) { + return null; + } + return imageDataStoreMgr.getImageStoreForWrite(stores); } @Override @@ -110,7 +119,7 @@ public DataStore getImageCacheStore(long zoneId) { if (stores == null || stores.size() == 0) { return null; } - return imageDataStoreMgr.getImageStore(stores); + return imageDataStoreMgr.getImageStoreForWrite(stores); } @Override diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java index 70b7a7c3c68d..f0cabd89bc3c 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java @@ -42,5 +42,7 @@ public interface ImageStoreProviderManager { boolean registerDriver(String uuid, ImageStoreDriver driver); - DataStore getImageStore(List imageStores); + DataStore getImageStoreForRead(List imageStores); + + DataStore getImageStoreForWrite(List imageStores); } diff --git a/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java b/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java index 9d63726d3474..0bcd775a895c 100644 --- a/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java +++ b/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java @@ -137,7 +137,7 @@ public String prepareSecondaryStorageStore(long zoneId) { private String getSecondaryStorageStoreUrl(long zoneId) { String secUrl = null; - DataStore secStore = _dataStoreMgr.getImageStore(zoneId); + DataStore secStore = _dataStoreMgr.getImageStoreForWrite(zoneId); if (secStore != null) { secUrl = secStore.getUri(); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index 23758d512baf..71966f093b93 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -48,6 +48,7 @@ import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobManagerImpl; +import org.apache.cloudstack.management.ManagementServerHost; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.log4j.Logger; @@ -62,7 +63,6 @@ import com.cloud.agent.api.StartupRoutingCommand; import com.cloud.api.query.dao.TemplateJoinDao; import com.cloud.cluster.ClusterManager; -import org.apache.cloudstack.management.ManagementServerHost; import com.cloud.cluster.dao.ManagementServerHostPeerDao; import com.cloud.configuration.Config; import com.cloud.dc.ClusterDetailsDao; @@ -492,7 +492,7 @@ public Pair getSecondaryStorageStoreUrlAndId(long dcId) { String secUrl = null; Long secId = null; - DataStore secStore = _dataStoreMgr.getImageStore(dcId); + DataStore secStore = _dataStoreMgr.getImageStoreForWrite(dcId); if (secStore != null) { secUrl = secStore.getUri(); secId = secStore.getId(); diff --git a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java index c4cb939572fc..b79a06219495 100644 --- a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java +++ b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java @@ -337,7 +337,7 @@ private DataStore findDataStore(VirtualMachineProfile profile, DeployDestination dataStore = pickExistingRootVolumeFromDataStore(profile, dataStore); } } else { - dataStore = _dataStoreMgr.getImageStore(dest.getDataCenter().getId()); + dataStore = _dataStoreMgr.getImageStoreForWrite(dest.getDataCenter().getId()); } return dataStore; } @@ -449,7 +449,7 @@ private boolean createConfigDriveIso(VirtualMachineProfile profile, DeployDestin } private boolean deleteConfigDriveIso(final VirtualMachine vm) throws ResourceUnavailableException { - DataStore dataStore = _dataStoreMgr.getImageStore(vm.getDataCenterId()); + DataStore dataStore = _dataStoreMgr.getImageStoreForWrite(vm.getDataCenterId()); Long agentId = findAgentIdForImageStore(dataStore); if (VirtualMachineManager.VmConfigDriveOnPrimaryPool.value()) { diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index cfcf124763b4..5b7269dc15c6 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2676,7 +2676,7 @@ private String orchestrateExtractVolume(long volumeId, long zoneId) { throw new InvalidParameterValueException("Volume to be extracted has been removed or not in right state!"); } // perform extraction - ImageStoreEntity secStore = (ImageStoreEntity)dataStoreMgr.getImageStore(zoneId); + ImageStoreEntity secStore = (ImageStoreEntity)dataStoreMgr.getImageStoreForWrite(zoneId); if (secStore == null) { throw new InvalidParameterValueException(String.format("Secondary storage to satisfy storage needs cannot be found for zone: %d", zoneId)); } diff --git a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java index 4a4f285aea8c..9028ecaf6181 100644 --- a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java +++ b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java @@ -176,7 +176,7 @@ public Long extractTemplate(VMTemplateVO template, String url, TemplateDataStore Type type = (template.getFormat() == ImageFormat.ISO) ? Type.ISO : Type.TEMPLATE; - DataStore secStore = storeMgr.getImageStore(dataCenterId); + DataStore secStore = storeMgr.getImageStoreForWrite(dataCenterId); if(secStore == null) { s_logger.error("Unable to extract template, secondary storage to satisfy storage needs cannot be found!"); return null; diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index e2db31a16c90..fa4fce9da060 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -607,7 +607,7 @@ public TemplateProfile prepareDelete(DeleteTemplateCmd cmd) { throw new InvalidParameterValueException("The DomR template cannot be deleted."); } - if (zoneIdList != null && (storeMgr.getImageStore(zoneIdList.get(0)) == null)) { + if (zoneIdList != null && (storeMgr.getImageStoreForWrite(zoneIdList.get(0)) == null)) { throw new InvalidParameterValueException("Failed to find a secondary storage in the specified zone."); } @@ -620,7 +620,7 @@ public TemplateProfile prepareDelete(DeleteIsoCmd cmd) { List zoneIdList = profile.getZoneIdList(); if (zoneIdList != null && - (storeMgr.getImageStore(zoneIdList.get(0)) == null)) { + (storeMgr.getImageStoreForWrite(zoneIdList.get(0)) == null)) { throw new InvalidParameterValueException("Failed to find a secondary storage in the specified zone."); } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index 5eb96aac11df..b4697fd876ce 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -430,7 +430,7 @@ public DataStore getImageStore(String storeUuid, Long zoneId) { if (storeUuid != null) { imageStore = _dataStoreMgr.getDataStore(storeUuid, DataStoreRole.Image); } else { - imageStore = _dataStoreMgr.getImageStore(zoneId); + imageStore = _dataStoreMgr.getImageStoreForWrite(zoneId); if (imageStore == null) { throw new CloudRuntimeException("cannot find an image store for zone " + zoneId); } @@ -1358,7 +1358,7 @@ public boolean deleteIso(DeleteIsoCmd cmd) { throw new InvalidParameterValueException("Unable to delete iso, as it's used by other vms"); } - if (zoneId != null && (_dataStoreMgr.getImageStore(zoneId) == null)) { + if (zoneId != null && (_dataStoreMgr.getImageStoreForWrite(zoneId) == null)) { throw new InvalidParameterValueException("Failed to find a secondary storage store in the specified zone."); } @@ -1627,7 +1627,7 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t volume = _volumeDao.findById(volumeId); zoneId = volume.getDataCenterId(); } - DataStore store = _dataStoreMgr.getImageStore(zoneId); + DataStore store = _dataStoreMgr.getImageStoreForWrite(zoneId); if (store == null) { throw new CloudRuntimeException("cannot find an image store for zone " + zoneId); } @@ -1953,7 +1953,7 @@ public Pair getAbsoluteIsoPath(long templateId, long dataCenterI @Override public String getSecondaryStorageURL(long zoneId) { - DataStore secStore = _dataStoreMgr.getImageStore(zoneId); + DataStore secStore = _dataStoreMgr.getImageStoreForWrite(zoneId); if (secStore == null) { return null; } diff --git a/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java b/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java index 01713de389d6..988e41cc9f58 100644 --- a/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java +++ b/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java @@ -156,7 +156,7 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { _configDrivesNetworkElement._networkModel = _networkModel; - when(_dataStoreMgr.getImageStore(DATACENTERID)).thenReturn(dataStore); + when(_dataStoreMgr.getImageStoreForWrite(DATACENTERID)).thenReturn(dataStore); when(_ep.select(dataStore)).thenReturn(endpoint); when(_vmDao.findById(VMID)).thenReturn(virtualMachine); diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index 9259ba298b61..adcaaf540f5c 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -603,7 +603,7 @@ protected NetworkVO getDefaultNetworkForBasicZone(DataCenter dc) { } protected Map createSecStorageVmInstance(long dataCenterId, SecondaryStorageVm.Role role) { - DataStore secStore = _dataStoreMgr.getImageStore(dataCenterId); + DataStore secStore = _dataStoreMgr.getImageStoreForWrite(dataCenterId); if (secStore == null) { String msg = "No secondary storage available in zone " + dataCenterId + ", cannot create secondary storage vm"; s_logger.warn(msg); @@ -1117,7 +1117,7 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl Map details = _vmDetailsDao.listDetailsKeyPairs(vm.getId()); vm.setDetails(details); - DataStore secStore = _dataStoreMgr.getImageStore(dest.getDataCenter().getId()); + DataStore secStore = _dataStoreMgr.getImageStoreForWrite(dest.getDataCenter().getId()); if (secStore == null) { s_logger.error(String.format("Unable to finalize virtual machine profile as no secondary storage available to satisfy storage needs for zone: %s", dest.getDataCenter().getUuid())); return false; From 9b8c8d22dfd3314a61eac5f5cc7d9f3d73e45a76 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 17 Jul 2019 12:40:19 +0530 Subject: [PATCH 4/7] engine, vmware: mount all secondary storage for datacenter on migrate cmd Signed-off-by: Abhishek Kumar --- .../api/storage/DataStoreManager.java | 2 + .../ImageStoreProviderManagerImpl.java | 18 ++++++ .../datastore/DataStoreManagerImpl.java | 9 +++ .../datastore/ImageStoreProviderManager.java | 2 + .../vmware/manager/VmwareManager.java | 13 ++-- .../vmware/manager/VmwareManagerImpl.java | 27 ++++++++ .../vmware/resource/VmwareResource.java | 61 +++++++++++-------- 7 files changed, 102 insertions(+), 30 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java index 145678e8f61d..ecd8258b4e2d 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java @@ -37,6 +37,8 @@ public interface DataStoreManager { DataStore getImageStoreForWrite(long zoneId); + List getImageStoresForWrite(long zoneId); + List getImageCacheStores(Scope scope); DataStore getImageCacheStore(long zoneId); diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java index 064948a17f2c..f86f01a6f93a 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java @@ -177,4 +177,22 @@ public int compare(DataStore store1, DataStore store2) { Math.round(_statsCollector.getImageStoreCapacityThreshold()*100))); return null; } + + @Override + public List getImageStoresForWrite(List imageStores) { + List stores = new ArrayList<>(); + for (DataStore imageStore : imageStores) { + // Return image store if used percentage is less then threshold value i.e. 90%. + if (_statsCollector.imageStoreHasEnoughCapacity(imageStore)) { + stores.add(imageStore); + } + } + + // No store with space found + if (stores.isEmpty()) { + s_logger.error(String.format("Can't find image storage in zone with less than %d usage", + Math.round(_statsCollector.getImageStoreCapacityThreshold() * 100))); + } + return stores; + } } diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java index 66c4a1ff1437..e6b402071bfc 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java @@ -90,6 +90,15 @@ public DataStore getImageStoreForWrite(long zoneId) { return imageDataStoreMgr.getImageStoreForWrite(stores); } + @Override + public List getImageStoresForWrite(long zoneId) { + List stores = getImageStoresByScope(new ZoneScope(zoneId)); + if (stores == null || stores.size() == 0) { + return null; + } + return imageDataStoreMgr.getImageStoresForWrite(stores); + } + @Override public boolean isRegionStore(DataStore store) { if (store.getScope().getScopeType() == ScopeType.ZONE && store.getScope().getScopeId() == null) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java index f0cabd89bc3c..0efc98c407ac 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java @@ -45,4 +45,6 @@ public interface ImageStoreProviderManager { DataStore getImageStoreForRead(List imageStores); DataStore getImageStoreForWrite(List imageStores); + + List getImageStoresForWrite(List imageStores); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java index efdbc724fbde..8cc328a7a386 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManager.java @@ -16,16 +16,17 @@ // under the License. package com.cloud.hypervisor.vmware.manager; +import java.io.File; +import java.util.List; +import java.util.Map; + +import org.apache.cloudstack.framework.config.ConfigKey; + import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.vmware.mo.HostMO; import com.cloud.hypervisor.vmware.util.VmwareContext; import com.cloud.utils.Pair; import com.vmware.vim25.ManagedObjectReference; -import org.apache.cloudstack.framework.config.ConfigKey; - -import java.io.File; -import java.util.List; -import java.util.Map; public interface VmwareManager { public final String CONTEXT_STOCK_NAME = "vmwareMgr"; @@ -65,6 +66,8 @@ public interface VmwareManager { Pair getSecondaryStorageStoreUrlAndId(long dcId); + List> getSecondaryStorageStoresUrlAndIdList(long dcId); + File getSystemVMKeyFile(); VmwareStorageManager getStorageManager(); diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index 71966f093b93..07250697eca0 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -53,6 +53,7 @@ import org.apache.cloudstack.utils.identity.ManagementServerNode; import org.apache.log4j.Logger; +import com.amazonaws.util.CollectionUtils; import com.cloud.agent.AgentManager; import com.cloud.agent.Listener; import com.cloud.agent.api.AgentControlAnswer; @@ -513,6 +514,32 @@ public Pair getSecondaryStorageStoreUrlAndId(long dcId) { return new Pair(secUrl, secId); } + @Override + public List> getSecondaryStorageStoresUrlAndIdList(long dcId) { + List> urlIdList = new ArrayList<>(); + List secStores = _dataStoreMgr.getImageStoresForWrite(dcId); + if (!CollectionUtils.isNullOrEmpty(secStores)) { + for (DataStore secStore : secStores) { + if (secStore != null) { + urlIdList.add(new Pair<>(secStore.getUri(), secStore.getId())); + } + } + } + + if (urlIdList.isEmpty()) { + // we are using non-NFS image store, then use cache storage instead + s_logger.info("Secondary storage is not NFS, we need to use staging storage"); + DataStore cacheStore = _dataStoreMgr.getImageCacheStore(dcId); + if (cacheStore != null) { + urlIdList.add(new Pair<>(cacheStore.getUri(), cacheStore.getId())); + } else { + s_logger.warn("No staging storage is found when non-NFS secondary storage is used"); + } + } + + return urlIdList; + } + @Override public String getServiceConsolePortGroupName() { return _serviceConsoleName; diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index c195712e62ac..56fea1efcada 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -3860,19 +3860,24 @@ protected Answer execute(PrepareForMigrationCommand cmd) { prepareNetworkFromNicInfo(new HostMO(getServiceContext(), _morHyperHost), nic, false, cmd.getVirtualMachine().getType()); } - Pair secStoreUrlAndId = mgr.getSecondaryStorageStoreUrlAndId(Long.parseLong(_dcId)); - String secStoreUrl = secStoreUrlAndId.first(); - Long secStoreId = secStoreUrlAndId.second(); - if (secStoreUrl == null) { - String msg = "secondary storage for dc " + _dcId + " is not ready yet?"; - throw new Exception(msg); - } - mgr.prepareSecondaryStorageStore(secStoreUrl, secStoreId); + List> secStoreUrlAndIdList = mgr.getSecondaryStorageStoresUrlAndIdList(Long.parseLong(_dcId)); + for (Pair secStoreUrlAndId : secStoreUrlAndIdList) { + String secStoreUrl = secStoreUrlAndId.first(); + Long secStoreId = secStoreUrlAndId.second(); + if (secStoreUrl == null) { + String msg = String.format("Secondary storage for dc %s is not ready yet?", _dcId); + throw new Exception(msg); + } - ManagedObjectReference morSecDs = prepareSecondaryDatastoreOnHost(secStoreUrl); - if (morSecDs == null) { - String msg = "Failed to prepare secondary storage on host, secondary store url: " + secStoreUrl; - throw new Exception(msg); + if (vm.getType() != VirtualMachine.Type.User) { + mgr.prepareSecondaryStorageStore(secStoreUrl, secStoreId); + } + + ManagedObjectReference morSecDs = prepareSecondaryDatastoreOnHost(secStoreUrl); + if (morSecDs == null) { + String msg = "Failed to prepare secondary storage on host, secondary store url: " + secStoreUrl; + throw new Exception(msg); + } } return new PrepareForMigrationAnswer(cmd); } catch (Throwable e) { @@ -4239,19 +4244,25 @@ protected Answer execute(MigrateWithStorageCommand cmd) { prepareNetworkFromNicInfo(new HostMO(getServiceContext(), morTgtHost), nic, false, vmTo.getType()); } - // Ensure secondary storage mounted on target host - Pair secStoreUrlAndId = mgr.getSecondaryStorageStoreUrlAndId(Long.parseLong(_dcId)); - String secStoreUrl = secStoreUrlAndId.first(); - Long secStoreId = secStoreUrlAndId.second(); - if (secStoreUrl == null) { - String msg = "secondary storage for dc " + _dcId + " is not ready yet?"; - throw new Exception(msg); - } - mgr.prepareSecondaryStorageStore(secStoreUrl, secStoreId); - ManagedObjectReference morSecDs = prepareSecondaryDatastoreOnSpecificHost(secStoreUrl, tgtHyperHost); - if (morSecDs == null) { - String msg = "Failed to prepare secondary storage on host, secondary store url: " + secStoreUrl; - throw new Exception(msg); + // Ensure all secondary storage mounted on target host + List> secStoreUrlAndIdList = mgr.getSecondaryStorageStoresUrlAndIdList(Long.parseLong(_dcId)); + for (Pair secStoreUrlAndId : secStoreUrlAndIdList) { + String secStoreUrl = secStoreUrlAndId.first(); + Long secStoreId = secStoreUrlAndId.second(); + if (secStoreUrl == null) { + String msg = String.format("Secondary storage for dc %s is not ready yet?", _dcId); + throw new Exception(msg); + } + + if (vmTo.getType() != VirtualMachine.Type.User) { + mgr.prepareSecondaryStorageStore(secStoreUrl, secStoreId); + } + + ManagedObjectReference morSecDs = prepareSecondaryDatastoreOnSpecificHost(secStoreUrl, tgtHyperHost); + if (morSecDs == null) { + String msg = "Failed to prepare secondary storage on host, secondary store url: " + secStoreUrl; + throw new Exception(msg); + } } if (srcHostApiVersion.compareTo("5.1") < 0) { From f3be50bfe21b7c62328543326a77c9d612d3d178 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 17 Jul 2019 17:26:04 +0530 Subject: [PATCH 5/7] server: assume new store has free space Signed-off-by: Abhishek Kumar --- server/src/main/java/com/cloud/server/StatsCollector.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/com/cloud/server/StatsCollector.java b/server/src/main/java/com/cloud/server/StatsCollector.java index c6a5dab073aa..b2ccfe274a54 100644 --- a/server/src/main/java/com/cloud/server/StatsCollector.java +++ b/server/src/main/java/com/cloud/server/StatsCollector.java @@ -1362,6 +1362,9 @@ protected void sendMetricsToInfluxdb(Map metrics) { } public boolean imageStoreHasEnoughCapacity(DataStore imageStore) { + if (!_storageStats.keySet().contains(imageStore.getId())) { // Stats not available for this store yet, can be a new store. Better to assume it has enough capacity? + return true; + } StorageStats imageStoreStats = _storageStats.get(imageStore.getId()); if (imageStoreStats != null && (imageStoreStats.getByteUsed() / (imageStoreStats.getCapacityBytes() * 1.0)) <= _imageStoreCapacityThreshold) { return true; From e4966ea2204b9c8db4b7578c39a09e02450dd5ac Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 24 Jul 2019 11:16:18 +0530 Subject: [PATCH 6/7] refactorings Signed-off-by: Abhishek Kumar --- .../subsystem/api/storage/DataStoreManager.java | 6 +++--- .../allocator/StorageCacheRandomAllocator.java | 4 ++-- .../storage/motion/AncientDataMotionStrategy.java | 2 +- .../KvmNonManagedStorageDataMotionStrategy.java | 2 +- .../KvmNonManagedStorageSystemDataMotionTest.java | 4 ++-- .../manager/ImageStoreProviderManagerImpl.java | 6 +++--- .../storage/snapshot/SnapshotServiceImpl.java | 4 ++-- .../storage/datastore/DataStoreManagerImpl.java | 14 +++++++------- .../image/datastore/ImageStoreProviderManager.java | 6 +++--- .../hyperv/manager/HypervManagerImpl.java | 2 +- .../vmware/manager/VmwareManagerImpl.java | 4 ++-- .../network/element/ConfigDriveNetworkElement.java | 4 ++-- .../com/cloud/storage/VolumeApiServiceImpl.java | 2 +- .../cloud/storage/upload/UploadMonitorImpl.java | 2 +- .../cloud/template/HypervisorTemplateAdapter.java | 4 ++-- .../com/cloud/template/TemplateManagerImpl.java | 8 ++++---- .../element/ConfigDriveNetworkElementTest.java | 2 +- .../SecondaryStorageManagerImpl.java | 4 ++-- 18 files changed, 40 insertions(+), 40 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java index ecd8258b4e2d..ad5b1622cd22 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreManager.java @@ -33,11 +33,11 @@ public interface DataStoreManager { List getImageStoresByScope(ZoneScope scope); - DataStore getImageStoreForRead(long zoneId); + DataStore getRandomImageStore(long zoneId); - DataStore getImageStoreForWrite(long zoneId); + DataStore getImageStoreWithFreeCapacity(long zoneId); - List getImageStoresForWrite(long zoneId); + List listImageStoresWithFreeCapacity(long zoneId); List getImageCacheStores(Scope scope); diff --git a/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java index 86a281e1be4f..22b3f46a9463 100644 --- a/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java +++ b/engine/storage/cache/src/main/java/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java @@ -62,7 +62,7 @@ public DataStore getCacheStore(Scope scope) { return null; } - return imageStoreMgr.getImageStoreForWrite(cacheStores); + return imageStoreMgr.getImageStoreWithFreeCapacity(cacheStores); } @Override @@ -88,6 +88,6 @@ public DataStore getCacheStore(DataObject data, Scope scope) { } } } - return imageStoreMgr.getImageStoreForWrite(cacheStores); + return imageStoreMgr.getImageStoreWithFreeCapacity(cacheStores); } } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index 60887638ad12..39851b47b62f 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -328,7 +328,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) if (cacheStore == null) { // need to find a nfs or cifs image store, assuming that can't copy volume // directly to s3 - ImageStoreEntity imageStore = (ImageStoreEntity)dataStoreMgr.getImageStoreForWrite(destScope.getScopeId()); + ImageStoreEntity imageStore = (ImageStoreEntity)dataStoreMgr.getImageStoreWithFreeCapacity(destScope.getScopeId()); if (imageStore == null || !imageStore.getProtocol().equalsIgnoreCase("nfs") && !imageStore.getProtocol().equalsIgnoreCase("cifs")) { s_logger.debug("can't find a nfs (or cifs) image store to satisfy the need for a staging store"); return null; diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java index 30b6f1bf1119..e6b5c85b924b 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageDataMotionStrategy.java @@ -197,7 +197,7 @@ protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolum Host destHost) { VMTemplateStoragePoolVO sourceVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(destStoragePool.getId(), srcVolumeInfo.getTemplateId()); if (sourceVolumeTemplateStoragePoolVO == null && destStoragePool.getPoolType() == StoragePoolType.Filesystem) { - DataStore sourceTemplateDataStore = dataStoreManagerImpl.getImageStoreForRead(srcVolumeInfo.getDataCenterId()); + DataStore sourceTemplateDataStore = dataStoreManagerImpl.getRandomImageStore(srcVolumeInfo.getDataCenterId()); if (sourceTemplateDataStore != null) { TemplateInfo sourceTemplateInfo = templateDataFactory.getTemplate(srcVolumeInfo.getTemplateId(), sourceTemplateDataStore); TemplateObjectTO sourceTemplate = new TemplateObjectTO(sourceTemplateInfo); diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java index c7b802385064..3dfc4af409f5 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java @@ -353,7 +353,7 @@ private void configureAndTestcopyTemplateToTargetStorageIfNeeded(VMTemplateStora Mockito.when(sourceTemplateInfo.getHypervisorType()).thenReturn(HypervisorType.KVM); Mockito.when(vmTemplatePoolDao.findByPoolTemplate(Mockito.anyLong(), Mockito.anyLong())).thenReturn(vmTemplateStoragePoolVO); - Mockito.when(dataStoreManagerImpl.getImageStoreForRead(Mockito.anyLong())).thenReturn(sourceTemplateDataStore); + Mockito.when(dataStoreManagerImpl.getRandomImageStore(Mockito.anyLong())).thenReturn(sourceTemplateDataStore); Mockito.when(templateDataFactory.getTemplate(Mockito.anyLong(), Mockito.eq(sourceTemplateDataStore))).thenReturn(sourceTemplateInfo); Mockito.when(templateDataFactory.getTemplate(Mockito.anyLong(), Mockito.eq(destDataStore))).thenReturn(sourceTemplateInfo); kvmNonManagedStorageDataMotionStrategy.copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, srcStoragePool, destDataStore, destStoragePool, destHost); @@ -362,7 +362,7 @@ private void configureAndTestcopyTemplateToTargetStorageIfNeeded(VMTemplateStora InOrder verifyInOrder = Mockito.inOrder(vmTemplatePoolDao, dataStoreManagerImpl, templateDataFactory, kvmNonManagedStorageDataMotionStrategy); verifyInOrder.verify(vmTemplatePoolDao, Mockito.times(1)).findByPoolTemplate(Mockito.anyLong(), Mockito.anyLong()); - verifyInOrder.verify(dataStoreManagerImpl, Mockito.times(times)).getImageStoreForRead(Mockito.anyLong()); + verifyInOrder.verify(dataStoreManagerImpl, Mockito.times(times)).getRandomImageStore(Mockito.anyLong()); verifyInOrder.verify(templateDataFactory, Mockito.times(times)).getTemplate(Mockito.anyLong(), Mockito.eq(sourceTemplateDataStore)); verifyInOrder.verify(templateDataFactory, Mockito.times(times)).getTemplate(Mockito.anyLong(), Mockito.eq(destDataStore)); verifyInOrder.verify(kvmNonManagedStorageDataMotionStrategy, Mockito.times(times)).sendCopyCommand(Mockito.eq(destHost), Mockito.any(TemplateObjectTO.class), diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java index f86f01a6f93a..80e5b38f1f76 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java @@ -143,7 +143,7 @@ public List listImageCacheStores(Scope scope) { } @Override - public DataStore getImageStoreForRead(List imageStores) { + public DataStore getRandomImageStore(List imageStores) { if (imageStores.size() > 1) { Collections.shuffle(imageStores); } @@ -151,7 +151,7 @@ public DataStore getImageStoreForRead(List imageStores) { } @Override - public DataStore getImageStoreForWrite(List imageStores) { + public DataStore getImageStoreWithFreeCapacity(List imageStores) { if (imageStores.size() > 1) { imageStores.sort(new Comparator() { // Sort data stores based on free capacity @Override @@ -179,7 +179,7 @@ public int compare(DataStore store1, DataStore store2) { } @Override - public List getImageStoresForWrite(List imageStores) { + public List listImageStoresWithFreeCapacity(List imageStores) { List stores = new ArrayList<>(); for (DataStore imageStore : imageStores) { // Return image store if used percentage is less then threshold value i.e. 90%. diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java index 9f085f4893e8..51a2741dddb5 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java @@ -240,7 +240,7 @@ private DataStore findSnapshotImageStore(SnapshotInfo snapshot) { fullSnapshot = snapshotFullBackup; } if (fullSnapshot) { - return dataStoreMgr.getImageStoreForWrite(snapshot.getDataCenterId()); + return dataStoreMgr.getImageStoreWithFreeCapacity(snapshot.getDataCenterId()); } else { SnapshotInfo parentSnapshot = snapshot.getParent(); // Note that DataStore information in parentSnapshot is for primary @@ -251,7 +251,7 @@ private DataStore findSnapshotImageStore(SnapshotInfo snapshot) { parentSnapshotOnBackupStore = _snapshotStoreDao.findBySnapshot(parentSnapshot.getId(), DataStoreRole.Image); } if (parentSnapshotOnBackupStore == null) { - return dataStoreMgr.getImageStoreForWrite(snapshot.getDataCenterId()); + return dataStoreMgr.getImageStoreWithFreeCapacity(snapshot.getDataCenterId()); } return dataStoreMgr.getDataStore(parentSnapshotOnBackupStore.getDataStoreId(), parentSnapshotOnBackupStore.getRole()); } diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java index e6b402071bfc..51421e4cd3dd 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/datastore/DataStoreManagerImpl.java @@ -73,30 +73,30 @@ public List getImageStoresByScope(ZoneScope scope) { } @Override - public DataStore getImageStoreForRead(long zoneId) { + public DataStore getRandomImageStore(long zoneId) { List stores = getImageStoresByScope(new ZoneScope(zoneId)); if (stores == null || stores.size() == 0) { return null; } - return imageDataStoreMgr.getImageStoreForRead(stores); + return imageDataStoreMgr.getRandomImageStore(stores); } @Override - public DataStore getImageStoreForWrite(long zoneId) { + public DataStore getImageStoreWithFreeCapacity(long zoneId) { List stores = getImageStoresByScope(new ZoneScope(zoneId)); if (stores == null || stores.size() == 0) { return null; } - return imageDataStoreMgr.getImageStoreForWrite(stores); + return imageDataStoreMgr.getImageStoreWithFreeCapacity(stores); } @Override - public List getImageStoresForWrite(long zoneId) { + public List listImageStoresWithFreeCapacity(long zoneId) { List stores = getImageStoresByScope(new ZoneScope(zoneId)); if (stores == null || stores.size() == 0) { return null; } - return imageDataStoreMgr.getImageStoresForWrite(stores); + return imageDataStoreMgr.listImageStoresWithFreeCapacity(stores); } @Override @@ -128,7 +128,7 @@ public DataStore getImageCacheStore(long zoneId) { if (stores == null || stores.size() == 0) { return null; } - return imageDataStoreMgr.getImageStoreForWrite(stores); + return imageDataStoreMgr.getImageStoreWithFreeCapacity(stores); } @Override diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java index 0efc98c407ac..12c1eae1c8bc 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java @@ -42,9 +42,9 @@ public interface ImageStoreProviderManager { boolean registerDriver(String uuid, ImageStoreDriver driver); - DataStore getImageStoreForRead(List imageStores); + DataStore getRandomImageStore(List imageStores); - DataStore getImageStoreForWrite(List imageStores); + DataStore getImageStoreWithFreeCapacity(List imageStores); - List getImageStoresForWrite(List imageStores); + List listImageStoresWithFreeCapacity(List imageStores); } diff --git a/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java b/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java index 0bcd775a895c..09e454479448 100644 --- a/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java +++ b/plugins/hypervisors/hyperv/src/main/java/com/cloud/hypervisor/hyperv/manager/HypervManagerImpl.java @@ -137,7 +137,7 @@ public String prepareSecondaryStorageStore(long zoneId) { private String getSecondaryStorageStoreUrl(long zoneId) { String secUrl = null; - DataStore secStore = _dataStoreMgr.getImageStoreForWrite(zoneId); + DataStore secStore = _dataStoreMgr.getImageStoreWithFreeCapacity(zoneId); if (secStore != null) { secUrl = secStore.getUri(); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java index 07250697eca0..1d3c1ad9d69f 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/manager/VmwareManagerImpl.java @@ -493,7 +493,7 @@ public Pair getSecondaryStorageStoreUrlAndId(long dcId) { String secUrl = null; Long secId = null; - DataStore secStore = _dataStoreMgr.getImageStoreForWrite(dcId); + DataStore secStore = _dataStoreMgr.getImageStoreWithFreeCapacity(dcId); if (secStore != null) { secUrl = secStore.getUri(); secId = secStore.getId(); @@ -517,7 +517,7 @@ public Pair getSecondaryStorageStoreUrlAndId(long dcId) { @Override public List> getSecondaryStorageStoresUrlAndIdList(long dcId) { List> urlIdList = new ArrayList<>(); - List secStores = _dataStoreMgr.getImageStoresForWrite(dcId); + List secStores = _dataStoreMgr.listImageStoresWithFreeCapacity(dcId); if (!CollectionUtils.isNullOrEmpty(secStores)) { for (DataStore secStore : secStores) { if (secStore != null) { diff --git a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java index b79a06219495..e2c3ca7b5979 100644 --- a/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java +++ b/server/src/main/java/com/cloud/network/element/ConfigDriveNetworkElement.java @@ -337,7 +337,7 @@ private DataStore findDataStore(VirtualMachineProfile profile, DeployDestination dataStore = pickExistingRootVolumeFromDataStore(profile, dataStore); } } else { - dataStore = _dataStoreMgr.getImageStoreForWrite(dest.getDataCenter().getId()); + dataStore = _dataStoreMgr.getImageStoreWithFreeCapacity(dest.getDataCenter().getId()); } return dataStore; } @@ -449,7 +449,7 @@ private boolean createConfigDriveIso(VirtualMachineProfile profile, DeployDestin } private boolean deleteConfigDriveIso(final VirtualMachine vm) throws ResourceUnavailableException { - DataStore dataStore = _dataStoreMgr.getImageStoreForWrite(vm.getDataCenterId()); + DataStore dataStore = _dataStoreMgr.getImageStoreWithFreeCapacity(vm.getDataCenterId()); Long agentId = findAgentIdForImageStore(dataStore); if (VirtualMachineManager.VmConfigDriveOnPrimaryPool.value()) { diff --git a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java index 5b7269dc15c6..c0dab8616f20 100644 --- a/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/main/java/com/cloud/storage/VolumeApiServiceImpl.java @@ -2676,7 +2676,7 @@ private String orchestrateExtractVolume(long volumeId, long zoneId) { throw new InvalidParameterValueException("Volume to be extracted has been removed or not in right state!"); } // perform extraction - ImageStoreEntity secStore = (ImageStoreEntity)dataStoreMgr.getImageStoreForWrite(zoneId); + ImageStoreEntity secStore = (ImageStoreEntity)dataStoreMgr.getImageStoreWithFreeCapacity(zoneId); if (secStore == null) { throw new InvalidParameterValueException(String.format("Secondary storage to satisfy storage needs cannot be found for zone: %d", zoneId)); } diff --git a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java index 9028ecaf6181..64ada6dc3096 100644 --- a/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java +++ b/server/src/main/java/com/cloud/storage/upload/UploadMonitorImpl.java @@ -176,7 +176,7 @@ public Long extractTemplate(VMTemplateVO template, String url, TemplateDataStore Type type = (template.getFormat() == ImageFormat.ISO) ? Type.ISO : Type.TEMPLATE; - DataStore secStore = storeMgr.getImageStoreForWrite(dataCenterId); + DataStore secStore = storeMgr.getImageStoreWithFreeCapacity(dataCenterId); if(secStore == null) { s_logger.error("Unable to extract template, secondary storage to satisfy storage needs cannot be found!"); return null; diff --git a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java index fa4fce9da060..85c4a77774e8 100644 --- a/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java +++ b/server/src/main/java/com/cloud/template/HypervisorTemplateAdapter.java @@ -607,7 +607,7 @@ public TemplateProfile prepareDelete(DeleteTemplateCmd cmd) { throw new InvalidParameterValueException("The DomR template cannot be deleted."); } - if (zoneIdList != null && (storeMgr.getImageStoreForWrite(zoneIdList.get(0)) == null)) { + if (zoneIdList != null && (storeMgr.getImageStoreWithFreeCapacity(zoneIdList.get(0)) == null)) { throw new InvalidParameterValueException("Failed to find a secondary storage in the specified zone."); } @@ -620,7 +620,7 @@ public TemplateProfile prepareDelete(DeleteIsoCmd cmd) { List zoneIdList = profile.getZoneIdList(); if (zoneIdList != null && - (storeMgr.getImageStoreForWrite(zoneIdList.get(0)) == null)) { + (storeMgr.getImageStoreWithFreeCapacity(zoneIdList.get(0)) == null)) { throw new InvalidParameterValueException("Failed to find a secondary storage in the specified zone."); } diff --git a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java index b4697fd876ce..b049da088884 100755 --- a/server/src/main/java/com/cloud/template/TemplateManagerImpl.java +++ b/server/src/main/java/com/cloud/template/TemplateManagerImpl.java @@ -430,7 +430,7 @@ public DataStore getImageStore(String storeUuid, Long zoneId) { if (storeUuid != null) { imageStore = _dataStoreMgr.getDataStore(storeUuid, DataStoreRole.Image); } else { - imageStore = _dataStoreMgr.getImageStoreForWrite(zoneId); + imageStore = _dataStoreMgr.getImageStoreWithFreeCapacity(zoneId); if (imageStore == null) { throw new CloudRuntimeException("cannot find an image store for zone " + zoneId); } @@ -1358,7 +1358,7 @@ public boolean deleteIso(DeleteIsoCmd cmd) { throw new InvalidParameterValueException("Unable to delete iso, as it's used by other vms"); } - if (zoneId != null && (_dataStoreMgr.getImageStoreForWrite(zoneId) == null)) { + if (zoneId != null && (_dataStoreMgr.getImageStoreWithFreeCapacity(zoneId) == null)) { throw new InvalidParameterValueException("Failed to find a secondary storage store in the specified zone."); } @@ -1627,7 +1627,7 @@ public VirtualMachineTemplate createPrivateTemplate(CreateTemplateCmd command) t volume = _volumeDao.findById(volumeId); zoneId = volume.getDataCenterId(); } - DataStore store = _dataStoreMgr.getImageStoreForWrite(zoneId); + DataStore store = _dataStoreMgr.getImageStoreWithFreeCapacity(zoneId); if (store == null) { throw new CloudRuntimeException("cannot find an image store for zone " + zoneId); } @@ -1953,7 +1953,7 @@ public Pair getAbsoluteIsoPath(long templateId, long dataCenterI @Override public String getSecondaryStorageURL(long zoneId) { - DataStore secStore = _dataStoreMgr.getImageStoreForWrite(zoneId); + DataStore secStore = _dataStoreMgr.getImageStoreWithFreeCapacity(zoneId); if (secStore == null) { return null; } diff --git a/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java b/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java index 988e41cc9f58..d4f96c99e98f 100644 --- a/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java +++ b/server/src/test/java/com/cloud/network/element/ConfigDriveNetworkElementTest.java @@ -156,7 +156,7 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { _configDrivesNetworkElement._networkModel = _networkModel; - when(_dataStoreMgr.getImageStoreForWrite(DATACENTERID)).thenReturn(dataStore); + when(_dataStoreMgr.getImageStoreWithFreeCapacity(DATACENTERID)).thenReturn(dataStore); when(_ep.select(dataStore)).thenReturn(endpoint); when(_vmDao.findById(VMID)).thenReturn(virtualMachine); diff --git a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java index adcaaf540f5c..8b2ed40c15db 100644 --- a/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java +++ b/services/secondary-storage/controller/src/main/java/org/apache/cloudstack/secondarystorage/SecondaryStorageManagerImpl.java @@ -603,7 +603,7 @@ protected NetworkVO getDefaultNetworkForBasicZone(DataCenter dc) { } protected Map createSecStorageVmInstance(long dataCenterId, SecondaryStorageVm.Role role) { - DataStore secStore = _dataStoreMgr.getImageStoreForWrite(dataCenterId); + DataStore secStore = _dataStoreMgr.getImageStoreWithFreeCapacity(dataCenterId); if (secStore == null) { String msg = "No secondary storage available in zone " + dataCenterId + ", cannot create secondary storage vm"; s_logger.warn(msg); @@ -1117,7 +1117,7 @@ public boolean finalizeVirtualMachineProfile(VirtualMachineProfile profile, Depl Map details = _vmDetailsDao.listDetailsKeyPairs(vm.getId()); vm.setDetails(details); - DataStore secStore = _dataStoreMgr.getImageStoreForWrite(dest.getDataCenter().getId()); + DataStore secStore = _dataStoreMgr.getImageStoreWithFreeCapacity(dest.getDataCenter().getId()); if (secStore == null) { s_logger.error(String.format("Unable to finalize virtual machine profile as no secondary storage available to satisfy storage needs for zone: %s", dest.getDataCenter().getUuid())); return false; From e9e041503c2fdbef34c4e9b310b21bf8a281bada Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Wed, 24 Jul 2019 11:44:31 +0530 Subject: [PATCH 7/7] refactoring: method docs Signed-off-by: Abhishek Kumar --- .../datastore/ImageStoreProviderManager.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java index 12c1eae1c8bc..01f2100f77f1 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/image/datastore/ImageStoreProviderManager.java @@ -42,9 +42,38 @@ public interface ImageStoreProviderManager { boolean registerDriver(String uuid, ImageStoreDriver driver); + /** + * Return a random DataStore from the a list of DataStores. + * + * @param imageStores the list of image stores from which a random store + * to be returned + * @return random DataStore + */ DataStore getRandomImageStore(List imageStores); + /** + * Return a DataStore which has free capacity. Stores will be sorted + * based on their free space and capacity check will be done based on + * the predefined threshold value. If a store is full beyond the + * threshold it won't be considered for response. First store in the + * sorted list free capacity will be returned. When there is no store + * with free capacity in the list a null value will be returned. + * + * @param imageStores the list of image stores from which stores with free + * capacity stores to be returned + * @return the DataStore which has free capacity + */ DataStore getImageStoreWithFreeCapacity(List imageStores); + /** + * Return a list of DataStore which have free capacity. Free capacity check + * will be done based on the predefined threshold value. If a store is full + * beyond the threshold it won't be considered for response. An empty list + * will be returned when no store in the parameter list has free capacity. + * + * @param imageStores the list of image stores from which stores with free + * capacity stores to be returned + * @return the list of DataStore which have free capacity + */ List listImageStoresWithFreeCapacity(List imageStores); }