From 246c850beba4c1bc27ae61ece80f14b435659418 Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Thu, 6 Feb 2025 13:00:56 -0300 Subject: [PATCH 1/2] Prioritize copying templates from other secondary storages instead of downloading them --- .../service/StorageOrchestrationService.java | 6 + .../api/storage/TemplateService.java | 2 + .../com/cloud/storage/StorageManager.java | 4 + .../orchestration/DataMigrationUtility.java | 5 +- .../orchestration/StorageOrchestrator.java | 133 ++++++++++---- .../storage/image/TemplateServiceImpl.java | 169 +++++++++++++----- .../image/TemplateServiceImplTest.java | 102 +++++++++++ .../com/cloud/storage/StorageManagerImpl.java | 3 +- 8 files changed, 341 insertions(+), 83 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java index 481d0ebbc769..8be2015bfef6 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/StorageOrchestrationService.java @@ -18,12 +18,18 @@ package org.apache.cloudstack.engine.orchestration.service; import java.util.List; +import java.util.concurrent.Future; import org.apache.cloudstack.api.response.MigrationResponse; +import org.apache.cloudstack.engine.subsystem.api.storage.DataStore; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.storage.ImageStoreService.MigrationPolicy; public interface StorageOrchestrationService { MigrationResponse migrateData(Long srcDataStoreId, List destDatastores, MigrationPolicy migrationPolicy); MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreId, List templateIdList, List snapshotIdList); + + Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore); } diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java index 115cf024617f..a8861d5acc68 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/TemplateService.java @@ -78,4 +78,6 @@ public TemplateInfo getTemplate() { AsyncCallFuture createDatadiskTemplateAsync(TemplateInfo parentTemplate, TemplateInfo dataDiskTemplate, String path, String diskId, long fileSize, boolean bootable); List getTemplateDatadisksOnImageStore(TemplateInfo templateInfo, String configurationId); + + AsyncCallFuture copyTemplateToImageStore(DataObject source, DataStore destStore); } 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 7b31ec6a81b9..4196d98daee7 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 @@ -220,6 +220,10 @@ public interface StorageManager extends StorageService { "storage.pool.host.connect.workers", "1", "Number of worker threads to be used to connect hosts to a primary storage", true); + ConfigKey COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES = new ConfigKey<>(Boolean.class, "copy.public.templates.from.other.storages", + "Storage", "true", "Allow SSVMs to try copying public templates from one secondary storage to another instead of downloading them from the source.", + true, ConfigKey.Scope.Zone, null); + /** * should we execute in sequence not involving any storages? * @return tru if commands should execute in sequence diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java index c260f48dcf8c..89fc75415b16 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java @@ -299,10 +299,9 @@ protected List getAllReadyVolumes(DataStore srcDataStore) { /** Returns the count of active SSVMs - SSVM with agents in connected state, so as to dynamically increase the thread pool * size when SSVMs scale */ - protected int activeSSVMCount(DataStore dataStore) { - long datacenterId = dataStore.getScope().getScopeId(); + protected int activeSSVMCount(Long zoneId) { List ssvms = - secStorageVmDao.getSecStorageVmListInStates(null, datacenterId, VirtualMachine.State.Running, VirtualMachine.State.Migrating); + secStorageVmDao.getSecStorageVmListInStates(null, zoneId, VirtualMachine.State.Running, VirtualMachine.State.Migrating); int activeSSVMs = 0; for (SecondaryStorageVmVO vm : ssvms) { String name = "s-"+vm.getId()+"-VM"; diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index 0773c20b6b98..f9366c2df7fe 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -46,6 +46,8 @@ import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateInfo; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService; +import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService.TemplateApiResult; import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.Configurable; @@ -91,6 +93,8 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra @Inject private SecondaryStorageService secStgSrv; @Inject + TemplateService templateService; + @Inject TemplateDataStoreDao templateDataStoreDao; @Inject VolumeDataStoreDao volumeDataStoreDao; @@ -106,6 +110,8 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra Integer numConcurrentCopyTasksPerSSVM = 2; + private final Map zoneExecutorMap = new HashMap<>(); + @Override public String getConfigComponentName() { return StorageOrchestrationService.class.getName(); @@ -167,8 +173,6 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto double meanstddev = getStandardDeviation(storageCapacities); double threshold = ImageStoreImbalanceThreshold.value(); MigrationResponse response = null; - ThreadPoolExecutor executor = new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM , numConcurrentCopyTasksPerSSVM, 30, - TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM)); Date start = new Date(); if (meanstddev < threshold && migrationPolicy == MigrationPolicy.BALANCE) { logger.debug("mean std deviation of the image stores is below threshold, no migration required"); @@ -177,7 +181,7 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto } int skipped = 0; - List>> futures = new ArrayList<>(); + List> futures = new ArrayList<>(); while (true) { DataObject chosenFileForMigration = null; if (files.size() > 0) { @@ -206,7 +210,7 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto } if (shouldMigrate(chosenFileForMigration, srcDatastore.getId(), destDatastoreId, storageCapacities, snapshotChains, childTemplates, migrationPolicy)) { - storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destDatastoreId, executor, futures); + storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destDatastoreId, futures); } else { if (migrationPolicy == MigrationPolicy.BALANCE) { continue; @@ -217,7 +221,7 @@ public MigrationResponse migrateData(Long srcDataStoreId, List destDatasto } } Date end = new Date(); - handleSnapshotMigration(srcDataStoreId, start, end, migrationPolicy, futures, storageCapacities, executor); + handleSnapshotMigration(srcDataStoreId, start, end, migrationPolicy, futures, storageCapacities); return handleResponse(futures, migrationPolicy, message, success); } @@ -250,9 +254,7 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI storageCapacities = getStorageCapacities(storageCapacities, srcImgStoreId); storageCapacities = getStorageCapacities(storageCapacities, destImgStoreId); - ThreadPoolExecutor executor = new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, 30, - TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM)); - List>> futures = new ArrayList<>(); + List> futures = new ArrayList<>(); Date start = new Date(); while (true) { @@ -272,7 +274,7 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI } if (storageCapacityBelowThreshold(storageCapacities, destImgStoreId)) { - storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destImgStoreId, executor, futures); + storageCapacities = migrateAway(chosenFileForMigration, storageCapacities, snapshotChains, childTemplates, srcDatastore, destImgStoreId, futures); } else { message = "Migration failed. Destination store doesn't have enough capacity for migration"; success = false; @@ -289,7 +291,7 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI SnapshotInfo snapshotInfo = snapshotFactory.getSnapshot(snap.getSnapshotId(), snap.getDataStoreId(), DataStoreRole.Image); SnapshotInfo parentSnapshot = snapshotInfo.getParent(); if (snapshotInfo.getDataStore().getId() == srcImgStoreId && parentSnapshot != null && migratedSnapshotIdList.contains(parentSnapshot.getSnapshotId())) { - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, dataStoreManager.getDataStore(destImgStoreId, DataStoreRole.Image)))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, dataStoreManager.getDataStore(destImgStoreId, DataStoreRole.Image)))); } }); } @@ -297,6 +299,11 @@ public MigrationResponse migrateResources(Long srcImgStoreId, Long destImgStoreI return handleResponse(futures, null, message, success); } + @Override + public Future orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore) { + return submit(destStore.getScope().getScopeId(), new CopyTemplateTask(source, destStore)); + } + protected Pair migrateCompleted(Long destDatastoreId, DataStore srcDatastore, List files, MigrationPolicy migrationPolicy, int skipped) { String message = ""; boolean success = true; @@ -332,19 +339,10 @@ protected Map> migrateAway( Map, Long>> templateChains, DataStore srcDatastore, Long destDatastoreId, - ThreadPoolExecutor executor, - List>> futures) { + List> futures) { Long fileSize = migrationHelper.getFileSize(chosenFileForMigration, snapshotChains, templateChains); - storageCapacities = assumeMigrate(storageCapacities, srcDatastore.getId(), destDatastoreId, fileSize); - long activeSsvms = migrationHelper.activeSSVMCount(srcDatastore); - long totalJobs = activeSsvms * numConcurrentCopyTasksPerSSVM; - // Increase thread pool size with increase in number of SSVMs - if ( totalJobs > executor.getCorePoolSize()) { - executor.setMaximumPoolSize((int) (totalJobs)); - executor.setCorePoolSize((int) (totalJobs)); - } MigrateDataTask task = new MigrateDataTask(chosenFileForMigration, srcDatastore, dataStoreManager.getDataStore(destDatastoreId, DataStoreRole.Image)); if (chosenFileForMigration instanceof SnapshotInfo ) { @@ -353,19 +351,56 @@ protected Map> migrateAway( if (chosenFileForMigration instanceof TemplateInfo) { task.setTemplateChain(templateChains); } - futures.add((executor.submit(task))); + futures.add(submit(srcDatastore.getScope().getScopeId(), task)); logger.debug("Migration of {}: {} is initiated.", chosenFileForMigration.getType().name(), chosenFileForMigration.getUuid()); return storageCapacities; } + protected synchronized Future submit(Long zoneId, Callable task) { + if (!zoneExecutorMap.containsKey(zoneId)) { + zoneExecutorMap.put(zoneId, new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, + 30, TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM))); + } + scaleExecutorIfNecessary(zoneId); + return zoneExecutorMap.get(zoneId).submit(task); + } + + protected void scaleExecutorIfNecessary(Long zoneId) { + long activeSsvms = migrationHelper.activeSSVMCount(zoneId); + long totalJobs = activeSsvms * numConcurrentCopyTasksPerSSVM; + ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); + if (totalJobs > executor.getCorePoolSize()) { + logger.debug("Scaling up executor of zone [{}] from [{}] to [{}] threads.", zoneId, executor.getCorePoolSize(), + totalJobs); + executor.setMaximumPoolSize((int) (totalJobs)); + executor.setCorePoolSize((int) (totalJobs)); + } + } + + protected synchronized void tryCleaningUpExecutor(Long zoneId) { + if (!zoneExecutorMap.containsKey(zoneId)) { + logger.debug("No executor exists for zone [{}].", zoneId); + return; + } + ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); + int activeTasks = executor.getActiveCount(); + if (activeTasks > 1) { + logger.debug("Not cleaning executor of zone [{}] yet, as there are [{}] active tasks.", zoneId, activeTasks); + return; + } - private MigrationResponse handleResponse(List>> futures, MigrationPolicy migrationPolicy, String message, boolean success) { + logger.debug("Cleaning executor of zone [{}].", zoneId); + zoneExecutorMap.remove(zoneId); + executor.shutdown(); + } + + private MigrationResponse handleResponse(List> futures, MigrationPolicy migrationPolicy, String message, boolean success) { int successCount = 0; - for (Future> future : futures) { + for (Future future : futures) { try { - AsyncCallFuture res = future.get(); - if (res.get().isSuccess()) { + DataObjectResult res = future.get(); + if (res.isSuccess()) { successCount++; } } catch ( InterruptedException | ExecutionException e) { @@ -379,7 +414,7 @@ private MigrationResponse handleResponse(List>> futures, Map> storageCapacities, ThreadPoolExecutor executor) { + List> futures, Map> storageCapacities) { DataStore srcDatastore = dataStoreManager.getDataStore(srcDataStoreId, DataStoreRole.Image); List snaps = snapshotDataStoreDao.findSnapshots(srcDataStoreId, start, end); if (!snaps.isEmpty()) { @@ -395,12 +430,12 @@ private void handleSnapshotMigration(Long srcDataStoreId, Date start, Date end, storeId = dstores.get(1); } DataStore datastore = dataStoreManager.getDataStore(storeId, DataStoreRole.Image); - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, datastore))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, datastore))); } if (parentSnapshot != null) { DataStore parentDS = dataStoreManager.getDataStore(parentSnapshot.getDataStore().getId(), DataStoreRole.Image); if (parentDS.getId() != snapshotInfo.getDataStore().getId()) { - futures.add(executor.submit(new MigrateDataTask(snapshotInfo, srcDatastore, parentDS))); + futures.add(submit(srcDatastore.getScope().getScopeId(), new MigrateDataTask(snapshotInfo, srcDatastore, parentDS))); } } } @@ -527,7 +562,7 @@ private double calculateStorageStandardDeviation(double[] metricValues, double m return standardDeviation.evaluate(metricValues, mean); } - private class MigrateDataTask implements Callable> { + private class MigrateDataTask implements Callable { private DataObject file; private DataStore srcDataStore; private DataStore destDataStore; @@ -557,8 +592,44 @@ public DataObject getFile() { } @Override - public AsyncCallFuture call() throws Exception { - return secStgSrv.migrateData(file, srcDataStore, destDataStore, snapshotChain, templateChain); + public DataObjectResult call() { + DataObjectResult result; + AsyncCallFuture future = secStgSrv.migrateData(file, srcDataStore, destDataStore, snapshotChain, templateChain); + try { + result = future.get(); + } catch (ExecutionException | InterruptedException e) { + logger.warn("Exception while migrating data to another secondary storage: {}", e.toString()); + result = new DataObjectResult(file); + result.setResult(e.toString()); + } + tryCleaningUpExecutor(srcDataStore.getScope().getScopeId()); + return result; + } + } + + private class CopyTemplateTask implements Callable { + private TemplateInfo sourceTmpl; + private DataStore destStore; + + public CopyTemplateTask(TemplateInfo sourceTmpl, DataStore destStore) { + this.sourceTmpl = sourceTmpl; + this.destStore = destStore; + } + + @Override + public TemplateApiResult call() { + TemplateApiResult result; + AsyncCallFuture future = templateService.copyTemplateToImageStore(sourceTmpl, destStore); + try { + result = future.get(); + } catch (ExecutionException | InterruptedException e) { + logger.warn("Exception while copying template [{}] from image store [{}] to image store [{}]: {}", + sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), destStore.getName(), e.toString()); + result = new TemplateApiResult(sourceTmpl); + result.setResult(e.getMessage()); + } + tryCleaningUpExecutor(destStore.getScope().getScopeId()); + return result; } } } diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java index 38e0d0d081cb..aa09c924775a 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java @@ -31,6 +31,7 @@ import javax.inject.Inject; +import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.CreateCmdResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionService; @@ -42,7 +43,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.State; -import org.apache.cloudstack.engine.subsystem.api.storage.Scope; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; import org.apache.cloudstack.engine.subsystem.api.storage.StorageCacheManager; import org.apache.cloudstack.engine.subsystem.api.storage.TemplateDataFactory; @@ -77,7 +77,6 @@ import com.cloud.agent.api.to.DatadiskTO; import com.cloud.alert.AlertManager; import com.cloud.configuration.Config; -import com.cloud.configuration.Resource; import com.cloud.configuration.Resource.ResourceType; import com.cloud.dc.DataCenterVO; import com.cloud.dc.dao.ClusterDao; @@ -157,6 +156,8 @@ public class TemplateServiceImpl implements TemplateService { ImageStoreDetailsUtil imageStoreDetailsUtil; @Inject TemplateDataFactory imageFactory; + @Inject + StorageOrchestrationService storageOrchestrator; class TemplateOpContext extends AsyncRpcContext { final TemplateObject template; @@ -320,7 +321,6 @@ public void handleTemplateSync(DataStore store) { if (syncLock.lock(3)) { try { Long zoneId = store.getScope().getScopeId(); - Map templateInfos = listTemplate(store); if (templateInfos == null) { return; @@ -529,10 +529,6 @@ public void handleTemplateSync(DataStore store) { availHypers.add(HypervisorType.None); // bug 9809: resume ISO // download. for (VMTemplateVO tmplt : toBeDownloaded) { - if (tmplt.getUrl() == null) { // If url is null, skip downloading - logger.info("Skip downloading template {} since no url is specified.", tmplt); - continue; - } // if this is private template, skip sync to a new image store if (isSkipTemplateStoreDownload(tmplt, zoneId)) { logger.info("Skip sync downloading private template {} to a new image store", tmplt); @@ -551,14 +547,10 @@ public void handleTemplateSync(DataStore store) { } if (availHypers.contains(tmplt.getHypervisorType())) { - logger.info("Downloading template {} to image store {}", tmplt, store); - associateTemplateToZone(tmplt.getId(), zoneId); - TemplateInfo tmpl = _templateFactory.getTemplate(tmplt.getId(), store); - TemplateOpContext context = new TemplateOpContext<>(null,(TemplateObject)tmpl, null); - AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); - caller.setCallback(caller.getTarget().createTemplateAsyncCallBack(null, null)); - caller.setContext(context); - createTemplateAsync(tmpl, store, caller); + boolean copied = isCopyFromOtherStoragesEnabled(zoneId) && tryCopyingTemplateToImageStore(tmplt, store); + if (!copied) { + tryDownloadingTemplateToImageStore(tmplt, store); + } } else { logger.info("Skip downloading template {} since current data center does not have hypervisor {}", tmplt, tmplt.getHypervisorType()); } @@ -605,6 +597,118 @@ public void handleTemplateSync(DataStore store) { } + protected void tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { + if (tmplt.getUrl() == null) { + logger.info("Not downloading template [{}] to image store [{}], as it has no URL.", tmplt.getUniqueName(), + destStore.getName()); + return; + } + logger.info("Downloading template [{}] to image store [{}].", tmplt.getUniqueName(), destStore.getName()); + associateTemplateToZone(tmplt.getId(), destStore.getScope().getScopeId()); + TemplateInfo tmpl = _templateFactory.getTemplate(tmplt.getId(), destStore); + TemplateOpContext context = new TemplateOpContext<>(null,(TemplateObject)tmpl, null); + AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); + caller.setCallback(caller.getTarget().createTemplateAsyncCallBack(null, null)); + caller.setContext(context); + createTemplateAsync(tmpl, destStore, caller); + } + + protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, DataStore destStore) { + Long zoneId = destStore.getScope().getScopeId(); + List storesInZone = _storeMgr.getImageStoresByZoneIds(zoneId); + for (DataStore sourceStore : storesInZone) { + Map existingTemplatesInSourceStore = listTemplate(sourceStore); + if (existingTemplatesInSourceStore == null || !existingTemplatesInSourceStore.containsKey(tmplt.getUniqueName())) { + logger.debug("Template [{}] does not exist on image store [{}]; searching on another one.", + tmplt.getUniqueName(), sourceStore.getName()); + continue; + } + TemplateObject sourceTmpl = (TemplateObject) _templateFactory.getTemplate(tmplt.getId(), sourceStore); + if (sourceTmpl.getInstallPath() == null) { + logger.warn("Can not copy template [{}] from image store [{}], as it returned a null install path.", tmplt.getUniqueName(), + sourceStore.getName()); + continue; + } + storageOrchestrator.orchestrateTemplateCopyToImageStore(sourceTmpl, destStore); + return true; + } + logger.debug("Can't copy template [{}] from another image store.", tmplt.getUniqueName()); + return false; + } + + @Override + public AsyncCallFuture copyTemplateToImageStore(DataObject source, DataStore destStore) { + TemplateObject sourceTmpl = (TemplateObject) source; + logger.debug("Copying template [{}] from image store [{}] to [{}].", sourceTmpl.getUniqueName(), sourceTmpl.getDataStore().getName(), + destStore.getName()); + TemplateObject destTmpl = (TemplateObject) destStore.create(sourceTmpl); + destTmpl.processEvent(Event.CreateOnlyRequested); + + AsyncCallFuture future = new AsyncCallFuture<>(); + TemplateOpContext context = new TemplateOpContext<>(null, destTmpl, future); + AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); + caller.setCallback(caller.getTarget().copyTemplateToImageStoreCallback(null, null)).setContext(context); + _motionSrv.copyAsync(sourceTmpl, destTmpl, caller); + return future; + } + + protected Void copyTemplateToImageStoreCallback(AsyncCallbackDispatcher callback, TemplateOpContext context) { + TemplateInfo tmplt = context.getTemplate(); + CopyCommandResult result = callback.getResult(); + AsyncCallFuture future = context.getFuture(); + TemplateApiResult res = new TemplateApiResult(tmplt); + if (result.isSuccess()) { + logger.info("Copied template [{}] to image store [{}].", tmplt.getUniqueName(), tmplt.getDataStore().getName()); + tmplt.processEvent(Event.OperationSuccessed, result.getAnswer()); + publishTemplateCreation(tmplt); + } else { + logger.warn("Failed to copy template [{}] to image store [{}].", tmplt.getUniqueName(), tmplt.getDataStore().getName()); + res.setResult(result.getResult()); + tmplt.processEvent(Event.OperationFailed); + } + future.complete(res); + return null; + } + + protected boolean isCopyFromOtherStoragesEnabled(Long zoneId) { + return StorageManager.COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES.valueIn(zoneId); + } + + protected void publishTemplateCreation(TemplateInfo tmplt) { + VMTemplateVO tmpltVo = _templateDao.findById(tmplt.getId()); + + if (tmpltVo.isPublicTemplate()) { + _messageBus.publish(null, TemplateManager.MESSAGE_REGISTER_PUBLIC_TEMPLATE_EVENT, PublishScope.LOCAL, tmpltVo.getId()); + } + + Long size = tmplt.getSize(); + if (size == null) { + return; + } + + DataStore store = tmplt.getDataStore(); + TemplateDataStoreVO tmpltStore = _vmTemplateStoreDao.findByStoreTemplate(store.getId(), tmpltVo.getId()); + + long physicalSize = 0; + if (tmpltStore != null) { + physicalSize = tmpltStore.getPhysicalSize(); + } else { + logger.warn("No entry found in template_store_ref for template [{}] and image store [{}] at the end of registering template!", + tmpltVo.getUniqueName(), store.getName()); + } + + Long zoneId = store.getScope().getScopeId(); + if (zoneId != null) { + String usageEvent = tmplt.getFormat() == ImageFormat.ISO ? EventTypes.EVENT_ISO_CREATE : EventTypes.EVENT_TEMPLATE_CREATE; + UsageEventUtils.publishUsageEvent(usageEvent, tmpltVo.getAccountId(), zoneId, tmpltVo.getId(), tmpltVo.getName(), + null, null, physicalSize, size, VirtualMachineTemplate.class.getName(), tmpltVo.getUuid()); + } else { + logger.warn("Zone-wide image store [{}] has a null scope ID.", store); + } + + _resourceLimitMgr.incrementResourceCount(tmpltVo.getAccountId(), ResourceType.secondary_storage, size); + } + // persist entry in template_zone_ref table. zoneId can be empty for // region-wide image store, in that case, // we will associate the template to all the zones. @@ -650,45 +754,14 @@ public void associateCrosszoneTemplatesToZone(long dcId) { protected Void createTemplateAsyncCallBack(AsyncCallbackDispatcher callback, TemplateOpContext context) { - TemplateInfo template = context.template; TemplateApiResult result = callback.getResult(); if (result.isSuccess()) { - VMTemplateVO tmplt = _templateDao.findById(template.getId()); - // need to grant permission for public templates - if (tmplt.isPublicTemplate()) { - _messageBus.publish(null, TemplateManager.MESSAGE_REGISTER_PUBLIC_TEMPLATE_EVENT, PublishScope.LOCAL, tmplt.getId()); - } - long accountId = tmplt.getAccountId(); - if (template.getSize() != null) { - // publish usage event - String etype = EventTypes.EVENT_TEMPLATE_CREATE; - if (tmplt.getFormat() == ImageFormat.ISO) { - etype = EventTypes.EVENT_ISO_CREATE; - } - // get physical size from template_store_ref table - long physicalSize = 0; - DataStore ds = template.getDataStore(); - TemplateDataStoreVO tmpltStore = _vmTemplateStoreDao.findByStoreTemplate(ds.getId(), template.getId()); - if (tmpltStore != null) { - physicalSize = tmpltStore.getPhysicalSize(); - } else { - logger.warn("No entry found in template_store_ref for template: {} and image store: {} at the end of registering template!", template, ds); - } - Scope dsScope = ds.getScope(); - if (dsScope.getScopeId() != null) { - UsageEventUtils.publishUsageEvent(etype, template.getAccountId(), dsScope.getScopeId(), template.getId(), template.getName(), null, null, - physicalSize, template.getSize(), VirtualMachineTemplate.class.getName(), template.getUuid()); - } else { - logger.warn("Zone scope image store {} has a null scope id", ds); - } - _resourceLimitMgr.incrementResourceCount(accountId, Resource.ResourceType.secondary_storage, template.getSize()); - } + publishTemplateCreation(context.template); } - return null; } - private Map listTemplate(DataStore ssStore) { + protected Map listTemplate(DataStore ssStore) { String nfsVersion = imageStoreDetailsUtil.getNfsVersion(ssStore.getId()); ListTemplateCommand cmd = new ListTemplateCommand(ssStore.getTO(), nfsVersion); EndPoint ep = _epSelector.select(ssStore); diff --git a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java index bc6f37b201a6..276581e2e482 100644 --- a/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java +++ b/engine/storage/image/src/test/java/org/apache/cloudstack/storage/image/TemplateServiceImplTest.java @@ -18,9 +18,17 @@ */ package org.apache.cloudstack.storage.image; +import com.cloud.storage.template.TemplateProp; +import org.apache.cloudstack.engine.orchestration.service.StorageOrchestrationService; +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.Scope; +import org.apache.cloudstack.framework.async.AsyncCallFuture; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO; +import org.apache.cloudstack.storage.image.store.TemplateObject; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; @@ -33,6 +41,10 @@ import com.cloud.storage.Storage; import com.cloud.storage.VMTemplateVO; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + @RunWith(MockitoJUnitRunner.class) public class TemplateServiceImplTest { @@ -43,6 +55,49 @@ public class TemplateServiceImplTest { @Mock TemplateDataStoreDao templateDataStoreDao; + @Mock + TemplateDataFactoryImpl templateDataFactoryMock; + + @Mock + DataStoreManager dataStoreManagerMock; + + @Mock + VMTemplateVO tmpltMock; + + @Mock + TemplateProp tmpltPropMock; + + @Mock + TemplateObject templateInfoMock; + + @Mock + DataStore sourceStoreMock; + + @Mock + DataStore destStoreMock; + + @Mock + Scope zoneScopeMock; + + @Mock + StorageOrchestrationService storageOrchestrator; + + Map templatesInSourceStore = new HashMap<>(); + + @Before + public void setUp() { + Long zoneId = 1L; + Mockito.doReturn(2L).when(tmpltMock).getId(); + Mockito.doReturn("unique-name").when(tmpltMock).getUniqueName(); + Mockito.doReturn(zoneId).when(zoneScopeMock).getScopeId(); + Mockito.doReturn(zoneScopeMock).when(destStoreMock).getScope(); + Mockito.doReturn(List.of(sourceStoreMock, destStoreMock)).when(dataStoreManagerMock).getImageStoresByZoneIds(zoneId); + Mockito.doReturn(templatesInSourceStore).when(templateService).listTemplate(sourceStoreMock); + Mockito.doReturn(null).when(templateService).listTemplate(destStoreMock); + Mockito.doReturn("install-path").when(templateInfoMock).getInstallPath(); + Mockito.doReturn(templateInfoMock).when(templateDataFactoryMock).getTemplate(2L, sourceStoreMock); + } + @Test public void testIsSkipTemplateStoreDownloadPublicTemplate() { VMTemplateVO templateVO = Mockito.mock(VMTemplateVO.class); @@ -81,4 +136,51 @@ public void testIsSkipTemplateStoreDownloadPrivateExistingTemplate() { Mockito.when(templateDataStoreDao.findByTemplateZone(id, id, DataStoreRole.Image)).thenReturn(Mockito.mock(TemplateDataStoreVO.class)); Assert.assertTrue(templateService.isSkipTemplateStoreDownload(templateVO, id)); } + + @Test + public void tryDownloadingTemplateToImageStoreTestDownloadsTemplateWhenUrlIsNotNull() { + Mockito.doReturn("url").when(tmpltMock).getUrl(); + Mockito.doNothing().when(templateService).associateTemplateToZone(Mockito.anyLong(), Mockito.any(Long.class)); + + templateService.tryDownloadingTemplateToImageStore(tmpltMock, destStoreMock); + + Mockito.verify(templateService).createTemplateAsync(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void tryDownloadingTemplateToImageStoreTestDoesNothingWhenUrlIsNull() { + templateService.tryDownloadingTemplateToImageStore(tmpltMock, destStoreMock); + + Mockito.verify(templateService, Mockito.never()).createTemplateAsync(Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenTemplateDoesNotExistOnAnotherImageStore() { + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertFalse(result); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsFalseWhenInstallPathIsNull() { + templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock); + Mockito.doReturn(null).when(templateInfoMock).getInstallPath(); + + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertFalse(result); + Mockito.verify(storageOrchestrator, Mockito.never()).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + } + + @Test + public void tryCopyingTemplateToImageStoreTestReturnsTrueWhenTemplateExistsInAnotherStorageAndTaskWasScheduled() { + templatesInSourceStore.put(tmpltMock.getUniqueName(), tmpltPropMock); + Mockito.doReturn(new AsyncCallFuture<>()).when(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + + boolean result = templateService.tryCopyingTemplateToImageStore(tmpltMock, destStoreMock); + + Assert.assertTrue(result); + Mockito.verify(storageOrchestrator).orchestrateTemplateCopyToImageStore(Mockito.any(), Mockito.any()); + } } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 74b7f6f358b5..a9947f087b63 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -4168,7 +4168,8 @@ public ConfigKey[] getConfigKeys() { VmwareAllowParallelExecution, DataStoreDownloadFollowRedirects, AllowVolumeReSizeBeyondAllocation, - StoragePoolHostConnectWorkers + StoragePoolHostConnectWorkers, + COPY_PUBLIC_TEMPLATES_FROM_OTHER_STORAGES }; } From a2a95591559adb2d2497acd25be304199014482d Mon Sep 17 00:00:00 2001 From: Fabricio Duarte Date: Sat, 13 Dec 2025 21:03:55 -0300 Subject: [PATCH 2/2] Treat some corner cases --- .../orchestration/DataMigrationUtility.java | 85 +++++++++++++++---- .../orchestration/StorageOrchestrator.java | 41 ++++++--- .../storage/image/TemplateServiceImpl.java | 10 +++ 3 files changed, 109 insertions(+), 27 deletions(-) diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java index 86debf68a9f5..5a8dc3038aa8 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/DataMigrationUtility.java @@ -22,10 +22,12 @@ import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Set; import javax.inject.Inject; @@ -206,12 +208,22 @@ public int compare(DataObject o1, DataObject o2) { protected List getAllReadyTemplates(DataStore srcDataStore, Map, Long>> childTemplates, List templates) { List files = new LinkedList<>(); + Set idsForMigration = new HashSet<>(); + for (TemplateDataStoreVO template : templates) { - VMTemplateVO templateVO = templateDao.findById(template.getTemplateId()); - if (shouldMigrateTemplate(template, templateVO)) { - files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore)); + long templateId = template.getTemplateId(); + if (idsForMigration.contains(templateId)) { + logger.warn("Template store reference [{}] is duplicated; not considering it for migration.", template); + continue; + } + VMTemplateVO templateVO = templateDao.findById(templateId); + if (!shouldMigrateTemplate(template, templateVO)) { + continue; } + files.add(templateFactory.getTemplate(template.getTemplateId(), srcDataStore)); + idsForMigration.add(templateId); } + for (TemplateInfo template: files) { List children = templateDao.listByParentTemplatetId(template.getId()); List temps = new ArrayList<>(); @@ -221,6 +233,7 @@ protected List getAllReadyTemplates(DataStore srcDataStore, Map(temps, getTotalChainSize(temps))); } + return (List) (List) files; } @@ -263,16 +276,37 @@ protected boolean shouldMigrateTemplate(TemplateDataStoreVO template, VMTemplate */ protected List getAllReadySnapshotsAndChains(DataStore srcDataStore, Map, Long>> snapshotChains, List snapshots) { List files = new LinkedList<>(); + Set idsForMigration = new HashSet<>(); + for (SnapshotDataStoreVO snapshot : snapshots) { - SnapshotVO snapshotVO = snapshotDao.findById(snapshot.getSnapshotId()); - if (snapshot.getState() == ObjectInDataStoreStateMachine.State.Ready && - snapshotVO != null && snapshotVO.getHypervisorType() != Hypervisor.HypervisorType.Simulator - && snapshot.getParentSnapshotId() == 0 ) { - SnapshotInfo snap = snapshotFactory.getSnapshot(snapshotVO.getSnapshotId(), snapshot.getDataStoreId(), snapshot.getRole()); - if (snap != null) { - files.add(snap); - } + long snapshotId = snapshot.getSnapshotId(); + if (idsForMigration.contains(snapshotId)) { + logger.warn("Snapshot store reference [{}] is duplicated; not considering it for migration.", snapshot); + continue; + } + if (snapshot.getState() != ObjectInDataStoreStateMachine.State.Ready) { + logger.warn("Not migrating snapshot [{}] because its state is not ready.", snapshot); + continue; + } + SnapshotVO snapshotVO = snapshotDao.findById(snapshotId); + if (snapshotVO == null) { + logger.debug("Not migrating snapshot [{}] because we could not find its database entry.", snapshot); + continue; + } + if (snapshotVO.getHypervisorType() == Hypervisor.HypervisorType.Simulator) { + logger.debug("Not migrating snapshot [{}] because its hypervisor type is simulator.", snapshot); + continue; } + if (snapshot.getParentSnapshotId() != 0) { + continue; // The child snapshot will be migrated in the for loop below. + } + SnapshotInfo snap = snapshotFactory.getSnapshot(snapshotVO.getSnapshotId(), snapshot.getDataStoreId(), snapshot.getRole()); + if (snap == null) { + logger.debug("Not migrating snapshot [{}] because we could not get its information.", snapshot); + continue; + } + files.add(snap); + idsForMigration.add(snapshotId); } for (SnapshotInfo parent : files) { @@ -285,7 +319,7 @@ protected List getAllReadySnapshotsAndChains(DataStore srcDataStore, chain.addAll(children); } } - snapshotChains.put(parent, new Pair, Long>(chain, getTotalChainSize(chain))); + snapshotChains.put(parent, new Pair<>(chain, getTotalChainSize(chain))); } return (List) (List) files; @@ -306,14 +340,31 @@ protected Long getTotalChainSize(List chain) { protected List getAllReadyVolumes(DataStore srcDataStore, List volumes) { List files = new LinkedList<>(); + Set idsForMigration = new HashSet<>(); + for (VolumeDataStoreVO volume : volumes) { - if (volume.getState() == ObjectInDataStoreStateMachine.State.Ready) { - VolumeInfo volumeInfo = volumeFactory.getVolume(volume.getVolumeId(), srcDataStore); - if (volumeInfo != null && volumeInfo.getHypervisorType() != Hypervisor.HypervisorType.Simulator) { - files.add(volumeInfo); - } + long volumeId = volume.getVolumeId(); + if (idsForMigration.contains(volumeId)) { + logger.warn("Volume store reference [{}] is duplicated; not considering it for migration.", volume); + continue; } + if (volume.getState() != ObjectInDataStoreStateMachine.State.Ready) { + logger.debug("Not migrating volume [{}] because its state is not ready.", volume); + continue; + } + VolumeInfo volumeInfo = volumeFactory.getVolume(volume.getVolumeId(), srcDataStore); + if (volumeInfo == null) { + logger.debug("Not migrating volume [{}] because we could not get its information.", volume); + continue; + } + if (volumeInfo.getHypervisorType() == Hypervisor.HypervisorType.Simulator) { + logger.debug("Not migrating volume [{}] because its hypervisor type is simulator.", volume); + continue; + } + files.add(volumeInfo); + idsForMigration.add(volumeId); } + return files; } diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java index f9366c2df7fe..37a1f8dc196e 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java @@ -73,9 +73,12 @@ import com.cloud.utils.Pair; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.exception.CloudRuntimeException; +import org.apache.logging.log4j.ThreadContext; public class StorageOrchestrator extends ManagerBase implements StorageOrchestrationService, Configurable { + private static final String LOGCONTEXTID = "logcontextid"; + @Inject SnapshotDataStoreDao snapshotDataStoreDao; @Inject @@ -111,6 +114,7 @@ public class StorageOrchestrator extends ManagerBase implements StorageOrchestra Integer numConcurrentCopyTasksPerSSVM = 2; private final Map zoneExecutorMap = new HashMap<>(); + private final Map zonePendingWorkCountMap = new HashMap<>(); @Override public String getConfigComponentName() { @@ -356,13 +360,20 @@ protected Map> migrateAway( return storageCapacities; } - protected synchronized Future submit(Long zoneId, Callable task) { - if (!zoneExecutorMap.containsKey(zoneId)) { - zoneExecutorMap.put(zoneId, new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, - 30, TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM))); + protected Future submit(Long zoneId, Callable task) { + ThreadPoolExecutor executor; + synchronized (this) { + if (!zoneExecutorMap.containsKey(zoneId)) { + zoneExecutorMap.put(zoneId, new ThreadPoolExecutor(numConcurrentCopyTasksPerSSVM, numConcurrentCopyTasksPerSSVM, + 30, TimeUnit.MINUTES, new MigrateBlockingQueue<>(numConcurrentCopyTasksPerSSVM))); + zonePendingWorkCountMap.put(zoneId, 0); + } + zonePendingWorkCountMap.merge(zoneId, 1, Integer::sum); + scaleExecutorIfNecessary(zoneId); + executor = zoneExecutorMap.get(zoneId); } - scaleExecutorIfNecessary(zoneId); - return zoneExecutorMap.get(zoneId).submit(task); + return executor.submit(task); + } protected void scaleExecutorIfNecessary(Long zoneId) { @@ -383,14 +394,15 @@ protected synchronized void tryCleaningUpExecutor(Long zoneId) { return; } - ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); - int activeTasks = executor.getActiveCount(); - if (activeTasks > 1) { - logger.debug("Not cleaning executor of zone [{}] yet, as there are [{}] active tasks.", zoneId, activeTasks); + zonePendingWorkCountMap.merge(zoneId, -1, Integer::sum); + Integer pendingWorkCount = zonePendingWorkCountMap.get(zoneId); + if (pendingWorkCount > 0) { + logger.debug("Not cleaning executor of zone [{}] yet, as there is [{}] pending work.", zoneId, pendingWorkCount); return; } logger.debug("Cleaning executor of zone [{}].", zoneId); + ThreadPoolExecutor executor = zoneExecutorMap.get(zoneId); zoneExecutorMap.remove(zoneId); executor.shutdown(); } @@ -568,10 +580,13 @@ private class MigrateDataTask implements Callable { private DataStore destDataStore; private Map, Long>> snapshotChain; private Map, Long>> templateChain; + private String logid; + public MigrateDataTask(DataObject file, DataStore srcDataStore, DataStore destDataStore) { this.file = file; this.srcDataStore = srcDataStore; this.destDataStore = destDataStore; + this.logid = ThreadContext.get(LOGCONTEXTID); } public void setSnapshotChains(Map, Long>> snapshotChain) { @@ -593,6 +608,7 @@ public DataObject getFile() { @Override public DataObjectResult call() { + ThreadContext.put(LOGCONTEXTID, logid); DataObjectResult result; AsyncCallFuture future = secStgSrv.migrateData(file, srcDataStore, destDataStore, snapshotChain, templateChain); try { @@ -603,6 +619,7 @@ public DataObjectResult call() { result.setResult(e.toString()); } tryCleaningUpExecutor(srcDataStore.getScope().getScopeId()); + ThreadContext.clearAll(); return result; } } @@ -610,14 +627,17 @@ public DataObjectResult call() { private class CopyTemplateTask implements Callable { private TemplateInfo sourceTmpl; private DataStore destStore; + private String logid; public CopyTemplateTask(TemplateInfo sourceTmpl, DataStore destStore) { this.sourceTmpl = sourceTmpl; this.destStore = destStore; + this.logid = ThreadContext.get(LOGCONTEXTID); } @Override public TemplateApiResult call() { + ThreadContext.put(LOGCONTEXTID, logid); TemplateApiResult result; AsyncCallFuture future = templateService.copyTemplateToImageStore(sourceTmpl, destStore); try { @@ -629,6 +649,7 @@ public TemplateApiResult call() { result.setResult(e.getMessage()); } tryCleaningUpExecutor(destStore.getScope().getScopeId()); + ThreadContext.clearAll(); return result; } } diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java index aa09c924775a..fd723b8bf34f 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java @@ -58,6 +58,7 @@ import org.apache.cloudstack.framework.messagebus.MessageBus; import org.apache.cloudstack.framework.messagebus.PublishScope; import org.apache.cloudstack.storage.command.CommandResult; +import org.apache.cloudstack.storage.command.CopyCmdAnswer; import org.apache.cloudstack.storage.command.DeleteCommand; import org.apache.cloudstack.storage.datastore.DataObjectManager; import org.apache.cloudstack.storage.datastore.ObjectInDataStoreManager; @@ -648,6 +649,15 @@ public AsyncCallFuture copyTemplateToImageStore(DataObject so TemplateOpContext context = new TemplateOpContext<>(null, destTmpl, future); AsyncCallbackDispatcher caller = AsyncCallbackDispatcher.create(this); caller.setCallback(caller.getTarget().copyTemplateToImageStoreCallback(null, null)).setContext(context); + + if (source.getDataStore().getId() == destStore.getId()) { + logger.debug("Destination image store [{}] is the same as the origin; returning success to normalize the metadata."); + CopyCmdAnswer answer = new CopyCmdAnswer(source.getTO()); + CopyCommandResult result = new CopyCommandResult("", answer); + caller.complete(result); + return future; + } + _motionSrv.copyAsync(sourceTmpl, destTmpl, caller); return future; }