This is an automated email from the ASF dual-hosted git repository.
nvazquez pushed a commit to branch 4.22
in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/4.22 by this push:
new 7ea1dca5167 Add NULL check during restoreVM operation when host is
removed (#13779)
7ea1dca5167 is described below
commit 7ea1dca5167ba46c3ef812826b7bfb4083bf408f
Author: Nicolas Vazquez <[email protected]>
AuthorDate: Mon Aug 24 15:26:29 2026 -0300
Add NULL check during restoreVM operation when host is removed (#13779)
* Add NULL check during restoreVM operation when host is removed (#571)
* Add NULL check during restore VM operation when host is not
available/removed
* fix line ending pre commit failure
* update logging with details of removed host and vm
---------
Co-authored-by: Sachin R Doddaguni <[email protected]>
(cherry picked from commit 1dbda12ccdca1aaf86ef8f9b185986c9d22b567e)
* Handle null host in VM restore to prevent NPE on deleted host records
* Fix build
* Fix unit test
---------
Co-authored-by: Sachin R <[email protected]>
Co-authored-by: Sachin R Doddaguni <[email protected]>
Co-authored-by: mprokopchuk <[email protected]>
---
.../main/java/com/cloud/vm/UserVmManagerImpl.java | 17 ++--
.../java/com/cloud/vm/UserVmManagerImplTest.java | 97 ++++++++++++++++++++++
2 files changed, 109 insertions(+), 5 deletions(-)
diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
index 857c2b05af4..b5843ad042f 100644
--- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
@@ -9275,17 +9275,24 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
Long hostId = vm.getHostId() != null ? vm.getHostId() :
vm.getLastHostId();
if (hostId != null) {
- // default findById() won't search entries with removed field
not null
- Host host = _hostDao.findById(hostId);
+ Host host = _hostDao.findByIdIncludingRemoved(hostId);
+
+ // host row may have been hard-deleted from DB, treat like
removed
if (host == null) {
- logger.warn("Host {} not found", hostId);
+ logger.warn(String.format("Host with id {} not found in DB
for VM %s ({})",
+ hostId, vm.getUuid(), vm.getName()));
+ return;
+ }
+ // host could be in removed state, in which case no operation
is performed.
+ if (host.getStatus() == Status.Removed) {
+ logger.warn("Host {} ({}) for VM {} ({}) removed on {}",
+ host.getUuid(), host.getName(), vm.getUuid(),
vm.getName(), host.getRemoved());
return;
}
-
- VolumeInfo volumeInfo = volFactory.getVolume(root.getId());
final Command cmd;
+ VolumeInfo volumeInfo = volFactory.getVolume(root.getId());
if (host.getHypervisorType() == HypervisorType.XenServer) {
DiskTO disk = new DiskTO(volumeInfo.getTO(),
root.getDeviceId(), root.getPath(), root.getVolumeType());
diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
index cbedd10825e..8e5f97ee347 100644
--- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
+++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
@@ -59,6 +59,8 @@ import java.util.Map;
import java.util.TimeZone;
import java.util.UUID;
+import com.cloud.event.dao.UsageEventDao;
+import com.cloud.host.Status;
import org.apache.cloudstack.acl.ControlledEntity;
import org.apache.cloudstack.acl.SecurityChecker;
import org.apache.cloudstack.api.ApiCommandResourceType;
@@ -82,6 +84,7 @@ import org.apache.cloudstack.backup.dao.BackupDao;
import org.apache.cloudstack.backup.dao.BackupScheduleDao;
import org.apache.cloudstack.context.CallContext;
import
org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService;
+import
org.apache.cloudstack.engine.orchestration.service.VolumeOrchestrationService;
import org.apache.cloudstack.resourcelimit.Reserver;
import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.Scope;
@@ -89,6 +92,8 @@ import
org.apache.cloudstack.engine.subsystem.api.storage.VolumeDataFactory;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.StoragePoolVO;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao;
+import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreVO;
import org.apache.cloudstack.storage.template.VnfTemplateManager;
import org.apache.cloudstack.userdata.UserDataManager;
import org.apache.cloudstack.vm.UnmanagedVMsManager;
@@ -426,6 +431,15 @@ public class UserVmManagerImplTest {
@Mock
private VolumeDataFactory volumeDataFactory;
+ @Mock
+ private VolumeOrchestrationService volumeMgr;
+
+ @Mock
+ private TemplateDataStoreDao templateDataStoreDao;
+
+ @Mock
+ private UsageEventDao usageEventDao;
+
@Mock
private VolumeInfo volumeInfo;
@@ -1664,6 +1678,89 @@ public class UserVmManagerImplTest {
Mockito.verify(userVmManagerImpl).addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(Mockito.any(),
Mockito.any(), Mockito.eq(VmDetailConstants.CPU_NUMBER), Mockito.any());
}
+ @Test
+ public void testRestoreVirtualMachineWhenHostRemoved() throws
ResourceUnavailableException, InsufficientCapacityException,
ResourceAllocationException {
+ long vmId = 1L;
+ Long lastHostId = 42L;
+ Long newTemplateId = 2L;
+ boolean expunge = false;
+ Map<String, String> details = new HashMap<>();
+
+ try (MockedStatic<CallContext> ignored =
Mockito.mockStatic(CallContext.class);
+ MockedStatic<UsageEventUtils> ignoredEventUtils =
mockStatic(UsageEventUtils.class)
+ ) {
+ UserVmVO vm = mock(UserVmVO.class);
+ when(vm.getId()).thenReturn(vmId);
+ when(vm.getAccountId()).thenReturn(accountId);
+ when(vm.getHostId()).thenReturn(null);
+ when(vm.getLastHostId()).thenReturn(lastHostId);
+ when(vm.getUuid()).thenReturn("test-uuid");
+ when(vm.getState()).thenReturn(VirtualMachine.State.Stopped);
+ when(vm.getTemplateId()).thenReturn(1L);
+ when(vm.getDataCenterId()).thenReturn(1L);
+
+ CallContext mockCallContext = mock(CallContext.class);
+ when(mockCallContext.getCallingAccount()).thenReturn(accountMock);
+
+ CallContext mockVolumeContext = mock(CallContext.class);
+ when(CallContext.register(any(CallContext.class),
any(ApiCommandResourceType.class))).thenReturn(mockVolumeContext);
+
+ when(CallContext.current()).thenReturn(mockCallContext);
+ when(accountDao.findById(accountId)).thenReturn(callerAccount);
+
when(accountDao.findByIdIncludingRemoved(accountId)).thenReturn(callerAccount);
+ when(callerAccount.getState()).thenReturn(Account.State.ENABLED);
+ VMTemplateVO template = mock(VMTemplateVO.class);
+ when(templateDao.findById(anyLong())).thenReturn(template);
+ when(template.getFormat()).thenReturn(Storage.ImageFormat.QCOW2);
+ when(template.getId()).thenReturn(1L);
+ when(template.isDirectDownload()).thenReturn(false);
+ when(template.getSize()).thenReturn(10L * 1024 * 1024 * 1024L); //
10GB
+
+ TemplateDataStoreVO templateStore =
mock(TemplateDataStoreVO.class);
+ when(templateDataStoreDao.findByTemplateZoneReady(1L,
1L)).thenReturn(templateStore);
+
+ ServiceOfferingVO serviceOffering = mock(ServiceOfferingVO.class);
+ when(vm.getServiceOfferingId()).thenReturn(serviceOfferingId);
+
+ List<VolumeVO> rootVols = new ArrayList<>();
+ VolumeVO rootVol = mock(VolumeVO.class);
+ when(rootVol.getId()).thenReturn(10L);
+ when(rootVol.getState()).thenReturn(Volume.State.Ready);
+ when(rootVol.getPoolId()).thenReturn(5L);
+ when(rootVol.getTemplateId()).thenReturn(1L);
+ when(rootVol.getSize()).thenReturn(20L * 1024 * 1024 * 1024L); //
20GB
+ when(rootVol.getDiskOfferingId()).thenReturn(100L);
+ when(rootVol.isDisplay()).thenReturn(true);
+ rootVols.add(rootVol);
+ DiskOfferingVO diskOffering = mock(DiskOfferingVO.class);
+ when(diskOfferingDao.findById(100L)).thenReturn(diskOffering);
+
+ StoragePoolVO storagePool = mock(StoragePoolVO.class);
+ when(storagePool.isManaged()).thenReturn(true);
+ when(primaryDataStoreDao.findById(5L)).thenReturn(storagePool);
+ when(vmSnapshotDaoMock.findByVm(vmId)).thenReturn(new
ArrayList<>());
+ when(volumeDaoMock.findByInstanceAndType(vmId,
Volume.Type.ROOT)).thenReturn(rootVols);
+ when(volumeDaoMock.findById(anyLong())).thenReturn(rootVol);
+ when(userVmDao.findById(vmId)).thenReturn(vm);
+
+ HostVO host = mock(HostVO.class);
+ when(host.getStatus()).thenReturn(Status.Removed);
+
when(hostDao.findByIdIncludingRemoved(lastHostId)).thenReturn(host);
+ VolumeInfo volumeInfo = mock(VolumeInfo.class);
+
+ VolumeVO newVolume = mock(VolumeVO.class);
+ when(volumeMgr.allocateDuplicateVolume(any(VolumeVO.class), any(),
anyLong()))
+ .thenReturn(newVolume);
+ when(newVolume.getId()).thenReturn(11L);
+
+ UserVm result =
userVmManagerImpl.restoreVirtualMachine(accountMock, vmId, newTemplateId, null,
expunge, details);
+ assertNotNull(result);
+
+ Mockito.verify(userVmDao).findById(vmId);
+ Mockito.verify(hostDao).findByIdIncludingRemoved(lastHostId);
+ }
+ }
+
@Test
public void testCheckVolumesLimits() {
long diskOffId1 = 1L;
