winterhazel commented on code in PR #12296:
URL: https://github.com/apache/cloudstack/pull/12296#discussion_r2714393468


##########
engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java:
##########
@@ -615,28 +625,134 @@ protected void 
tryDownloadingTemplateToImageStore(VMTemplateVO tmplt, DataStore
     }
 
     protected boolean tryCopyingTemplateToImageStore(VMTemplateVO tmplt, 
DataStore destStore) {
-        Long zoneId = destStore.getScope().getScopeId();
-        List<DataStore> storesInZone = 
_storeMgr.getImageStoresByZoneIds(zoneId);
-        for (DataStore sourceStore : storesInZone) {
-            Map<String, TemplateProp> 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());
+        if (searchAndCopyWithinZone(tmplt, destStore)) {
+            return true;
+        }
+
+        Long destZoneId = destStore.getScope().getScopeId();
+        logger.debug("Template [{}] not found in any image store of zone [{}]. 
Checking other zones.",
+                tmplt.getUniqueName(), destZoneId);
+
+        return searchAndCopyAcrossZones(tmplt, destStore, destZoneId);
+    }
+
+    private boolean searchAndCopyAcrossZones(VMTemplateVO tmplt, DataStore 
destStore, Long destZoneId) {
+        List<Long> allZoneIds = _dcDao.listAllIds();
+        for (Long otherZoneId : allZoneIds) {
+            if (otherZoneId.equals(destZoneId)) {
                 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());
+
+            List<DataStore> storesInOtherZone = 
_storeMgr.getImageStoresByZoneIds(otherZoneId);
+            logger.debug("Checking zone [{}] for template [{}]...", 
otherZoneId, tmplt.getUniqueName());
+
+            if (storesInOtherZone == null || storesInOtherZone.isEmpty()) {

Review Comment:
   Just a nitpick: you can simply use 
CollectionUtils.isEmpty(storesInOtherZone) here ;)



##########
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/StorageOrchestrator.java:
##########
@@ -304,8 +317,9 @@ public MigrationResponse migrateResources(Long 
srcImgStoreId, Long destImgStoreI
     }
 
     @Override
-    public Future<TemplateApiResult> 
orchestrateTemplateCopyToImageStore(TemplateInfo source, DataStore destStore) {
-        return submit(destStore.getScope().getScopeId(), new 
CopyTemplateTask(source, destStore));

Review Comment:
   As class `CopyTemplateTask` is not used anymore, could you remove it?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to