JoaoJandre commented on code in PR #10363:
URL: https://github.com/apache/cloudstack/pull/10363#discussion_r1970292746


##########
engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/TemplateServiceImpl.java:
##########
@@ -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<TemplateApiResult> context = new 
TemplateOpContext<>(null,(TemplateObject)tmpl, null);
+        AsyncCallbackDispatcher<TemplateServiceImpl, TemplateApiResult> 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<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());
+                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<TemplateApiResult> 
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<TemplateApiResult> future = new AsyncCallFuture<>();
+        TemplateOpContext<TemplateApiResult> context = new 
TemplateOpContext<>(null, destTmpl, future);
+        AsyncCallbackDispatcher<TemplateServiceImpl, CopyCommandResult> caller 
= AsyncCallbackDispatcher.create(this);
+        
caller.setCallback(caller.getTarget().copyTemplateToImageStoreCallback(null, 
null)).setContext(context);
+        _motionSrv.copyAsync(sourceTmpl, destTmpl, caller);
+        return future;
+    }
+
+    protected Void 
copyTemplateToImageStoreCallback(AsyncCallbackDispatcher<TemplateServiceImpl, 
CopyCommandResult> callback, TemplateOpContext<TemplateApiResult> context) {
+        TemplateInfo tmplt = context.getTemplate();
+        CopyCommandResult result = callback.getResult();
+        AsyncCallFuture<TemplateApiResult> 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());

Review Comment:
   If we cannot find the reference on the database, the template will be 
useless anyway. It will not even be listed by the template API. Thus, it would 
be an easy miss that it would be using resources (even if we update the usage 
count). 
   
   An easier way to find about an inconsistent template is throwing an error 
and stopping the process when we find out that the template is inconsistent. 
Then the operator can go to the storage and delete it, and fix the database if 
needed. Updating the usage count will only make the situation worse by adding 
more stuff to normalize on the database later.



-- 
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: commits-unsubscr...@cloudstack.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to