From 72cf486b2c0c4eca72fbecea60bf030cccb6a5fb Mon Sep 17 00:00:00 2001 From: Laszlo Szomor Date: Fri, 19 May 2023 10:52:03 +0200 Subject: [PATCH] Avoid NPE on refreshlun call without plugged VMs --- .../storage/pool/SyncDirectLunsCommand.java | 8 ++++++- .../pool/SyncDirectLunsCommandTest.java | 22 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommand.java index 2e67ff7b62c..b473104a8fc 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommand.java @@ -1,5 +1,6 @@ package org.ovirt.engine.core.bll.storage.pool; +import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -218,7 +219,12 @@ private boolean updateLunDisksOnGuests(Set lunsToUpdateInDb) { } private List getPluggedVms(Guid diskId) { - return vmDao.getForDisk(diskId, false).get(Boolean.TRUE); + List result = vmDao.getForDisk(diskId, false).get(Boolean.TRUE); + // Return an empty list if no plugged VMs found + if (result == null) { + result = new ArrayList<>(); + } + return result; } private Stream getIdsOfDirectLunsToSync() { diff --git a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommandTest.java b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommandTest.java index 2fcb72a5601..60906334d42 100644 --- a/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommandTest.java +++ b/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/storage/pool/SyncDirectLunsCommandTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @@ -27,12 +28,15 @@ import org.mockito.junit.jupiter.MockitoSettings; import org.mockito.quality.Strictness; import org.ovirt.engine.core.bll.ValidateTestUtils; +import org.ovirt.engine.core.common.AuditLogType; import org.ovirt.engine.core.common.action.SyncDirectLunsParameters; import org.ovirt.engine.core.common.businessentities.VM; import org.ovirt.engine.core.common.businessentities.storage.DiskLunMap; import org.ovirt.engine.core.common.businessentities.storage.LUNs; import org.ovirt.engine.core.common.errors.EngineMessage; import org.ovirt.engine.core.compat.Guid; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector; +import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogable; import org.ovirt.engine.core.dao.DiskLunMapDao; import org.ovirt.engine.core.dao.LunDao; import org.ovirt.engine.core.dao.StoragePoolDao; @@ -62,11 +66,15 @@ public class SyncDirectLunsCommandTest { @Mock private StorageServerConnectionDao serverConnectionDao; + @Mock + private AuditLogDirector auditLogDirector; + private LUNs lun1; private LUNs lun2; private LUNs lun3; private DiskLunMap disk1Lun1Map; private Map> lunsPerHost; + private Map> emptyLunsPerHost; @Spy @InjectMocks @@ -119,6 +127,19 @@ public void testGetLunsToUpdateInDbLunIsPartOfStorageDomain() { assertEquals(Collections.singleton(lun1), command.getLunsToUpdateInDb()); } + @Test + public void testSyncDirectLunsWithoutPluggedVms() { + command.getParameters().setDeviceList(Arrays.asList(lun1)); + mockLunToDiskIdsOfDirectLunsAttachedToVmsInPool(lun1); + command.getParameters().setDirectLunIds(Collections.singleton(lun1.getDiskId())); + when(diskLunMapDao.getDiskLunMapByDiskId(lun1.getDiskId())).thenReturn(disk1Lun1Map); + when(vmDao.getForDisk(lun1.getDiskId(), false)).thenReturn(emptyLunsPerHost); + doReturn(true).when(command).validateVds(); + doNothing().when(auditLogDirector).log(any(AuditLogable.class), any(AuditLogType.class)); + command.executeCommand(); + assertTrue(command.getReturnValue().getSucceeded(), "SyncDirectLuns without plugged VMs failed"); + } + @Test public void validateWithDirectLunIdAndInvalidVds() { command.getParameters().setDirectLunIds(Collections.singleton(lun1.getDiskId())); @@ -155,6 +176,7 @@ private void mockLunDao() { private void mockVmDao() { lunsPerHost = new HashMap<>(); + emptyLunsPerHost = new HashMap<>(); List vms = new ArrayList<>(); VM vm = new VM(); vm.setRunOnVds(Guid.newGuid());