From c9207ac3834cc51fe59e06775b62d9f09c5efc48 Mon Sep 17 00:00:00 2001 From: Laszlo Szomor Date: Fri, 19 May 2023 10:52:03 +0200 Subject: [PATCH 1/2] Avoid NPE on refreshlun call without plugged VMs Signed-off-by: Laszlo Szomor --- .../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()); From 863eda241f3969a6c9b1634bee88092b2e99532f Mon Sep 17 00:00:00 2001 From: Arik Hadas Date: Sun, 21 May 2023 11:11:30 +0300 Subject: [PATCH 2/2] core: simplify SyncDirectLunsCommand#getPluggedVms Signed-off-by: Arik Hadas --- .../core/bll/storage/pool/SyncDirectLunsCommand.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) 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 b473104a8fc..c9bce2e3dfb 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,6 +1,5 @@ package org.ovirt.engine.core.bll.storage.pool; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; @@ -219,12 +218,7 @@ private boolean updateLunDisksOnGuests(Set lunsToUpdateInDb) { } private List getPluggedVms(Guid diskId) { - 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; + return vmDao.getForDisk(diskId, false).computeIfAbsent(Boolean.TRUE, b -> Collections.emptyList()); } private Stream getIdsOfDirectLunsToSync() {