Copilot commented on code in PR #11291:
URL: https://github.com/apache/cloudstack/pull/11291#discussion_r2229268659
##########
engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java:
##########
@@ -595,6 +599,20 @@ public void updateSystemVMEntries(Long templateId,
Hypervisor.HypervisorType hyp
vmInstanceDao.updateSystemVmTemplateId(templateId, hypervisorType);
}
+ public void updateSystemVmTemplateGuestOsId() {
+ String systemVmGuestOsName = "Debian GNU/Linux 12 (64-bit)";
+ GuestOSVO guestOS =
guestOSDao.findOneByDisplayName(systemVmGuestOsName);
+ if (guestOS != null) {
+ LOGGER.debug("Updating SystemVM Template Guest OS [{}}] id",
systemVmGuestOsName);
+ SystemVmTemplateRegistration.LINUX_12_ID =
Math.toIntExact(guestOS.getId());
+ hypervisorGuestOsMap.put(Hypervisor.HypervisorType.KVM,
LINUX_12_ID);
+ hypervisorGuestOsMap.put(Hypervisor.HypervisorType.XenServer,
LINUX_12_ID);
+ hypervisorGuestOsMap.put(Hypervisor.HypervisorType.VMware,
LINUX_12_ID);
+ hypervisorGuestOsMap.put(Hypervisor.HypervisorType.Hyperv,
LINUX_12_ID);
+ hypervisorGuestOsMap.put(Hypervisor.HypervisorType.LXC,
LINUX_12_ID);
Review Comment:
The method updateSystemVmTemplateGuestOsId() silently fails if the guest OS
is not found. Consider adding error handling or logging when guestOS is null to
help diagnose configuration issues.
```suggestion
LOGGER.debug("Updating SystemVM Template Guest OS [{}] id",
systemVmGuestOsName);
SystemVmTemplateRegistration.LINUX_12_ID =
Math.toIntExact(guestOS.getId());
hypervisorGuestOsMap.put(Hypervisor.HypervisorType.KVM,
LINUX_12_ID);
hypervisorGuestOsMap.put(Hypervisor.HypervisorType.XenServer,
LINUX_12_ID);
hypervisorGuestOsMap.put(Hypervisor.HypervisorType.VMware,
LINUX_12_ID);
hypervisorGuestOsMap.put(Hypervisor.HypervisorType.Hyperv,
LINUX_12_ID);
hypervisorGuestOsMap.put(Hypervisor.HypervisorType.LXC,
LINUX_12_ID);
} else {
LOGGER.warn("Guest OS with display name [{}] not found. SystemVM
Template Guest OS ID update skipped.", systemVmGuestOsName);
```
##########
engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java:
##########
@@ -101,16 +104,14 @@ public class SystemVmTemplateRegistration {
public static final String TEMPORARY_SECONDARY_STORE = "tmp";
private static final String PARTIAL_TEMPLATE_FOLDER =
String.format("/template/tmpl/%d/", Account.ACCOUNT_ID_SYSTEM);
private static final String storageScriptsDir =
"scripts/storage/secondary";
- private static final Integer OTHER_LINUX_ID = 99;
- private static final Integer LINUX_5_ID = 15;
+ private static Integer LINUX_12_ID = 363;
Review Comment:
The hardcoded guest OS ID (363) creates a magic number that may not be valid
across different CloudStack deployments. Since the
updateSystemVmTemplateGuestOsId() method dynamically resolves this ID, consider
initializing LINUX_12_ID as null or using a placeholder value to make it clear
that it will be resolved at runtime.
```suggestion
// The guest OS ID for Linux 12 is resolved dynamically at runtime.
private static Integer LINUX_12_ID = null;
```
##########
engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java:
##########
@@ -595,6 +599,20 @@ public void updateSystemVMEntries(Long templateId,
Hypervisor.HypervisorType hyp
vmInstanceDao.updateSystemVmTemplateId(templateId, hypervisorType);
}
+ public void updateSystemVmTemplateGuestOsId() {
+ String systemVmGuestOsName = "Debian GNU/Linux 12 (64-bit)";
+ GuestOSVO guestOS =
guestOSDao.findOneByDisplayName(systemVmGuestOsName);
+ if (guestOS != null) {
+ LOGGER.debug("Updating SystemVM Template Guest OS [{}}] id",
systemVmGuestOsName);
Review Comment:
The log message contains an extra closing brace. It should be "Updating
SystemVM Template Guest OS [{}] id" instead of "Updating SystemVM Template
Guest OS [{}}] id".
```suggestion
LOGGER.debug("Updating SystemVM Template Guest OS [{}] id",
systemVmGuestOsName);
```
--
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]
