This is an automated email from the ASF dual-hosted git repository. gabriel pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/master by this push: new 7c5eca9 Copy template to target KVM host if needed when migrating local <> local storage (#3154) 7c5eca9 is described below commit 7c5eca94810c88a7e4a0849be1b1c23bf603e61b Author: Gabriel Beims Bräscher <gabrasc...@gmail.com> AuthorDate: Tue Feb 5 00:18:29 2019 -0200 Copy template to target KVM host if needed when migrating local <> local storage (#3154) * Migrate template to target host if needed. Fix KVM VM local storage live migration by migrating its template to the target host if needed. * Address reviewer and add method that updates the DB template reference * Remove deprecated Config.PrimaryStorageDownloadWait * Code formating of @Inject to follow checkstyle --- .../java/com/cloud/storage/StorageManager.java | 3 + .../storage/motion/AncientDataMotionStrategy.java | 7 +- .../KvmNonManagedStorageDataMotionStrategy.java | 97 ++++++++++++- .../motion/StorageSystemDataMotionStrategy.java | 104 ++++++++------ .../KvmNonManagedStorageSystemDataMotionTest.java | 150 ++++++++++++++++++++- .../CloudStackPrimaryDataStoreDriverImpl.java | 7 +- .../main/java/com/cloud/configuration/Config.java | 8 -- .../java/com/cloud/storage/StorageManagerImpl.java | 2 +- 8 files changed, 316 insertions(+), 62 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index c9c24d8..fd98101 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -101,6 +101,9 @@ public interface StorageManager extends StorageService { ConfigKey.Scope.Cluster, null); + ConfigKey<Integer> PRIMARY_STORAGE_DOWNLOAD_WAIT = new ConfigKey<Integer>("Storage", Integer.class, "primary.storage.download.wait", "10800", + "In second, timeout for download template to primary storage", false); + /** * Returns a comma separated list of tags for the specified storage pool * @param poolId 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 9471fad..7b52645 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 @@ -62,6 +62,7 @@ import com.cloud.configuration.Config; import com.cloud.host.Host; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.DataStoreRole; +import com.cloud.storage.StorageManager; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.StoragePool; import com.cloud.storage.VolumeVO; @@ -145,8 +146,7 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { } protected Answer copyObject(DataObject srcData, DataObject destData, Host destHost) { - String value = configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); - int _primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); Answer answer = null; DataObject cacheData = null; DataObject srcForCopy = srcData; @@ -156,7 +156,8 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { srcForCopy = cacheData = cacheMgr.createCacheObject(srcData, destScope); } - CopyCommand cmd = new CopyCommand(srcForCopy.getTO(), addFullCloneFlagOnVMwareDest(destData.getTO()), _primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); + CopyCommand cmd = new CopyCommand(srcForCopy.getTO(), addFullCloneFlagOnVMwareDest(destData.getTO()), primaryStorageDownloadWait, + VirtualMachineManager.ExecuteInSequence.value()); EndPoint ep = destHost != null ? RemoteHostEndPoint.getHypervisorHostEndPoint(destHost) : selector.select(srcForCopy, destData); if (ep == null) { String errMsg = "No remote endpoint to send command, check if host or ssvm is down?"; 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 bb75a66..2cf236d 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 @@ -29,7 +29,13 @@ 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.lang.StringUtils; +import org.apache.log4j.Logger; import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateCommand; @@ -37,14 +43,22 @@ import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo; import com.cloud.agent.api.storage.CreateAnswer; import com.cloud.agent.api.storage.CreateCommand; import com.cloud.agent.api.to.VirtualMachineTO; +import com.cloud.exception.AgentUnavailableException; +import com.cloud.exception.OperationTimedoutException; import com.cloud.host.Host; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.DataStoreRole; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.Storage.StoragePoolType; +import com.cloud.storage.StorageManager; +import com.cloud.storage.StoragePool; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.VMTemplateStorageResourceAssoc; import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DiskProfile; +import com.cloud.vm.VirtualMachineManager; /** * Extends {@link StorageSystemDataMotionStrategy}, allowing KVM hosts to migrate VMs with the ROOT volume on a non managed local storage pool. @@ -54,6 +68,14 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot @Inject private TemplateDataFactory templateDataFactory; + @Inject + private VMTemplatePoolDao vmTemplatePoolDao; + @Inject + private DataStoreManagerImpl dataStoreManagerImpl; + @Inject + private VirtualMachineManager virtualMachineManager; + + private static final Logger LOGGER = Logger.getLogger(KvmNonManagedStorageDataMotionStrategy.class); /** * Uses the canHandle from the Super class {@link StorageSystemDataMotionStrategy}. If the storage pool is of file and the internalCanHandle from {@link StorageSystemDataMotionStrategy} CANT_HANDLE, returns the StrategyPriority.HYPERVISOR strategy priority. otherwise returns CANT_HANDLE. @@ -95,7 +117,7 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot String templateUuid = getTemplateUuid(destVolumeInfo.getTemplateId()); CreateCommand rootImageProvisioningCommand = new CreateCommand(diskProfile, templateUuid, destStoragePool, true); - Answer rootImageProvisioningAnswer = _agentMgr.easySend(destHost.getId(), rootImageProvisioningCommand); + Answer rootImageProvisioningAnswer = agentManager.easySend(destHost.getId(), rootImageProvisioningCommand); if (rootImageProvisioningAnswer == null) { throw new CloudRuntimeException(String.format("Migration with storage of vm [%s] failed while provisioning root image", vmTO.getName())); @@ -140,4 +162,77 @@ public class KvmNonManagedStorageDataMotionStrategy extends StorageSystemDataMot protected boolean shouldMigrateVolume(StoragePoolVO sourceStoragePool, Host destHost, StoragePoolVO destStoragePool) { return sourceStoragePool.getPoolType() == StoragePoolType.Filesystem; } + + /** + * If the template is not on the target primary storage then it copies the template. + */ + @Override + protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolumeInfo, StoragePool srcStoragePool, DataStore destDataStore, StoragePool destStoragePool, + Host destHost) { + 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); + + 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); + + if (copyCommandAnswer != null && copyCommandAnswer.getResult()) { + updateTemplateReferenceIfSuccessfulCopy(srcVolumeInfo, srcStoragePool, destTemplateInfo, destDataStore); + } + } + } + + /** + * Update the template reference on table "template_spool_ref" (VMTemplateStoragePoolVO). + */ + protected void updateTemplateReferenceIfSuccessfulCopy(VolumeInfo srcVolumeInfo, StoragePool srcStoragePool, TemplateInfo destTemplateInfo, DataStore destDataStore) { + VMTemplateStoragePoolVO srcVolumeTemplateStoragePoolVO = vmTemplatePoolDao.findByPoolTemplate(srcStoragePool.getId(), srcVolumeInfo.getTemplateId()); + VMTemplateStoragePoolVO destVolumeTemplateStoragePoolVO = new VMTemplateStoragePoolVO(destDataStore.getId(), srcVolumeInfo.getTemplateId()); + destVolumeTemplateStoragePoolVO.setDownloadPercent(100); + destVolumeTemplateStoragePoolVO.setDownloadState(VMTemplateStorageResourceAssoc.Status.DOWNLOADED); + destVolumeTemplateStoragePoolVO.setState(ObjectInDataStoreStateMachine.State.Ready); + destVolumeTemplateStoragePoolVO.setTemplateSize(srcVolumeTemplateStoragePoolVO.getTemplateSize()); + destVolumeTemplateStoragePoolVO.setLocalDownloadPath(destTemplateInfo.getUuid()); + destVolumeTemplateStoragePoolVO.setInstallPath(destTemplateInfo.getUuid()); + vmTemplatePoolDao.persist(destVolumeTemplateStoragePoolVO); + } + + /** + * Sends the CopyCommand to migrate the template to the dest host. + */ + protected Answer sendCopyCommand(Host destHost, TemplateObjectTO sourceTemplate, TemplateObjectTO destTemplate, DataStore destDataStore) { + boolean executeInSequence = virtualMachineManager.getExecuteInSequence(HypervisorType.KVM); + CopyCommand copyCommand = new CopyCommand(sourceTemplate, destTemplate, StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(), executeInSequence); + try { + Answer copyCommandAnswer = agentManager.send(destHost.getId(), copyCommand); + logInCaseOfTemplateCopyFailure(copyCommandAnswer, sourceTemplate, destDataStore); + return copyCommandAnswer; + } catch (AgentUnavailableException | OperationTimedoutException e) { + throw new CloudRuntimeException(generateFailToCopyTemplateMessage(sourceTemplate, destDataStore), e); + } + } + + private String generateFailToCopyTemplateMessage(TemplateObjectTO sourceTemplate, DataStore destDataStore) { + return String.format("Failed to copy template [id=%s, name=%s] to the primary storage pool [id=%s].", sourceTemplate.getId(), + sourceTemplate.getName(), destDataStore.getId()); + } + + /** + * Logs in debug mode the copy command failure if the CopyCommand Answer has result as false. + */ + protected void logInCaseOfTemplateCopyFailure(Answer copyCommandAnswer, TemplateObjectTO sourceTemplate, DataStore destDataStore) { + if (copyCommandAnswer != null && !copyCommandAnswer.getResult()) { + String failureDetails = StringUtils.EMPTY; + if (copyCommandAnswer.getDetails() != null) { + failureDetails = " Details: " + copyCommandAnswer.getDetails(); + } + LOGGER.error(generateFailToCopyTemplateMessage(sourceTemplate, destDataStore) + failureDetails); + } + } } diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index adeb0d1..6bcaebe 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -112,7 +112,6 @@ import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.lang.StringUtils; -import org.apache.commons.lang.math.NumberUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -138,28 +137,45 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { private static final String OPERATION_NOT_SUPPORTED = "This operation is not supported."; @Inject - protected AgentManager _agentMgr; - @Inject private ConfigurationDao _configDao; - @Inject private DataStoreManager dataStoreMgr; + protected AgentManager agentManager; + @Inject + private ConfigurationDao _configDao; + @Inject + private DataStoreManager dataStoreMgr; @Inject protected DiskOfferingDao _diskOfferingDao; - @Inject private GuestOSCategoryDao _guestOsCategoryDao; - @Inject private GuestOSDao _guestOsDao; - @Inject private ClusterDao clusterDao; - @Inject private HostDao _hostDao; + @Inject + private GuestOSCategoryDao _guestOsCategoryDao; + @Inject + private GuestOSDao _guestOsDao; + @Inject + private ClusterDao clusterDao; + @Inject + private HostDao _hostDao; @Inject protected PrimaryDataStoreDao _storagePoolDao; - @Inject private SnapshotDao _snapshotDao; - @Inject private SnapshotDataStoreDao _snapshotDataStoreDao; - @Inject private SnapshotDetailsDao _snapshotDetailsDao; - @Inject private VMInstanceDao _vmDao; - @Inject private VMTemplateDao _vmTemplateDao; - @Inject private VolumeDao _volumeDao; - @Inject private VolumeDataFactory _volumeDataFactory; - @Inject private VolumeDetailsDao volumeDetailsDao; - @Inject private VolumeService _volumeService; - @Inject private StorageCacheManager cacheMgr; - @Inject private EndPointSelector selector; + @Inject + private SnapshotDao _snapshotDao; + @Inject + private SnapshotDataStoreDao _snapshotDataStoreDao; + @Inject + private SnapshotDetailsDao _snapshotDetailsDao; + @Inject + private VMInstanceDao _vmDao; + @Inject + private VMTemplateDao _vmTemplateDao; + @Inject + private VolumeDao _volumeDao; + @Inject + private VolumeDataFactory _volumeDataFactory; + @Inject + private VolumeDetailsDao volumeDetailsDao; + @Inject + private VolumeService _volumeService; + @Inject + private StorageCacheManager cacheMgr; + @Inject + private EndPointSelector selector; @Override public StrategyPriority canHandle(DataObject srcData, DataObject destData) { @@ -926,8 +942,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { } } - String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); - int primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(), destOnStore.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); @@ -953,7 +968,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { copyCommand.setOptions(srcDetails); - copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand); + copyCmdAnswer = (CopyCmdAnswer)agentManager.send(hostVO.getId(), copyCommand); if (!copyCmdAnswer.getResult()) { errMsg = copyCmdAnswer.getDetails(); @@ -1101,8 +1116,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { verifyCopyCmdAnswer(copyCmdAnswer, snapshotInfo); } - String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); - int primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); CopyCommand copyCommand = new CopyCommand(snapshotInfo.getTO(), volumeInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); @@ -1120,7 +1134,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { copyCommand.setOptions(srcDetails); - copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand); + copyCmdAnswer = (CopyCmdAnswer)agentManager.send(hostVO.getId(), copyCommand); if (!copyCmdAnswer.getResult()) { errMsg = copyCmdAnswer.getDetails(); @@ -1594,8 +1608,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { } private CopyCmdAnswer copyImageToVolume(DataObject srcDataObject, VolumeInfo destVolumeInfo, HostVO hostVO) { - String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); - int primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); CopyCommand copyCommand = new CopyCommand(srcDataObject.getTO(), destVolumeInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); @@ -1609,7 +1622,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { copyCommand.setOptions2(destDetails); - copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand); + copyCmdAnswer = (CopyCmdAnswer)agentManager.send(hostVO.getId(), copyCommand); } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) { String msg = "Failed to copy image : "; @@ -1724,6 +1737,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { continue; } + copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost); + VolumeVO destVolume = duplicateVolumeOnAnotherStorage(srcVolume, destStoragePool); VolumeInfo destVolumeInfo = _volumeDataFactory.getVolume(destVolume.getId(), destDataStore); @@ -1763,7 +1778,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { PrepareForMigrationCommand pfmc = new PrepareForMigrationCommand(vmTO); try { - Answer pfma = _agentMgr.send(destHost.getId(), pfmc); + Answer pfma = agentManager.send(destHost.getId(), pfmc); if (pfma == null || !pfma.getResult()) { String details = pfma != null ? pfma.getDetails() : "null answer returned"; @@ -1791,7 +1806,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { migrateCommand.setAutoConvergence(kvmAutoConvergence); - MigrateAnswer migrateAnswer = (MigrateAnswer)_agentMgr.send(srcHost.getId(), migrateCommand); + MigrateAnswer migrateAnswer = (MigrateAnswer)agentManager.send(srcHost.getId(), migrateCommand); boolean success = migrateAnswer != null && migrateAnswer.getResult(); @@ -1859,6 +1874,15 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { volume.setPath(volume.get_iScsiName()); } + /** + * For this strategy it is not necessary to copy the template before migrating the VM. + * However, classes that extend this one may need to copy the template to the target storage pool before migrating the VM. + */ + protected void copyTemplateToTargetFilesystemStorageIfNeeded(VolumeInfo srcVolumeInfo, StoragePool srcStoragePool, DataStore destDataStore, StoragePool destStoragePool, + Host destHost) { + // This method is used by classes that extend this one + } + private void handlePostMigration(boolean success, Map<VolumeInfo, VolumeInfo> srcVolumeInfoToDestVolumeInfo, VirtualMachineTO vmTO, Host destHost) { if (!success) { try { @@ -1866,7 +1890,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { pfmc.setRollback(true); - Answer pfma = _agentMgr.send(destHost.getId(), pfmc); + Answer pfma = agentManager.send(destHost.getId(), pfmc); if (pfma == null || !pfma.getResult()) { String details = pfma != null ? pfma.getDetails() : "null answer returned"; @@ -2007,7 +2031,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { } private List<String> sendModifyTargetsCommand(ModifyTargetsCommand cmd, long hostId) { - ModifyTargetsAnswer modifyTargetsAnswer = (ModifyTargetsAnswer)_agentMgr.easySend(hostId, cmd); + ModifyTargetsAnswer modifyTargetsAnswer = (ModifyTargetsAnswer)agentManager.easySend(hostId, cmd); if (modifyTargetsAnswer == null) { throw new CloudRuntimeException("Unable to get an answer to the modify targets command"); @@ -2109,8 +2133,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { HostVO hostVO = getHost(volumeInfo.getDataCenterId(), HypervisorType.KVM, false); DataStore srcDataStore = volumeInfo.getDataStore(); - String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); - int primaryStorageDownloadWait = NumberUtils.toInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); CopyCommand copyCommand = new CopyCommand(volumeInfo.getTO(), templateInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); @@ -2125,7 +2148,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { copyCommand.setOptions(srcDetails); - copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand); + copyCmdAnswer = (CopyCmdAnswer)agentManager.send(hostVO.getId(), copyCommand); if (!copyCmdAnswer.getResult()) { errMsg = copyCmdAnswer.getDetails(); @@ -2389,7 +2412,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { try { _volumeService.grantAccess(dataObj, hostVO, dataStore); - answer = (ResignatureAnswer)_agentMgr.send(hostVO.getId(), command); + answer = (ResignatureAnswer)agentManager.send(hostVO.getId(), command); } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) { keepGrantedAccess = false; @@ -2466,7 +2489,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { _volumeService.grantAccess(destVolumeInfo, hostVO, destVolumeInfo.getDataStore()); - MigrateVolumeAnswer migrateVolumeAnswer = (MigrateVolumeAnswer)_agentMgr.send(hostVO.getId(), migrateVolumeCommand); + MigrateVolumeAnswer migrateVolumeAnswer = (MigrateVolumeAnswer)agentManager.send(hostVO.getId(), migrateVolumeCommand); if (migrateVolumeAnswer == null || !migrateVolumeAnswer.getResult()) { if (migrateVolumeAnswer != null && !StringUtils.isEmpty(migrateVolumeAnswer.getDetails())) { @@ -2534,7 +2557,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { _volumeService.grantAccess(srcVolumeInfo, hostVO, srcVolumeInfo.getDataStore()); } - CopyVolumeAnswer copyVolumeAnswer = (CopyVolumeAnswer)_agentMgr.send(hostVO.getId(), copyVolumeCommand); + CopyVolumeAnswer copyVolumeAnswer = (CopyVolumeAnswer)agentManager.send(hostVO.getId(), copyVolumeCommand); if (copyVolumeAnswer == null || !copyVolumeAnswer.getResult()) { if (copyVolumeAnswer != null && !StringUtils.isEmpty(copyVolumeAnswer.getDetails())) { @@ -2592,8 +2615,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { private CopyCmdAnswer performCopyOfVdi(VolumeInfo volumeInfo, SnapshotInfo snapshotInfo, HostVO hostVO) { Snapshot.LocationType locationType = snapshotInfo.getLocationType(); - String value = _configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); - int primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); DataObject srcData = snapshotInfo; CopyCmdAnswer copyCmdAnswer = null; @@ -2623,7 +2645,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { copyCommand.setOptions2(destDetails); - copyCmdAnswer = (CopyCmdAnswer)_agentMgr.send(hostVO.getId(), copyCommand); + copyCmdAnswer = (CopyCmdAnswer)agentManager.send(hostVO.getId(), copyCommand); } catch (CloudRuntimeException | AgentUnavailableException | OperationTimedoutException ex) { String msg = "Failed to perform VDI copy : "; 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 b344f83..c0d8ad3 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 @@ -25,14 +25,22 @@ 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; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo; +import org.apache.cloudstack.storage.command.CopyCommand; +import org.apache.cloudstack.storage.datastore.DataStoreManagerImpl; import org.apache.cloudstack.storage.datastore.PrimaryDataStoreImpl; +import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.image.datastore.ImageStoreEntity; +import org.apache.cloudstack.storage.image.store.ImageStoreImpl; +import org.apache.cloudstack.storage.to.TemplateObjectTO; import org.apache.cloudstack.storage.volume.VolumeObject; import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.InOrder; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; @@ -40,37 +48,52 @@ import org.mockito.Spy; import org.mockito.runners.MockitoJUnitRunner; import com.cloud.agent.AgentManager; +import com.cloud.agent.api.Answer; import com.cloud.agent.api.MigrateCommand; import com.cloud.agent.api.storage.CreateAnswer; import com.cloud.agent.api.storage.CreateCommand; import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.agent.api.to.VolumeTO; +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.HypervisorType; import com.cloud.storage.DataStoreRole; import com.cloud.storage.DiskOfferingVO; import com.cloud.storage.Storage; +import com.cloud.storage.StoragePool; +import com.cloud.storage.VMTemplateStoragePoolVO; +import com.cloud.storage.Storage.ImageFormat; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; import com.cloud.storage.dao.DiskOfferingDao; +import com.cloud.storage.dao.VMTemplatePoolDao; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.DiskProfile; +import com.cloud.vm.VirtualMachineManager; @RunWith(MockitoJUnitRunner.class) public class KvmNonManagedStorageSystemDataMotionTest { @Mock private PrimaryDataStoreDao primaryDataStoreDao; - @Mock private TemplateDataFactory templateDataFactory; - @Mock private AgentManager agentManager; - @Mock private DiskOfferingDao diskOfferingDao; + @Mock + private VirtualMachineManager virtualMachineManager; + @Mock + private VMTemplatePoolDao vmTemplatePoolDao; + @Mock + private DataStoreManagerImpl dataStoreManagerImpl; + @Mock + private VolumeDataFactory volumeDataFactory; @Spy @InjectMocks @@ -253,4 +276,125 @@ public class KvmNonManagedStorageSystemDataMotionTest { } } } + + @Test + public void sendCopyCommandTest() throws AgentUnavailableException, OperationTimedoutException { + configureAndTestSendCommandTest(null); + } + + @Test(expected = CloudRuntimeException.class) + public void sendCopyCommandTestThrowAgentUnavailableException() throws AgentUnavailableException, OperationTimedoutException { + configureAndTestSendCommandTest(AgentUnavailableException.class); + } + + @Test(expected = CloudRuntimeException.class) + public void sendCopyCommandTestThrowOperationTimedoutException() throws AgentUnavailableException, OperationTimedoutException { + configureAndTestSendCommandTest(OperationTimedoutException.class); + } + + private void configureAndTestSendCommandTest(Class<? extends CloudException> exception) throws AgentUnavailableException, OperationTimedoutException { + Host destHost = new HostVO("guid"); + TemplateObjectTO sourceTemplate = new TemplateObjectTO(); + sourceTemplate.setName("name"); + sourceTemplate.setId(0l); + TemplateObjectTO destTemplate = new TemplateObjectTO(); + ImageStoreVO dataStoreVO = Mockito.mock(ImageStoreVO.class); + Mockito.when(dataStoreVO.getId()).thenReturn(0l); + + ImageStoreEntity destDataStore = Mockito.mock(ImageStoreImpl.class); + Mockito.doReturn(0l).when(destDataStore).getId(); + + Answer copyCommandAnswer = Mockito.mock(Answer.class); + + if (exception == null) { + Mockito.doReturn(copyCommandAnswer).when(agentManager).send(Mockito.anyLong(), Mockito.any(CopyCommand.class)); + } else { + Mockito.doThrow(exception).when(agentManager).send(Mockito.anyLong(), Mockito.any(CopyCommand.class)); + } + + Mockito.doNothing().when(kvmNonManagedStorageDataMotionStrategy).logInCaseOfTemplateCopyFailure(Mockito.any(Answer.class), Mockito.any(TemplateObjectTO.class), + Mockito.any(DataStore.class)); + + kvmNonManagedStorageDataMotionStrategy.sendCopyCommand(destHost, sourceTemplate, destTemplate, destDataStore); + + InOrder verifyInOrder = Mockito.inOrder(virtualMachineManager, agentManager, kvmNonManagedStorageDataMotionStrategy); + + verifyInOrder.verify(virtualMachineManager).getExecuteInSequence(HypervisorType.KVM); + verifyInOrder.verify(agentManager).send(Mockito.anyLong(), Mockito.any(CopyCommand.class)); + verifyInOrder.verify(kvmNonManagedStorageDataMotionStrategy).logInCaseOfTemplateCopyFailure(Mockito.any(Answer.class), Mockito.any(TemplateObjectTO.class), + Mockito.any(DataStore.class)); + } + + @Test + public void copyTemplateToTargetStorageIfNeededTestTemplateAlreadyOnTargetHost() throws AgentUnavailableException, OperationTimedoutException { + Answer copyCommandAnswer = Mockito.mock(Answer.class); + Mockito.when(copyCommandAnswer.getResult()).thenReturn(true); + configureAndTestcopyTemplateToTargetStorageIfNeeded(new VMTemplateStoragePoolVO(0l, 0l), StoragePoolType.Filesystem, 0); + } + + @Test + public void migrateTemplateToTargetStorageIfNeededTestTemplateNotOnTargetHost() throws AgentUnavailableException, OperationTimedoutException { + configureAndTestcopyTemplateToTargetStorageIfNeeded(null, StoragePoolType.Filesystem, 1); + } + + @Test + public void migrateTemplateToTargetStorageIfNeededTestNonDesiredStoragePoolType() throws AgentUnavailableException, OperationTimedoutException { + StoragePoolType[] storagePoolTypeArray = StoragePoolType.values(); + for (int i = 0; i < storagePoolTypeArray.length; i++) { + if (storagePoolTypeArray[i] == StoragePoolType.Filesystem) { + continue; + } + configureAndTestcopyTemplateToTargetStorageIfNeeded(new VMTemplateStoragePoolVO(0l, 0l), storagePoolTypeArray[i], 0); + } + } + + private void configureAndTestcopyTemplateToTargetStorageIfNeeded(VMTemplateStoragePoolVO vmTemplateStoragePoolVO, StoragePoolType storagePoolType, int times) { + DataStore destDataStore = Mockito.mock(DataStore.class); + Host destHost = Mockito.mock(Host.class); + + VolumeInfo srcVolumeInfo = Mockito.mock(VolumeInfo.class); + Mockito.when(srcVolumeInfo.getTemplateId()).thenReturn(0l); + + StoragePool srcStoragePool = Mockito.mock(StoragePool.class); + + VolumeInfo destVolumeInfo = Mockito.mock(VolumeInfo.class); + Mockito.when(volumeDataFactory.getVolume(Mockito.anyLong(), Mockito.any(DataStore.class))).thenReturn(destVolumeInfo); + + StoragePool destStoragePool = Mockito.mock(StoragePool.class); + Mockito.when(destStoragePool.getId()).thenReturn(0l); + Mockito.when(destStoragePool.getPoolType()).thenReturn(storagePoolType); + + DataStore sourceTemplateDataStore = Mockito.mock(DataStore.class); + Mockito.when(sourceTemplateDataStore.getName()).thenReturn("sourceTemplateName"); + + TemplateInfo sourceTemplateInfo = Mockito.mock(TemplateInfo.class); + Mockito.when(sourceTemplateInfo.getInstallPath()).thenReturn("installPath"); + Mockito.when(sourceTemplateInfo.getUuid()).thenReturn("uuid"); + Mockito.when(sourceTemplateInfo.getId()).thenReturn(0l); + Mockito.when(sourceTemplateInfo.getUrl()).thenReturn("url"); + Mockito.when(sourceTemplateInfo.getDisplayText()).thenReturn("display text"); + Mockito.when(sourceTemplateInfo.getChecksum()).thenReturn("checksum"); + Mockito.when(sourceTemplateInfo.isRequiresHvm()).thenReturn(true); + Mockito.when(sourceTemplateInfo.getAccountId()).thenReturn(0l); + Mockito.when(sourceTemplateInfo.getUniqueName()).thenReturn("unique name"); + Mockito.when(sourceTemplateInfo.getFormat()).thenReturn(ImageFormat.QCOW2); + Mockito.when(sourceTemplateInfo.getSize()).thenReturn(0l); + 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(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); + Mockito.doNothing().when(kvmNonManagedStorageDataMotionStrategy).updateTemplateReferenceIfSuccessfulCopy(Mockito.any(VolumeInfo.class), Mockito.any(StoragePool.class), + Mockito.any(TemplateInfo.class), Mockito.any(DataStore.class)); + + 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(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), + Mockito.any(TemplateObjectTO.class), Mockito.eq(destDataStore)); + } } diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java index 96a03b7..4153ba1 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/driver/CloudStackPrimaryDataStoreDriverImpl.java @@ -61,7 +61,6 @@ import com.cloud.agent.api.to.DataObjectType; import com.cloud.agent.api.to.DataStoreTO; import com.cloud.agent.api.to.DataTO; import com.cloud.agent.api.to.StorageFilerTO; -import com.cloud.configuration.Config; import com.cloud.exception.StorageUnavailableException; import com.cloud.host.Host; import com.cloud.host.dao.HostDao; @@ -77,7 +76,6 @@ import com.cloud.storage.dao.VMTemplateDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.storage.snapshot.SnapshotManager; import com.cloud.template.TemplateManager; -import com.cloud.utils.NumbersUtil; import com.cloud.vm.dao.VMInstanceDao; public class CloudStackPrimaryDataStoreDriverImpl implements PrimaryDataStoreDriver { @@ -256,13 +254,12 @@ public class CloudStackPrimaryDataStoreDriverImpl implements PrimaryDataStoreDri callback.complete(result); } else if (srcdata.getType() == DataObjectType.TEMPLATE && destData.getType() == DataObjectType.VOLUME) { //For CLVM, we need to pass template on secondary storage to hypervisor - String value = configDao.getValue(Config.PrimaryStorageDownloadWait.toString()); - int _primaryStorageDownloadWait = NumbersUtil.parseInt(value, Integer.parseInt(Config.PrimaryStorageDownloadWait.getDefaultValue())); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); StoragePoolVO storagePoolVO = primaryStoreDao.findById(store.getId()); DataStore imageStore = templateManager.getImageStore(storagePoolVO.getDataCenterId(), srcdata.getId()); DataObject srcData = templateDataFactory.getTemplate(srcdata.getId(), imageStore); - CopyCommand cmd = new CopyCommand(srcData.getTO(), destData.getTO(), _primaryStorageDownloadWait, true); + CopyCommand cmd = new CopyCommand(srcData.getTO(), destData.getTO(), primaryStorageDownloadWait, true); EndPoint ep = epSelector.select(srcData, destData); Answer answer = null; if (ep == null) { diff --git a/server/src/main/java/com/cloud/configuration/Config.java b/server/src/main/java/com/cloud/configuration/Config.java index eda34e5..d365ef0 100644 --- a/server/src/main/java/com/cloud/configuration/Config.java +++ b/server/src/main/java/com/cloud/configuration/Config.java @@ -187,14 +187,6 @@ public enum Config { "3600", "Timeout (in seconds) to synchronize storage pool operations.", null), - PrimaryStorageDownloadWait( - "Storage", - TemplateManager.class, - Integer.class, - "primary.storage.download.wait", - "10800", - "In second, timeout for download template to primary storage", - null), CreateVolumeFromSnapshotWait( "Storage", StorageManager.class, diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index a299540..40ecb84 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2489,7 +2489,7 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Override public ConfigKey<?>[] getConfigKeys() { return new ConfigKey<?>[] { StorageCleanupInterval, StorageCleanupDelay, StorageCleanupEnabled, TemplateCleanupEnabled, - KvmStorageOfflineMigrationWait, KvmStorageOnlineMigrationWait, MaxNumberOfManagedClusteredFileSystems }; + KvmStorageOfflineMigrationWait, KvmStorageOnlineMigrationWait, MaxNumberOfManagedClusteredFileSystems, PRIMARY_STORAGE_DOWNLOAD_WAIT}; } @Override