Skip to content

SharedFS: fix NPE when accessing systemvm templates for unsupported hypervisors #13930

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abh1sar wants to merge 3 commits into
base: 4.22
Choose a base branch
from
Open

SharedFS: fix NPE when accessing systemvm templates for unsupported hypervisors #13930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5fb68f8
fix NPE when accessing systemvm templates for unsupported hypervisors…
abh1sar Aug 20, 2026
099e74a
systemvm -> SystemVM
abh1sar Aug 20, 2026
ac688c7
Add unit test for skip-hypervisor-without-template fix in deployShare…
abh1sar Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/src/main/java/com/cloud/resource/ResourceService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public interface ResourceService {

DataCenter getZone(Long zoneId);

List<HypervisorType> getSupportedHypervisorTypes(long zoneId, boolean forVirtualRouter, Long podId);
List<HypervisorType> getSupportedHypervisorTypes(long zoneId, boolean forSystemVm, Long podId);

boolean releaseHostReservation(Long hostId);

Expand Down
11 changes: 8 additions & 3 deletions ...ain/java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycle.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ private UserVm deploySharedFSVM(Long zoneId, Account owner, List<Long> networkId
ServiceOffering serviceOffering = serviceOfferingDao.findById(serviceOfferingId);
DataCenter zone = dataCenterDao.findById(zoneId);

List<Hypervisor.HypervisorType> hypervisors = resourceMgr.getSupportedHypervisorTypes(zoneId, false, null);
List<Hypervisor.HypervisorType> hypervisors = resourceMgr.getSupportedHypervisorTypes(zoneId, true, null);
if (hypervisors.size() > 0) {
Collections.shuffle(hypervisors);
} else {
Expand All @@ -180,8 +180,13 @@ private UserVm deploySharedFSVM(Long zoneId, Account owner, List<Long> networkId
for (final Iterator<Hypervisor.HypervisorType> iter = hypervisors.iterator(); iter.hasNext();) {
final Hypervisor.HypervisorType hypervisor = iter.next();
VMTemplateVO template = templateDao.findSystemVMReadyTemplate(zoneId, hypervisor, preferredArchitecture);
if (template == null && !iter.hasNext()) {
throw new CloudRuntimeException(String.format("Unable to find the systemvm template for %s or it was not downloaded in %s.", hypervisor.toString(), zone.toString()));
if (template == null) {
if (iter.hasNext()) {
continue;
} else {
Comment thread
abh1sar marked this conversation as resolved.
throw new CloudRuntimeException(String.format("Unable to find the SystemVM template for any of the available hypervisors in zone %s.", zone.toString()));
}

}

LaunchPermissionVO existingPermission = launchPermissionDao.findByTemplateAndAccount(template.getId(), owner.getId());
Expand Down
62 changes: 60 additions & 2 deletions ...java/org/apache/cloudstack/storage/sharedfs/lifecycle/StorageVmSharedFSLifeCycleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import com.cloud.vm.dao.NicDao;
import com.cloud.vm.dao.UserVmDao;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import org.apache.cloudstack.api.ApiCommandResourceType;
Expand Down Expand Up @@ -80,9 +81,12 @@
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@RunWith(MockitoJUnitRunner.class)
Expand Down Expand Up @@ -229,7 +233,7 @@ private SharedFS prepareDeploySharedFS() throws ResourceUnavailableException, In

DataCenterVO zone = mock(DataCenterVO.class);
when(dataCenterDao.findById(s_zoneId)).thenReturn(zone);
when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, false, null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));
when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, true, null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));

ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
when(serviceOfferingDao.findById(s_serviceOfferingId)).thenReturn(serviceOffering);
Expand Down Expand Up @@ -300,11 +304,65 @@ public void testDeploySharedFSTemplateNotFound() throws ResourceUnavailableExcep
when(accountMgr.getActiveAccountById(s_ownerId)).thenReturn(null);
DataCenterVO zone = mock(DataCenterVO.class);
when(dataCenterDao.findById(s_zoneId)).thenReturn(zone);
when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, false, null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));
when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, true, null)).thenReturn(List.of(Hypervisor.HypervisorType.KVM));

lifeCycle.deploySharedFS(sharedFS, s_networkId, s_diskOfferingId, s_size, s_minIops, s_maxIops);
}

@Test
public void testDeploySharedFSSkipsHypervisorWithoutTemplate() throws ResourceUnavailableException, InsufficientCapacityException, ResourceAllocationException, IOException, OperationTimedoutException {
// Verifies that when the systemvm template is missing for one of the supported hypervisors,
// deploySharedFSVM() skips it and tries the next one instead of failing with an NPE.
SharedFS sharedFS = mock(SharedFS.class);
when(sharedFS.getDataCenterId()).thenReturn(s_zoneId);
when(sharedFS.getName()).thenReturn(s_name);
when(sharedFS.getServiceOfferingId()).thenReturn(s_serviceOfferingId);
when(sharedFS.getFsType()).thenReturn(SharedFS.FileSystemType.valueOf(s_fsFormat));
when(sharedFS.getAccountId()).thenReturn(s_ownerId);

Account owner = mock(Account.class);
when(owner.getId()).thenReturn(s_ownerId);
when(accountMgr.getActiveAccountById(s_ownerId)).thenReturn(owner);

DataCenterVO zone = mock(DataCenterVO.class);
when(dataCenterDao.findById(s_zoneId)).thenReturn(zone);

List<Hypervisor.HypervisorType> hypervisors = new ArrayList<>(List.of(Hypervisor.HypervisorType.XenServer, Hypervisor.HypervisorType.KVM));
when(resourceMgr.getSupportedHypervisorTypes(s_zoneId, true, null)).thenReturn(hypervisors);

ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
when(serviceOfferingDao.findById(s_serviceOfferingId)).thenReturn(serviceOffering);

// The hypervisor list is shuffled before iteration, so instead of pinning which hypervisor is
// tried first, the first lookup (whichever hypervisor that is) returns no template and the
// second lookup returns a valid one, exercising the "skip and try the next hypervisor" fix.
VMTemplateVO template = mock(VMTemplateVO.class);
when(template.getId()).thenReturn(s_templateId);
when(templateDao.findSystemVMReadyTemplate(eq(s_zoneId), any(Hypervisor.HypervisorType.class), eq(ResourceManager.SystemVmPreferredArchitecture.defaultValue())))
.thenReturn(null, template);

UserVm vm = mock(UserVm.class);
when(vm.getId()).thenReturn(s_vmId);
when(userVmService.createAdvancedVirtualMachine(
any(DataCenter.class), any(ServiceOffering.class), any(VirtualMachineTemplate.class), anyList(), any(Account.class), anyString(),
anyString(), anyLong(), anyLong(), any(), isNull(), any(Hypervisor.HypervisorType.class), any(BaseCmd.HTTPMethod.class), anyString(),
isNull(), isNull(), anyList(), isNull(), any(Network.IpAddresses.class), isNull(), isNull(), isNull(),
anyMap(), isNull(), isNull(), isNull(), isNull(),
anyBoolean(), anyString(), isNull(), isNull(), isNull())).thenReturn(vm);

VolumeVO dataVol = mock(VolumeVO.class);
when(dataVol.getId()).thenReturn(s_volumeId);
when(dataVol.getName()).thenReturn("DATA-1");
when(dataVol.getVolumeType()).thenReturn(Volume.Type.DATADISK);
when(volumeDao.findByInstance(s_vmId)).thenReturn(List.of(dataVol));

Pair<Long, Long> result = lifeCycle.deploySharedFS(sharedFS, s_networkId, s_diskOfferingId, s_size, s_minIops, s_maxIops);
Assert.assertEquals(Optional.of(s_volumeId), Optional.ofNullable(result.first()));
Assert.assertEquals(Optional.of(s_vmId), Optional.ofNullable(result.second()));

verify(templateDao, times(2)).findSystemVMReadyTemplate(eq(s_zoneId), any(Hypervisor.HypervisorType.class), eq(ResourceManager.SystemVmPreferredArchitecture.defaultValue()));
}

@Test
public void testDeleteSharedFS() throws ResourceUnavailableException {
SharedFS sharedFS = mock(SharedFS.class);
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2958,7 +2958,7 @@ public boolean configure(final String name, final Map<String, Object> params) th
}

@Override
public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId, final boolean forVirtualRouter, final Long podId) {
public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId, final boolean forSystemVm, final Long podId) {
final List<HypervisorType> hypervisorTypes = new ArrayList<>();

List<ClusterVO> clustersForZone;
Expand All @@ -2970,7 +2970,7 @@ public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId, final

for (final ClusterVO cluster : clustersForZone) {
final HypervisorType hType = cluster.getHypervisorType();
if (!forVirtualRouter || (hType != HypervisorType.BareMetal && hType != HypervisorType.External && hType != HypervisorType.Ovm)) {
if (!forSystemVm || (hType != HypervisorType.BareMetal && hType != HypervisorType.External && hType != HypervisorType.Ovm)) {
hypervisorTypes.add(hType);
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/com/cloud/resource/MockResourceManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public DataCenter getZone(Long zoneId) {
* @see com.cloud.resource.ResourceService#getSupportedHypervisorTypes(long, boolean, java.lang.Long)
*/
@Override
public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId, final boolean forVirtualRouter, final Long podId) {
public List<HypervisorType> getSupportedHypervisorTypes(final long zoneId, final boolean forSystemVm, final Long podId) {
// TODO Auto-generated method stub
return null;
}
Expand Down
Loading