Copilot commented on code in PR #12874:
URL: https://github.com/apache/cloudstack/pull/12874#discussion_r3008226929
##########
plugins/hypervisors/simulator/src/main/java/com/cloud/resource/SimulatorDiscoverer.java:
##########
@@ -20,8 +20,10 @@
import java.net.URLDecoder;
import java.util.Date;
import java.util.HashMap;
+import java.util.HashSet;
Review Comment:
SimulatorDiscoverer no longer uses java.util.Date after switching to
TemplateService-based association, but the Date import remains. Unused imports
are compilation errors in Java; please remove this import (and any other
now-unused imports introduced by this refactor).
```suggestion
import java.util.HashMap;
```
##########
plugins/hypervisors/simulator/src/main/java/com/cloud/resource/SimulatorDiscoverer.java:
##########
@@ -50,6 +52,7 @@
import com.cloud.storage.VMTemplateZoneVO;
import com.cloud.storage.dao.VMTemplateDao;
import com.cloud.storage.dao.VMTemplateZoneDao;
+import org.apache.cloudstack.engine.subsystem.api.storage.TemplateService;
Review Comment:
After refactoring to call TemplateService in postDiscovery, the VMTemplate*
imports are no longer used in this class. These unused imports will fail
compilation; remove them (and consider dropping the corresponding unused
injected fields as well).
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -4232,7 +4232,6 @@ private void duplicateCacheStoreRecordsToRegionStore(long
storeId) {
private void associateCrosszoneTemplatesToZone(Long zoneId) {
VMTemplateZoneVO tmpltZone;
Review Comment:
This helper is still named `associateCrosszoneTemplatesToZone` while the
TemplateService API and most call sites now use
`associateCrossZoneTemplatesToZone`. Renaming this private method (and its call
at StorageManagerImpl.java:4004) would keep naming consistent and make grepping
for the behavior easier.
```suggestion
```
##########
services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/SecondaryStorageDiscoverer.java:
##########
@@ -285,23 +290,24 @@ public void postDiscovery(List<HostVO> hosts, long msId) {
_agentMgr.agentStatusTransitTo(h, Event.AgentDisconnected,
msId);
}
}
+ Set<Long> dcIds = new HashSet<>();
for (HostVO h : hosts) {
- associateTemplatesToZone(h.getId(), h.getDataCenterId());
+ dcIds.add(h.getDataCenterId());
+ }
+ for (Long dcId : dcIds) {
+ templateService.associateCrossZoneTemplatesToZone(dcId);
}
-
}
private void associateTemplatesToZone(long hostId, long dcId) {
VMTemplateZoneVO tmpltZone;
- List<VMTemplateVO> allTemplates = _vmTemplateDao.listAll();
- for (VMTemplateVO vt : allTemplates) {
- if (vt.isCrossZones()) {
- tmpltZone = _vmTemplateZoneDao.findByZoneTemplate(dcId,
vt.getId());
- if (tmpltZone == null) {
- VMTemplateZoneVO vmTemplateZone = new
VMTemplateZoneVO(dcId, vt.getId(), new Date());
- _vmTemplateZoneDao.persist(vmTemplateZone);
- }
+ List<VMTemplateVO> crossZoneTemplates =
_vmTemplateDao.listAllCrossZoneTemplates();
+ for (VMTemplateVO vt : crossZoneTemplates) {
+ tmpltZone = _vmTemplateZoneDao.findByZoneTemplate(dcId,
vt.getId());
+ if (tmpltZone == null) {
+ VMTemplateZoneVO vmTemplateZone = new VMTemplateZoneVO(dcId,
vt.getId(), new Date());
+ _vmTemplateZoneDao.persist(vmTemplateZone);
}
}
}
Review Comment:
postDiscovery now delegates cross-zone association to TemplateService, but
the private associateTemplatesToZone(...) helper remains and is no longer
called (and its hostId parameter is unused). Removing this unused method (and
the now-redundant VMTemplate* DAO dependencies it exists solely to support)
will reduce dead code and avoid maintaining two association paths.
##########
server/src/main/java/com/cloud/storage/StorageManagerImpl.java:
##########
@@ -4232,7 +4232,6 @@ private void duplicateCacheStoreRecordsToRegionStore(long
storeId) {
private void associateCrosszoneTemplatesToZone(Long zoneId) {
VMTemplateZoneVO tmpltZone;
Review Comment:
associateCrosszoneTemplatesToZone now just delegates to
_imageSrv.associateCrossZoneTemplatesToZone, so the local variable `tmpltZone`
is no longer used. Please remove the unused variable to keep the method clean
(and avoid IDE/compiler warnings).
```suggestion
```
--
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]