diff --git a/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotAnswer.java b/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotAnswer.java new file mode 100644 index 000000000000..f79846f44fc9 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/storage/MergeDiskOnlyVmSnapshotAnswer.java @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.cloud.agent.api.storage; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.Command; + +import java.util.ArrayList; +import java.util.List; + +/** + * Answer to {@link MergeDiskOnlyVmSnapshotCommand}. Besides the overall result, it reports which volumes had their merge applied on the host, so that the + * management server can update its metadata for exactly those volumes, even when the merge of other volumes failed or was aborted. The volumes not listed + * were left with their backing chain unchanged. + */ +public class MergeDiskOnlyVmSnapshotAnswer extends Answer { + + private List mergedVolumeUuids; + + public MergeDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, List mergedVolumeUuids) { + super(command, success, details); + this.mergedVolumeUuids = mergedVolumeUuids == null ? new ArrayList<>() : mergedVolumeUuids; + } + + public List getMergedVolumeUuids() { + return mergedVolumeUuids; + } +} diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java index 79e2a9722039..571e87a8f88b 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java @@ -18,11 +18,13 @@ */ package org.apache.cloudstack.storage.vmsnapshot; +import com.cloud.agent.AgentManager; import com.cloud.agent.api.Answer; import com.cloud.agent.api.VMSnapshotTO; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotCommand; import com.cloud.agent.api.storage.DeleteDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotCommand; import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand; @@ -76,10 +78,12 @@ import javax.inject.Inject; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Objects; +import java.util.Set; import java.util.stream.Collectors; public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStrategy { @@ -137,11 +141,22 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { VMSnapshotVO vmSnapshotBeingDeleted = (VMSnapshotVO) vmSnapshot; Long hostId = pickHostForNvramSidecarCleanup(vmSnapshotBeingDeleted, userVm, "delete"); validateHostSupportsNvramSidecarCleanup(vmSnapshotBeingDeleted, hostId, "delete"); - long virtualSize = 0; boolean isCurrent = vmSnapshotBeingDeleted.getCurrent(); transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.ExpungeRequested); + try { + return deleteVMSnapshotAfterExpungeRequested(vmSnapshotBeingDeleted, userVm, hostId, isCurrent); + } catch (RuntimeException ex) { + logger.error("Failed to delete VM snapshot [{}]; it will be marked as [{}]. The volumes that were merged before the failure had their metadata updated;" + + " the others were left unchanged on storage, and deleting the VM snapshot again will retry their merge.", vmSnapshot.getUuid(), VMSnapshot.State.Error, ex); + transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.OperationFailed); + throw ex; + } + } + + private boolean deleteVMSnapshotAfterExpungeRequested(VMSnapshotVO vmSnapshotBeingDeleted, UserVmVO userVm, Long hostId, boolean isCurrent) { + long virtualSize = 0; List volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshotBeingDeleted.getVmId()); List snapshotChildren = vmSnapshotDao.listByParentAndStateIn(vmSnapshotBeingDeleted.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden); PrimaryDataStoreTO nvramPrimaryDataStore = getPrimaryDataStoreForNvramCleanup(vmSnapshotBeingDeleted, volumeTOs); @@ -153,18 +168,21 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { if (isCurrent && numberOfChildren == 0) { volumeSnapshotVos = mergeCurrentDeltaOnSnapshot(vmSnapshotBeingDeleted, userVm, hostId, volumeTOs); } else if (numberOfChildren == 0) { - logger.debug("Deleting VM snapshot [{}] as no snapshots/volumes depend on it.", vmSnapshot.getUuid()); + logger.debug("Deleting VM snapshot [{}] as no snapshots/volumes depend on it.", vmSnapshotBeingDeleted.getUuid()); volumeSnapshotVos = deleteSnapshot(vmSnapshotBeingDeleted, hostId); - mergeOldSiblingWithOldParentIfOldParentIsDead(vmSnapshotDao.findByIdIncludingRemoved(vmSnapshotBeingDeleted.getParent()), userVm, hostId, volumeTOs); + VMSnapshotVO oldParent = vmSnapshotDao.findByIdIncludingRemoved(vmSnapshotBeingDeleted.getParent()); + try { + mergeOldSiblingWithOldParentIfOldParentIsDead(oldParent, userVm, hostId, volumeTOs); + } catch (CloudRuntimeException ex) { + logger.warn("Deleted VM snapshot [{}], but failed to merge its hidden parent [{}] with its remaining child; the parent will be kept hidden on storage.", + vmSnapshotBeingDeleted.getUuid(), oldParent.getUuid(), ex); + } } else if (!isCurrent && numberOfChildren == 1) { VMSnapshotVO childSnapshot = snapshotChildren.get(0); volumeSnapshotVos = mergeSnapshots(vmSnapshotBeingDeleted, childSnapshot, userVm, volumeTOs, hostId); } - for (SnapshotVO snapshotVO : volumeSnapshotVos) { - snapshotVO.setState(Snapshot.State.Destroyed); - snapshotDao.update(snapshotVO.getId(), snapshotVO); - } + markVolumeSnapshotsAsDestroyed(volumeSnapshotVos); for (VolumeObjectTO volumeTo : volumeTOs) { publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_DELETE, vmSnapshotBeingDeleted, userVm, volumeTo); @@ -306,10 +324,7 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen snapshotVos = mergeSnapshots(oldParent, oldSibling, userVm, volumeTOs, hostId); } - for (SnapshotVO snapshotVO : snapshotVos) { - snapshotVO.setState(Snapshot.State.Destroyed); - snapshotDao.update(snapshotVO.getId(), snapshotVO); - } + markVolumeSnapshotsAsDestroyed(snapshotVos); deleteNvramSnapshotIfNeeded(oldParent, hostId, nvramPrimaryDataStore); @@ -436,29 +451,39 @@ private List mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO for (VolumeObjectTO volumeObjectTO : volumeObjectTOS) { snapshotMergeTreeToList.stream().filter(snapshotTree -> Objects.equals(((SnapshotObjectTO) snapshotTree.getParent()).getVolume().getId(), volumeObjectTO.getId())) .findFirst() - .orElseThrow(() -> new CloudRuntimeException(String.format("Failed to find volume snapshot for volume [%s].", volumeObjectTO.getUuid()))) - .addGrandChild(volumeObjectTO); + .ifPresentOrElse(snapshotTree -> snapshotTree.addGrandChild(volumeObjectTO), + () -> logger.debug("Volume [{}] has no snapshot left to merge in VM snapshot [{}]; it was merged by a previous attempt.", + volumeObjectTO.getUuid(), vmSnapshotVO.getUuid())); } } - MergeDiskOnlyVmSnapshotCommand mergeDiskOnlyVMSnapshotCommand = new MergeDiskOnlyVmSnapshotCommand(snapshotMergeTreeToList, userVm.getState(), userVm.getName()); - Answer answer = agentMgr.easySend(hostId, mergeDiskOnlyVMSnapshotCommand); - if (answer == null || !answer.getResult()) { - throw new CloudRuntimeException(String.format("Failed to merge VM snapshot [%s] due to %s.", vmSnapshotVO.getUuid(), answer != null ? answer.getDetails() : "Communication failure")); - } - - logger.debug("Updating metadata of VM snapshot [{}] and its child [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid()); List snapshotVOList = new ArrayList<>(); - for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeToList) { - SnapshotObjectTO childTO = (SnapshotObjectTO) snapshotMergeTreeTO.getChild(); - SnapshotObjectTO parentTO = (SnapshotObjectTO) snapshotMergeTreeTO.getParent(); + if (snapshotMergeTreeToList.isEmpty()) { + logger.debug("Every volume of VM snapshot [{}] was merged with its child [{}] by a previous attempt.", vmSnapshotVO.getUuid(), childSnapshot.getUuid()); + } else { + MergeDiskOnlyVmSnapshotCommand mergeDiskOnlyVMSnapshotCommand = new MergeDiskOnlyVmSnapshotCommand(snapshotMergeTreeToList, userVm.getState(), userVm.getName()); + Answer answer = sendMergeCommand(hostId, mergeDiskOnlyVMSnapshotCommand); + Set mergedVolumeUuids = getMergedVolumeUuids(answer, snapshotMergeTreeToList); + + logger.debug("Updating metadata of VM snapshot [{}] and its child [{}] for the merged volumes [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid(), + mergedVolumeUuids); + for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeToList) { + SnapshotObjectTO childTO = (SnapshotObjectTO) snapshotMergeTreeTO.getChild(); + SnapshotObjectTO parentTO = (SnapshotObjectTO) snapshotMergeTreeTO.getParent(); + if (!mergedVolumeUuids.contains(parentTO.getVolume().getUuid())) { + continue; + } - SnapshotDataStoreVO childSnapshotDataStoreVO = snapshotDataStoreDao.findBySnapshotIdInAnyState(childTO.getId(), DataStoreRole.Primary); - childSnapshotDataStoreVO.setInstallPath(parentTO.getPath()); - snapshotDataStoreDao.update(childSnapshotDataStoreVO.getId(), childSnapshotDataStoreVO); + SnapshotDataStoreVO childSnapshotDataStoreVO = snapshotDataStoreDao.findBySnapshotIdInAnyState(childTO.getId(), DataStoreRole.Primary); + childSnapshotDataStoreVO.setInstallPath(parentTO.getPath()); + snapshotDataStoreDao.update(childSnapshotDataStoreVO.getId(), childSnapshotDataStoreVO); - snapshotDataStoreDao.expungeReferenceBySnapshotIdAndDataStoreRole(parentTO.getId(), childSnapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary); - snapshotVOList.add(snapshotDao.findById(parentTO.getId())); + snapshotDataStoreDao.expungeReferenceBySnapshotIdAndDataStoreRole(parentTO.getId(), childSnapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary); + removeVolumeSnapshotFromVmSnapshot(vmSnapshotVO, parentTO.getId()); + snapshotVOList.add(snapshotDao.findById(parentTO.getId())); + } + + failIfNotEveryVolumeWasMerged(vmSnapshotVO, answer, snapshotMergeTreeToList, mergedVolumeUuids, snapshotVOList); } childSnapshot.setParent(vmSnapshotVO.getParent()); @@ -474,31 +499,42 @@ private List mergeCurrentDeltaOnSnapshot(VMSnapshotVO vmSnapshotVo, for (VolumeObjectTO volumeObjectTO : volumeObjectTOS) { SnapshotDataStoreVO volumeParentSnapshot = volumeSnapshots.stream().filter(snapshot -> Objects.equals(snapshot.getVolumeId(), volumeObjectTO.getId())) - .findFirst() - .orElseThrow(() -> new CloudRuntimeException(String.format("Failed to find volume snapshot for volume [%s].", volumeObjectTO.getUuid()))); + .findFirst().orElse(null); + if (volumeParentSnapshot == null) { + logger.debug("Volume [{}] has no snapshot left to merge in VM snapshot [{}]; it was merged by a previous attempt.", volumeObjectTO.getUuid(), + vmSnapshotVo.getUuid()); + continue; + } DataTO parentSnapshot = snapshotDataFactory.getSnapshot(volumeParentSnapshot.getSnapshotId(), volumeParentSnapshot.getDataStoreId(), DataStoreRole.Primary).getTO(); snapshotMergeTreeTOList.add(new SnapshotMergeTreeTO(parentSnapshot, volumeObjectTO, new ArrayList<>())); } - MergeDiskOnlyVmSnapshotCommand mergeDiskOnlyVMSnapshotCommand = new MergeDiskOnlyVmSnapshotCommand(snapshotMergeTreeTOList, userVmVO.getState(), userVmVO.getName()); - - Answer answer = agentMgr.easySend(hostId, mergeDiskOnlyVMSnapshotCommand); - if (answer == null || !answer.getResult()) { - throw new CloudRuntimeException(String.format("Failed to delete VM snapshot [%s] due to %s.", vmSnapshotVo.getUuid(), answer != null ? answer.getDetails() : "Communication failure")); - } - - logger.debug("Updating metadata of VM snapshot [{}].", vmSnapshotVo.getUuid()); List snapshotVOList = new ArrayList<>(); - for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeTOList) { - VolumeObjectTO volumeObjectTO = (VolumeObjectTO) snapshotMergeTreeTO.getChild(); - SnapshotObjectTO parentTO = (SnapshotObjectTO) snapshotMergeTreeTO.getParent(); + if (snapshotMergeTreeTOList.isEmpty()) { + logger.debug("Every volume of VM snapshot [{}] was merged with the current delta by a previous attempt.", vmSnapshotVo.getUuid()); + } else { + MergeDiskOnlyVmSnapshotCommand mergeDiskOnlyVMSnapshotCommand = new MergeDiskOnlyVmSnapshotCommand(snapshotMergeTreeTOList, userVmVO.getState(), userVmVO.getName()); + Answer answer = sendMergeCommand(hostId, mergeDiskOnlyVMSnapshotCommand); + Set mergedVolumeUuids = getMergedVolumeUuids(answer, snapshotMergeTreeTOList); + + logger.debug("Updating metadata of VM snapshot [{}] for the merged volumes [{}].", vmSnapshotVo.getUuid(), mergedVolumeUuids); + for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeTOList) { + VolumeObjectTO volumeObjectTO = (VolumeObjectTO) snapshotMergeTreeTO.getChild(); + SnapshotObjectTO parentTO = (SnapshotObjectTO) snapshotMergeTreeTO.getParent(); + if (!mergedVolumeUuids.contains(parentTO.getVolume().getUuid())) { + continue; + } - VolumeVO volumeVO = volumeDao.findById(volumeObjectTO.getId()); - volumeVO.setPath(parentTO.getPath()); - volumeDao.update(volumeVO.getId(), volumeVO); + VolumeVO volumeVO = volumeDao.findById(volumeObjectTO.getId()); + volumeVO.setPath(parentTO.getPath()); + volumeDao.update(volumeVO.getId(), volumeVO); - snapshotDataStoreDao.expungeReferenceBySnapshotIdAndDataStoreRole(parentTO.getId(), volumeVO.getPoolId(), DataStoreRole.Primary); - snapshotVOList.add(snapshotDao.findById(parentTO.getId())); + snapshotDataStoreDao.expungeReferenceBySnapshotIdAndDataStoreRole(parentTO.getId(), volumeVO.getPoolId(), DataStoreRole.Primary); + removeVolumeSnapshotFromVmSnapshot(vmSnapshotVo, parentTO.getId()); + snapshotVOList.add(snapshotDao.findById(parentTO.getId())); + } + + failIfNotEveryVolumeWasMerged(vmSnapshotVo, answer, snapshotMergeTreeTOList, mergedVolumeUuids, snapshotVOList); } vmSnapshotVo.setCurrent(false); @@ -511,6 +547,66 @@ private List mergeCurrentDeltaOnSnapshot(VMSnapshotVO vmSnapshotVo, return snapshotVOList; } + /** + * Sends the merge command with the wait that the management server will use for it, so that the host aborts the merge before the management server + * stops waiting for the answer, instead of finishing it after the management server has already given up. + */ + protected Answer sendMergeCommand(Long hostId, MergeDiskOnlyVmSnapshotCommand mergeDiskOnlyVmSnapshotCommand) { + if (mergeDiskOnlyVmSnapshotCommand.getWait() <= 0) { + mergeDiskOnlyVmSnapshotCommand.setWait(AgentManager.Wait.value()); + } + return agentMgr.easySend(hostId, mergeDiskOnlyVmSnapshotCommand); + } + + /** + * Returns the UUIDs of the volumes that the host reports as merged. Hosts that do not report the volumes individually either merged all of them or none. + */ + protected Set getMergedVolumeUuids(Answer answer, List snapshotMergeTreeTOList) { + if (answer instanceof MergeDiskOnlyVmSnapshotAnswer) { + return new HashSet<>(((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids()); + } + if (answer != null && answer.getResult()) { + return snapshotMergeTreeTOList.stream().map(this::getVolumeUuid).collect(Collectors.toSet()); + } + return new HashSet<>(); + } + + private void failIfNotEveryVolumeWasMerged(VMSnapshotVO vmSnapshotVO, Answer answer, List snapshotMergeTreeTOList, Set mergedVolumeUuids, + List mergedVolumeSnapshots) { + List notMergedVolumeUuids = snapshotMergeTreeTOList.stream().map(this::getVolumeUuid) + .filter(volumeUuid -> !mergedVolumeUuids.contains(volumeUuid)) + .collect(Collectors.toList()); + if (notMergedVolumeUuids.isEmpty()) { + return; + } + + markVolumeSnapshotsAsDestroyed(mergedVolumeSnapshots); + throw new CloudRuntimeException(String.format("Failed to merge volumes [%s] of VM snapshot [%s] due to %s.", StringUtils.join(notMergedVolumeUuids, ", "), + vmSnapshotVO.getUuid(), answer != null ? answer.getDetails() : "Communication failure")); + } + + private String getVolumeUuid(SnapshotMergeTreeTO snapshotMergeTreeTO) { + return ((SnapshotObjectTO) snapshotMergeTreeTO.getParent()).getVolume().getUuid(); + } + + /** + * Removes a merged volume snapshot from the VM snapshot, so that a later attempt to delete the VM snapshot only merges the volumes that are left. + */ + private void removeVolumeSnapshotFromVmSnapshot(VMSnapshotVO vmSnapshotVO, long volumeSnapshotId) { + for (VMSnapshotDetailsVO vmSnapshotDetailsVO : vmSnapshotDetailsDao.findDetails(vmSnapshotVO.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT)) { + if (String.valueOf(volumeSnapshotId).equals(vmSnapshotDetailsVO.getValue())) { + vmSnapshotDetailsDao.remove(vmSnapshotDetailsVO.getId()); + } + } + } + + private void markVolumeSnapshotsAsDestroyed(List snapshotVOs) { + for (SnapshotVO snapshotVO : snapshotVOs) { + snapshotVO.setState(Snapshot.State.Destroyed); + snapshotDao.update(snapshotVO.getId(), snapshotVO); + } + } + /** * Takes a disk-only VM snapshot, exceptions thrown will be caught deeper in the stack and treated there. * @param vmSnapshot the definition of the VM Snapshot that will be created. diff --git a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java index 35b1f9ff65bf..dbf3c9e4a5c6 100644 --- a/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java @@ -21,6 +21,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.anyString; @@ -30,12 +32,16 @@ import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; @@ -57,8 +63,12 @@ import com.cloud.agent.api.Answer; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.DeleteDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotAnswer; +import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotCommand; import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.SnapshotMergeTreeTO; +import com.cloud.agent.api.to.DataTO; import com.cloud.alert.AlertManager; import com.cloud.host.DetailVO; import com.cloud.host.Host; @@ -66,8 +76,11 @@ import com.cloud.host.dao.HostDao; import com.cloud.host.dao.HostDetailsDao; import com.cloud.storage.DataStoreRole; +import com.cloud.storage.Snapshot; +import com.cloud.storage.SnapshotVO; import com.cloud.storage.Volume; import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.SnapshotDao; import com.cloud.storage.dao.VolumeDao; import com.cloud.user.ResourceLimitService; import com.cloud.uservm.UserVm; @@ -547,4 +560,173 @@ private void mockCandidateHostScope(long vmId, long hostId) { when(host.getId()).thenReturn(hostId); when(hostDao.listAllUpAndEnabledNonHAHosts(Host.Type.Routing, clusterId, podId, dataCenterId, null)).thenReturn(List.of(host)); } + + @Test + public void testGetMergedVolumeUuidsUsesPerVolumeResultsAndFallsBackForLegacyAnswers() { + List trees = List.of(mergeTree(volumeTo(1L, "vol-a"), "snap-a", 101L), mergeTree(volumeTo(2L, "vol-b"), "snap-b", 102L)); + + assertEquals(Set.of("vol-a"), strategy.getMergedVolumeUuids(new MergeDiskOnlyVmSnapshotAnswer(null, false, "aborted", List.of("vol-a")), trees)); + assertEquals(Set.of("vol-a", "vol-b"), strategy.getMergedVolumeUuids(new Answer(null, true, null), trees)); + assertTrue(strategy.getMergedVolumeUuids(new Answer(null, false, "failed"), trees).isEmpty()); + assertTrue(strategy.getMergedVolumeUuids(null, trees).isEmpty()); + } + + @Test + public void testSendMergeCommandExposesTheManagementServerWaitToTheHost() { + MergeDiskOnlyVmSnapshotCommand command = new MergeDiskOnlyVmSnapshotCommand(new ArrayList<>(), VirtualMachine.State.Running, "vm"); + + strategy.sendMergeCommand(40L, command); + assertEquals(AgentManager.Wait.value().intValue(), command.getWait()); + + command.setWait(20); + strategy.sendMergeCommand(40L, command); + assertEquals(20, command.getWait()); + verify(agentMgr, times(2)).easySend(40L, command); + } + + @Test + public void testMergeCurrentDeltaOnSnapshotOnlyUpdatesTheVolumesThatWereMerged() throws Exception { + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VolumeObjectTO volumeA = volumeTo(1L, "vol-a"); + VolumeObjectTO volumeB = volumeTo(2L, "vol-b"); + VMSnapshotDetailsVO detailA = volumeSnapshotDetail(11L, 101L); + VMSnapshotDetailsVO detailB = volumeSnapshotDetail(12L, 102L); + VolumeVO volumeVoA = mock(VolumeVO.class); + SnapshotVO snapshotVoA = mock(SnapshotVO.class); + strategy.snapshotDao = mock(SnapshotDao.class); + + when(vmSnapshot.getId()).thenReturn(20L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshotDetailsDao.findDetails(eq(20L), anyString())).thenReturn(List.of(detailA, detailB)); + mockVolumeSnapshot(volumeA, 101L, "snap-a"); + mockVolumeSnapshot(volumeB, 102L, "snap-b"); + when(strategy.volumeDao.findById(1L)).thenReturn(volumeVoA); + when(volumeVoA.getId()).thenReturn(1L); + when(volumeVoA.getPoolId()).thenReturn(5L); + when(snapshotVoA.getId()).thenReturn(101L); + when(strategy.snapshotDao.findById(101L)).thenReturn(snapshotVoA); + when(agentMgr.easySend(eq(40L), any(MergeDiskOnlyVmSnapshotCommand.class))) + .thenReturn(new MergeDiskOnlyVmSnapshotAnswer(null, false, "the block commits did not finish in time", List.of("vol-a"))); + + try { + invokeMergeCurrentDeltaOnSnapshot(vmSnapshot, List.of(volumeA, volumeB)); + fail("The merge should fail as volume [vol-b] was not merged."); + } catch (CloudRuntimeException ex) { + assertTrue(ex.getMessage(), ex.getMessage().contains("vol-b")); + } + + verify(volumeVoA).setPath("snap-a"); + verify(strategy.volumeDao).update(1L, volumeVoA); + verify(strategy.volumeDao, never()).findById(2L); + verify(snapshotDataStoreDao).expungeReferenceBySnapshotIdAndDataStoreRole(101L, 5L, DataStoreRole.Primary); + verify(snapshotDataStoreDao, never()).expungeReferenceBySnapshotIdAndDataStoreRole(eq(102L), anyLong(), any()); + verify(vmSnapshotDetailsDao).remove(11L); + verify(vmSnapshotDetailsDao, never()).remove(12L); + verify(snapshotVoA).setState(Snapshot.State.Destroyed); + verify(vmSnapshot, never()).setCurrent(Mockito.anyBoolean()); + } + + @Test + public void testMergeCurrentDeltaOnSnapshotOnlyMergesTheVolumesLeftByAPreviousAttempt() throws Exception { + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VolumeObjectTO volumeA = volumeTo(1L, "vol-a"); + VolumeObjectTO volumeB = volumeTo(2L, "vol-b"); + VolumeVO volumeVoB = mock(VolumeVO.class); + VMSnapshotDetailsVO detailB = volumeSnapshotDetail(12L, 102L); + strategy.snapshotDao = mock(SnapshotDao.class); + + when(vmSnapshot.getId()).thenReturn(20L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshot.getParent()).thenReturn(null); + when(vmSnapshotDetailsDao.findDetails(eq(20L), anyString())).thenReturn(List.of(detailB)); + mockVolumeSnapshot(volumeB, 102L, "snap-b"); + when(strategy.volumeDao.findById(2L)).thenReturn(volumeVoB); + when(volumeVoB.getId()).thenReturn(2L); + when(agentMgr.easySend(eq(40L), any(MergeDiskOnlyVmSnapshotCommand.class))) + .thenReturn(new MergeDiskOnlyVmSnapshotAnswer(null, true, null, List.of("vol-b"))); + + invokeMergeCurrentDeltaOnSnapshot(vmSnapshot, List.of(volumeA, volumeB)); + + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(MergeDiskOnlyVmSnapshotCommand.class); + verify(agentMgr).easySend(eq(40L), commandCaptor.capture()); + assertEquals(1, commandCaptor.getValue().getSnapshotMergeTreeToList().size()); + assertSame(volumeB, commandCaptor.getValue().getSnapshotMergeTreeToList().get(0).getChild()); + assertEquals(AgentManager.Wait.value().intValue(), commandCaptor.getValue().getWait()); + verify(volumeVoB).setPath("snap-b"); + verify(vmSnapshot).setCurrent(false); + } + + @Test(expected = CloudRuntimeException.class) + public void testDeleteVMSnapshotMarksSnapshotAsErrorWhenTheDeletionFails() throws Exception { + long vmId = 10L; + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(vmSnapshot.getId()).thenReturn(20L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Running); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(40L); + when(vmSnapshotHelper.getVolumeTOList(vmId)).thenThrow(new CloudRuntimeException("Failed to merge volumes [vol-b] of VM snapshot [vm-snapshot].")); + + try { + strategy.deleteVMSnapshot(vmSnapshot); + } finally { + InOrder inOrder = inOrder(vmSnapshotHelper); + inOrder.verify(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.ExpungeRequested); + inOrder.verify(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); + } + } + + private void invokeMergeCurrentDeltaOnSnapshot(VMSnapshotVO vmSnapshot, List volumeTOs) throws Exception { + UserVmVO userVm = mock(UserVmVO.class); + when(userVm.getState()).thenReturn(VirtualMachine.State.Running); + when(userVm.getName()).thenReturn("vm"); + + Method method = KvmFileBasedStorageVmSnapshotStrategy.class.getDeclaredMethod("mergeCurrentDeltaOnSnapshot", VMSnapshotVO.class, UserVmVO.class, Long.class, + List.class); + method.setAccessible(true); + try { + method.invoke(strategy, vmSnapshot, userVm, 40L, volumeTOs); + } catch (InvocationTargetException ex) { + throw (Exception) ex.getCause(); + } + } + + private VolumeObjectTO volumeTo(long id, String uuid) { + VolumeObjectTO volumeTO = mock(VolumeObjectTO.class); + when(volumeTO.getId()).thenReturn(id); + when(volumeTO.getUuid()).thenReturn(uuid); + return volumeTO; + } + + private SnapshotMergeTreeTO mergeTree(VolumeObjectTO volumeTO, String parentPath, long parentId) { + SnapshotObjectTO parent = mock(SnapshotObjectTO.class); + when(parent.getVolume()).thenReturn(volumeTO); + when(parent.getPath()).thenReturn(parentPath); + when(parent.getId()).thenReturn(parentId); + return new SnapshotMergeTreeTO(parent, volumeTO, new ArrayList<>()); + } + + private VMSnapshotDetailsVO volumeSnapshotDetail(long detailId, long volumeSnapshotId) { + VMSnapshotDetailsVO detail = mock(VMSnapshotDetailsVO.class); + when(detail.getId()).thenReturn(detailId); + when(detail.getValue()).thenReturn(String.valueOf(volumeSnapshotId)); + return detail; + } + + private void mockVolumeSnapshot(VolumeObjectTO volumeTO, long volumeSnapshotId, String path) { + SnapshotDataStoreVO snapshotDataStoreVO = mock(SnapshotDataStoreVO.class); + SnapshotInfo snapshotInfo = mock(SnapshotInfo.class); + DataTO parent = mergeTree(volumeTO, path, volumeSnapshotId).getParent(); + long volumeId = volumeTO.getId(); + + when(snapshotDataStoreVO.getSnapshotId()).thenReturn(volumeSnapshotId); + when(snapshotDataStoreVO.getDataStoreId()).thenReturn(5L); + when(snapshotDataStoreVO.getVolumeId()).thenReturn(volumeId); + when(snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(volumeSnapshotId, DataStoreRole.Primary)).thenReturn(snapshotDataStoreVO); + when(strategy.snapshotDataFactory.getSnapshot(volumeSnapshotId, 5L, DataStoreRole.Primary)).thenReturn(snapshotInfo); + when(snapshotInfo.getTO()).thenReturn(parent); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index a9ed0c62cab8..623d6e749e9a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -796,6 +796,10 @@ public int getCmdsTimeout() { return cmdsTimeout; } + public int getQcow2DeltaMergeTimeout() { + return qcow2DeltaMergeTimeout; + } + public String manageSnapshotPath() { return manageSnapshotPath; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java index ad0678080ed7..2ac0f63fb094 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java @@ -20,6 +20,7 @@ import com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotCommand; import com.cloud.agent.api.storage.SnapshotMergeTreeTO; import com.cloud.agent.api.to.DataObjectType; @@ -38,19 +39,49 @@ import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.cloudstack.utils.security.ParserUtils; +import org.apache.commons.lang3.StringUtils; import org.libvirt.Connect; import org.libvirt.Domain; +import org.libvirt.DomainBlockJobInfo; import org.libvirt.LibvirtException; +import org.w3c.dom.Document; +import org.w3c.dom.Element; +import org.w3c.dom.Node; +import org.w3c.dom.NodeList; +import org.xml.sax.InputSource; import java.io.IOException; +import java.io.StringReader; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; +/** + * Merges the deltas of disk-only VM snapshots when one of them is deleted.

+ * The merge is done so that every volume either has its merge fully applied, or is left with its backing chain exactly as it was, and the answer reports + * which volumes are in which case. It is also bounded by the command's wait, so that the host never keeps merging after the management server has given + * up on the answer, which would leave the management server's metadata pointing to files that no longer exist.

+ * For a running VM, all block commits are started together. Active commits (the current delta being merged into the snapshot) are held in the ready + * state and are only pivoted once every disk is ready; if any disk fails or the deadline is reached, every job is cancelled, which leaves every disk on + * its original active image. Non-active commits have no ready phase and finish on their own, thus each of them is reported individually.

+ * For a stopped VM, every delta is first committed into its parent without changing any backing chain; the chains are only changed once all the commits + * succeeded.

+ * In every case, once the copy of a delta into its parent has started, the parent no longer holds the point-in-time state of the snapshot being deleted. + * Retrying the merge reproduces the same result. + */ @ResourceWrapper(handles = MergeDiskOnlyVmSnapshotCommand.class) public class LibvirtMergeDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper { + protected static final int MIN_SAFETY_MARGIN_IN_SECONDS = 5; + protected static final int MAX_SAFETY_MARGIN_IN_SECONDS = 60; + protected static final long POLL_INTERVAL_IN_MILLIS = 1000; + @Override public Answer execute(MergeDiskOnlyVmSnapshotCommand command, LibvirtComputingResource serverResource) { VirtualMachine.State vmState = command.getVmState(); @@ -60,51 +91,103 @@ public Answer execute(MergeDiskOnlyVmSnapshotCommand command, LibvirtComputingRe return mergeDiskOnlySnapshotsForRunningVM(command, serverResource); } return mergeDiskOnlySnapshotsForStoppedVM(command, serverResource); - } catch (LibvirtException | QemuImgException | CloudRuntimeException ex) { - return new Answer(command, ex); + } catch (LibvirtException | CloudRuntimeException ex) { + logger.error("Failed to merge disk-only snapshots of VM [{}] before any volume was merged.", command.getVmName(), ex); + return new MergeDiskOnlyVmSnapshotAnswer(command, false, ex.getMessage(), new ArrayList<>()); } } - protected Answer mergeDiskOnlySnapshotsForStoppedVM(MergeDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws QemuImgException, LibvirtException { - QemuImg qemuImg = new QemuImg(resource.getCmdsTimeout()); - KVMStoragePoolManager storageManager = resource.getStoragePoolMgr(); + /** + * Returns how long, in seconds, the host may spend merging. When the management server informs the command's wait, a safety margin is subtracted + * from it so that the jobs can be cancelled and the answer delivered before the management server stops waiting. Otherwise, the given fallback is used. + */ + protected int getMergeTimeoutInSeconds(MergeDiskOnlyVmSnapshotCommand cmd, int fallbackInSeconds) { + int wait = cmd.getWait(); + if (wait <= 0) { + return fallbackInSeconds; + } + + int timeout = wait - getSafetyMarginInSeconds(wait); + if (timeout <= 0) { + timeout = Math.max(1, wait / 2); + } + return fallbackInSeconds > 0 ? Math.min(timeout, fallbackInSeconds) : timeout; + } + + protected int getSafetyMarginInSeconds(int wait) { + return Math.min(MAX_SAFETY_MARGIN_IN_SECONDS, Math.max(MIN_SAFETY_MARGIN_IN_SECONDS, wait / 10)); + } + protected Answer mergeDiskOnlySnapshotsForStoppedVM(MergeDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { + KVMStoragePoolManager storageManager = resource.getStoragePoolMgr(); List snapshotMergeTreeTOList = cmd.getSnapshotMergeTreeToList(); + long deadline = System.currentTimeMillis() + getMergeTimeoutInSeconds(cmd, resource.getCmdsTimeout() / 1000) * 1000L; logger.debug("Merging disk-only snapshots for stopped VM [{}] using the following Snapshot Merge Trees [{}].", cmd.getVmName(), snapshotMergeTreeTOList); + List diskMerges = new ArrayList<>(); for (SnapshotMergeTreeTO snapshotMergeTreeTO : snapshotMergeTreeTOList) { DataTO parentTo = snapshotMergeTreeTO.getParent(); PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) parentTo.getDataStore(); KVMStoragePool storagePool = storageManager.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); - String childLocalPath = storagePool.getLocalPathFor(snapshotMergeTreeTO.getChild().getPath()); - - QemuImgFile parent = new QemuImgFile(storagePool.getLocalPathFor(parentTo.getPath()), QemuImg.PhysicalDiskFormat.QCOW2); - QemuImgFile child = new QemuImgFile(childLocalPath, QemuImg.PhysicalDiskFormat.QCOW2); - - logger.debug("Committing child delta [{}] into parent snapshot [{}].", parentTo, snapshotMergeTreeTO.getChild()); - qemuImg.commit(child, parent, true); + diskMerges.add(new StoppedVmDiskMerge(getVolumeUuid(snapshotMergeTreeTO), storagePool.getLocalPathFor(parentTo.getPath()), + storagePool.getLocalPathFor(snapshotMergeTreeTO.getChild().getPath()), + snapshotMergeTreeTO.getGrandChildren().stream().map(grandChild -> storagePool.getLocalPathFor(grandChild.getPath())).collect(Collectors.toList()))); + } - List grandChildren = snapshotMergeTreeTO.getGrandChildren().stream() - .map(snapshotTo -> new QemuImgFile(storagePool.getLocalPathFor(snapshotTo.getPath()), QemuImg.PhysicalDiskFormat.QCOW2)) - .collect(Collectors.toList()); + for (StoppedVmDiskMerge diskMerge : diskMerges) { + long remaining = deadline - System.currentTimeMillis(); + if (remaining <= 0) { + return abortStoppedVmMerge(cmd, String.format("the merge did not finish within the time allowed by the command's wait of [%s] seconds", cmd.getWait())); + } - logger.debug("Rebasing grandChildren [{}] into parent at [{}].", grandChildren, parent.getFileName()); - for (QemuImgFile grandChild : grandChildren) { - qemuImg.rebase(grandChild, parent, parent.getFormat().toString(), false); + QemuImgFile parent = new QemuImgFile(diskMerge.parentPath, QemuImg.PhysicalDiskFormat.QCOW2); + QemuImgFile child = new QemuImgFile(diskMerge.childPath, QemuImg.PhysicalDiskFormat.QCOW2); + logger.debug("Committing child delta [{}] into parent snapshot [{}] of volume [{}].", diskMerge.childPath, diskMerge.parentPath, diskMerge.volumeUuid); + try { + getQemuImg(remaining).commit(child, parent, true); + } catch (QemuImgException | LibvirtException ex) { + return abortStoppedVmMerge(cmd, String.format("committing [%s] into [%s] failed due to [%s]", diskMerge.childPath, diskMerge.parentPath, ex.getMessage())); } + } - logger.debug("Deleting child at [{}] as it is useless.", childLocalPath); + List mergedVolumeUuids = new ArrayList<>(); + List failures = new ArrayList<>(); + for (StoppedVmDiskMerge diskMerge : diskMerges) { try { - Files.deleteIfExists(Path.of(childLocalPath)); - } catch (IOException e) { - return new Answer(cmd, e); + if (!diskMerge.grandChildPaths.isEmpty()) { + QemuImg qemuImg = getQemuImg(resource.getCmdsTimeout()); + QemuImgFile parent = new QemuImgFile(diskMerge.parentPath, QemuImg.PhysicalDiskFormat.QCOW2); + logger.debug("Rebasing grandChildren [{}] into parent at [{}].", diskMerge.grandChildPaths, diskMerge.parentPath); + for (String grandChildPath : diskMerge.grandChildPaths) { + qemuImg.rebase(new QemuImgFile(grandChildPath, QemuImg.PhysicalDiskFormat.QCOW2), parent, parent.getFormat().toString(), false); + } + } + + logger.debug("Deleting child at [{}] as it is useless.", diskMerge.childPath); + Files.deleteIfExists(Path.of(diskMerge.childPath)); + mergedVolumeUuids.add(diskMerge.volumeUuid); + } catch (QemuImgException | LibvirtException | IOException ex) { + logger.error("Failed to finish the merge of volume [{}]; its child [{}] was kept, thus its chain is still consistent.", diskMerge.volumeUuid, + diskMerge.childPath, ex); + failures.add(String.format("volume [%s]: %s", diskMerge.volumeUuid, ex.getMessage())); } } - return new Answer(cmd, true, null); + + if (failures.isEmpty()) { + return new MergeDiskOnlyVmSnapshotAnswer(cmd, true, null, mergedVolumeUuids); + } + return new MergeDiskOnlyVmSnapshotAnswer(cmd, false, String.format("Failed to finish the merge of some volumes of VM [%s]: %s.", cmd.getVmName(), + StringUtils.join(failures, "; ")), mergedVolumeUuids); + } + + private Answer abortStoppedVmMerge(MergeDiskOnlyVmSnapshotCommand cmd, String reason) { + String details = String.format("Aborted the merge of disk-only snapshots of stopped VM [%s] because %s. No backing chain was changed.", cmd.getVmName(), reason); + logger.error(details); + return new MergeDiskOnlyVmSnapshotAnswer(cmd, false, details, new ArrayList<>()); } - protected Answer mergeDiskOnlySnapshotsForRunningVM(MergeDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws LibvirtException, QemuImgException { + protected Answer mergeDiskOnlySnapshotsForRunningVM(MergeDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws LibvirtException { String vmName = cmd.getVmName(); List snapshotMergeTreeTOList = cmd.getSnapshotMergeTreeToList(); @@ -113,38 +196,400 @@ protected Answer mergeDiskOnlySnapshotsForRunningVM(MergeDiskOnlyVmSnapshotComma Domain domain = resource.getDomain(conn, vmName); List disks = resource.getDisks(conn, vmName); KVMStoragePoolManager storageManager = resource.getStoragePoolMgr(); - QemuImg qemuImg = new QemuImg(resource.getCmdsTimeout()); + int timeout = getMergeTimeoutInSeconds(cmd, resource.getQcow2DeltaMergeTimeout()); + long deadline = System.currentTimeMillis() + timeout * 1000L; - logger.debug("Merging disk-only snapshots for running VM [{}] using the following Snapshot Merge Trees [{}].", vmName, snapshotMergeTreeTOList); + logger.debug("Merging disk-only snapshots for running VM [{}] within [{}] seconds using the following Snapshot Merge Trees [{}].", vmName, timeout, + snapshotMergeTreeTOList); + List diskMerges = new ArrayList<>(); for (SnapshotMergeTreeTO mergeTreeTO : snapshotMergeTreeTOList) { DataTO childTO = mergeTreeTO.getChild(); SnapshotObjectTO parentSnapshotTO = (SnapshotObjectTO) mergeTreeTO.getParent(); VolumeObjectTO volumeObjectTO = parentSnapshotTO.getVolume(); KVMStoragePool storagePool = libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(volumeObjectTO, storageManager); - boolean active = DataObjectType.VOLUME.equals(childTO.getObjectType()); String label = resource.getDiskWithPathOfVolumeObjectTO(disks, volumeObjectTO).getDiskLabel(); - String parentSnapshotLocalPath = storagePool.getLocalPathFor(parentSnapshotTO.getPath()); - String childDeltaPath = storagePool.getLocalPathFor(childTO.getPath()); + diskMerges.add(new RunningVmDiskMerge(volumeObjectTO.getUuid(), label, storagePool.getLocalPathFor(parentSnapshotTO.getPath()), + storagePool.getLocalPathFor(childTO.getPath()), DataObjectType.VOLUME.equals(childTO.getObjectType()), + mergeTreeTO.getGrandChildren().stream().map(grandChild -> storagePool.getLocalPathFor(grandChild.getPath())).collect(Collectors.toList()))); + } + + String failure; + try { + failure = startBlockCommits(domain, vmName, diskMerges); + if (failure == null) { + failure = waitForBlockCommits(domain, vmName, diskMerges, deadline, timeout); + } + if (failure == null) { + failure = pivotActiveBlockCommits(domain, vmName, diskMerges); + } + } catch (RuntimeException ex) { + logger.error("Unexpected error while merging the disk-only snapshots of VM [{}].", vmName, ex); + failure = String.format("an unexpected error occurred [%s]", ex.getMessage()); + } + + cancelUnfinishedBlockCommits(domain, vmName, diskMerges, System.currentTimeMillis() + getSafetyMarginInSeconds(Math.max(cmd.getWait(), 0)) * 1000L); + + return finishRunningVmMerge(cmd, domain, diskMerges, failure, resource.getCmdsTimeout()); + } + + /** + * Starts the block commit of every disk. Active commits are started without pivoting, so that the pivot of every disk can be done only once all of them + * are ready. The merged-away files are not deleted by Libvirt, as they are only deleted once the merge of their disk is known to be applied. + * @return the reason of the failure, or null if every block commit was started. + */ + protected String startBlockCommits(Domain domain, String vmName, List diskMerges) { + for (RunningVmDiskMerge diskMerge : diskMerges) { + int flags = diskMerge.active ? Domain.BlockCommitFlags.ACTIVE : 0; + logger.info("Starting block commit of top file [{}] into base file [{}] on disk [{}] of VM [{}] (active: [{}]).", diskMerge.topPath, diskMerge.basePath, + diskMerge.label, vmName, diskMerge.active); + try { + domain.blockCommit(diskMerge.label, diskMerge.basePath, diskMerge.topPath, 0, flags); + } catch (LibvirtException ex) { + return String.format("starting the block commit of disk [%s] failed due to [%s]", diskMerge.label, ex.getMessage()); + } + } + return null; + } + + /** + * Waits until every active commit is ready to be pivoted and every non-active commit has finished. + * @return the reason of the failure, or null if every disk got there before the deadline. + */ + protected String waitForBlockCommits(Domain domain, String vmName, List diskMerges, long deadline, int timeout) { + while (true) { + Map diskChains; + try { + diskChains = getDiskChains(domain); + } catch (LibvirtException | CloudRuntimeException ex) { + return String.format("reading the disks of the VM failed due to [%s]", ex.getMessage()); + } + + boolean allDone = true; + for (RunningVmDiskMerge diskMerge : diskMerges) { + DiskChain diskChain = diskChains.get(diskMerge.label); + if (diskMerge.active && diskChain != null && diskChain.isActiveCommitReady()) { + continue; + } + + boolean jobRunning; + try { + jobRunning = isBlockJobRunning(domain, diskMerge.label); + } catch (LibvirtException ex) { + return String.format("reading the block job of disk [%s] failed due to [%s]", diskMerge.label, ex.getMessage()); + } + + if (jobRunning) { + allDone = false; + continue; + } + if (diskMerge.active || diskChain == null || diskChain.contains(diskMerge.topPath)) { + return String.format("the block commit of disk [%s] ended before being %s", diskMerge.label, diskMerge.active ? "ready to pivot" : "completed"); + } + diskMerge.observedApplied = true; + } + + if (allDone) { + logger.debug("Every block commit of VM [{}] is ready to be finished.", vmName); + return null; + } + if (System.currentTimeMillis() >= deadline) { + return String.format("the block commits did not finish within [%s] seconds", timeout); + } + + try { + Thread.sleep(getPollIntervalInMillis()); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + return "the thread waiting for the block commits was interrupted"; + } + } + } + + /** + * Pivots every active commit. It is only called once all of them are ready, thus the pivots are quick; it stops at the first failure so that the + * remaining disks keep their original active image. + * @return the reason of the failure, or null if every active commit was pivoted. + */ + protected String pivotActiveBlockCommits(Domain domain, String vmName, List diskMerges) { + for (RunningVmDiskMerge diskMerge : diskMerges) { + if (!diskMerge.active) { + continue; + } + logger.debug("Pivoting disk [{}] of VM [{}] to [{}].", diskMerge.label, vmName, diskMerge.basePath); + try { + domain.blockJobAbort(diskMerge.label, Domain.BlockJobAbortFlags.PIVOT); + diskMerge.observedApplied = true; + } catch (LibvirtException ex) { + return String.format("pivoting disk [%s] failed due to [%s]", diskMerge.label, ex.getMessage()); + } + } + return null; + } + + /** + * Cancels every block job that is still running, without pivoting. Cancelling a block commit leaves the disk's backing chain unchanged. Then, waits + * until the given deadline for the jobs to be gone. + */ + protected void cancelUnfinishedBlockCommits(Domain domain, String vmName, List diskMerges, long deadline) { + List unfinished = new ArrayList<>(diskMerges); + while (!unfinished.isEmpty()) { + for (RunningVmDiskMerge diskMerge : new ArrayList<>(unfinished)) { + try { + if (!isBlockJobRunning(domain, diskMerge.label)) { + unfinished.remove(diskMerge); + continue; + } + logger.warn("Cancelling the block commit of disk [{}] of VM [{}]; its backing chain will be kept as it was.", diskMerge.label, vmName); + domain.blockJobAbort(diskMerge.label, 0); + } catch (LibvirtException ex) { + logger.warn("Failed to cancel the block commit of disk [{}] of VM [{}] due to [{}].", diskMerge.label, vmName, ex.getMessage()); + } + } + + if (unfinished.isEmpty() || System.currentTimeMillis() >= deadline) { + break; + } + try { + Thread.sleep(getPollIntervalInMillis()); + } catch (InterruptedException ex) { + Thread.currentThread().interrupt(); + break; + } + } + + if (!unfinished.isEmpty()) { + logger.error("The block jobs of disks [{}] of VM [{}] could not be cancelled; they will be reported as not merged.", + unfinished.stream().map(diskMerge -> diskMerge.label).collect(Collectors.toList()), vmName); + } + } + + /** + * Reads the backing chain of every disk from Libvirt to know which merges were applied, then finishes those: off-chain grandchildren are rebased into + * the base file and the merged-away file is deleted. + */ + protected Answer finishRunningVmMerge(MergeDiskOnlyVmSnapshotCommand cmd, Domain domain, List diskMerges, String failure, int cmdsTimeout) { + String vmName = cmd.getVmName(); + Map diskChains = null; + try { + diskChains = getDiskChains(domain); + } catch (LibvirtException | CloudRuntimeException ex) { + logger.error("Failed to read the disks of VM [{}] after merging; only the pivots and completions observed while merging will be reported as merged.", + vmName, ex); + failure = StringUtils.defaultIfBlank(failure, String.format("reading the disks of the VM failed due to [%s]", ex.getMessage())); + } + + List mergedVolumeUuids = new ArrayList<>(); + List notMergedVolumeUuids = new ArrayList<>(); + for (RunningVmDiskMerge diskMerge : diskMerges) { + DiskChain diskChain = diskChains == null ? null : diskChains.get(diskMerge.label); + boolean applied = diskChains == null ? diskMerge.observedApplied : isMergeApplied(diskMerge, diskChain); + if (diskChains != null && applied != diskMerge.observedApplied) { + logger.warn("Libvirt reports the merge of volume [{}] of VM [{}] as {}applied, which differs from what was observed while merging; using Libvirt's view.", + diskMerge.volumeUuid, vmName, applied ? "" : "not "); + } + if (!applied) { + logger.info("The merge of volume [{}] of VM [{}] was not applied; its backing chain is [{}].", diskMerge.volumeUuid, vmName, diskChain); + notMergedVolumeUuids.add(diskMerge.volumeUuid); + continue; + } + + mergedVolumeUuids.add(diskMerge.volumeUuid); + finishAppliedMerge(vmName, diskMerge, diskChain, cmdsTimeout); + } + + if (failure == null && notMergedVolumeUuids.isEmpty()) { + logger.info("Merged the disk-only snapshots of every volume of VM [{}].", vmName); + return new MergeDiskOnlyVmSnapshotAnswer(cmd, true, null, mergedVolumeUuids); + } + + String details = String.format("Aborted the merge of disk-only snapshots of VM [%s] because %s. Volumes [%s] were left with their backing chain unchanged;" + + " volumes [%s] were merged.", vmName, StringUtils.defaultIfBlank(failure, "not every merge was applied"), StringUtils.join(notMergedVolumeUuids, ", "), + StringUtils.join(mergedVolumeUuids, ", ")); + logger.error(details); + return new MergeDiskOnlyVmSnapshotAnswer(cmd, false, details, mergedVolumeUuids); + } + + protected boolean isMergeApplied(RunningVmDiskMerge diskMerge, DiskChain diskChain) { + if (diskChain == null || diskChain.mirrorJob != null) { + return false; + } + if (diskMerge.active) { + return samePath(diskChain.activeSource, diskMerge.basePath); + } + return !diskChain.contains(diskMerge.topPath) && diskChain.contains(diskMerge.basePath); + } + + private void finishAppliedMerge(String vmName, RunningVmDiskMerge diskMerge, DiskChain diskChain, int cmdsTimeout) { + QemuImgFile base = new QemuImgFile(diskMerge.basePath, QemuImg.PhysicalDiskFormat.QCOW2); + boolean allGrandChildrenRebased = true; + for (String grandChildPath : diskMerge.grandChildPaths) { + if (diskChain != null && diskChain.contains(grandChildPath)) { + logger.debug("Grandchild [{}] is in the backing chain of VM [{}], thus Libvirt has already rebased it.", grandChildPath, vmName); + continue; + } + try { + getQemuImg(cmdsTimeout).rebase(new QemuImgFile(grandChildPath, QemuImg.PhysicalDiskFormat.QCOW2), base, base.getFormat().toString(), false); + } catch (QemuImgException | LibvirtException ex) { + allGrandChildrenRebased = false; + logger.error("Failed to rebase grandchild [{}] into [{}]; [{}] will be kept as the grandchild still depends on it.", grandChildPath, + diskMerge.basePath, diskMerge.topPath, ex); + } + } - logger.debug("Found label [{}] for [{}]. Will merge delta at [{}] into delta at [{}].", label, volumeObjectTO, parentSnapshotLocalPath, childDeltaPath); + if (!allGrandChildrenRebased) { + return; + } + try { + logger.debug("Deleting [{}] as it was merged into [{}].", diskMerge.topPath, diskMerge.basePath); + Files.deleteIfExists(Path.of(diskMerge.topPath)); + } catch (IOException ex) { + logger.warn("Failed to delete [{}], which is no longer used by volume [{}].", diskMerge.topPath, diskMerge.volumeUuid, ex); + } + } + + protected boolean isBlockJobRunning(Domain domain, String label) throws LibvirtException { + DomainBlockJobInfo blockJobInfo = domain.getBlockJobInfo(label, 0); + return blockJobInfo != null && !(blockJobInfo.type == 0 && blockJobInfo.end == 0 && blockJobInfo.cur == 0); + } + + /** + * Reads, from the domain's live XML, the active image, the backing files and the mirror job (if any) of every disk, indexed by the disk's label. + */ + protected Map getDiskChains(Domain domain) throws LibvirtException { + String xml = domain.getXMLDesc(0); + Map diskChains = new HashMap<>(); + try { + Document document = ParserUtils.getSaferDocumentBuilderFactory().newDocumentBuilder().parse(new InputSource(new StringReader(xml))); + NodeList diskNodes = document.getElementsByTagName("disk"); + for (int i = 0; i < diskNodes.getLength(); i++) { + Element disk = (Element) diskNodes.item(i); + Element target = getChildElement(disk, "target"); + if (target == null) { + continue; + } - resource.mergeSnapshotIntoBaseFile(domain, label, parentSnapshotLocalPath, childDeltaPath, active, childTO.getPath(), - volumeObjectTO, conn); + List backingFiles = new ArrayList<>(); + Element backingStore = getChildElement(disk, "backingStore"); + while (backingStore != null) { + String backingFile = getSourcePath(getChildElement(backingStore, "source")); + if (backingFile == null) { + break; + } + backingFiles.add(backingFile); + backingStore = getChildElement(backingStore, "backingStore"); + } - QemuImgFile parent = new QemuImgFile(parentSnapshotLocalPath, QemuImg.PhysicalDiskFormat.QCOW2); + Element mirror = getChildElement(disk, "mirror"); + diskChains.put(target.getAttribute("dev"), new DiskChain(getSourcePath(getChildElement(disk, "source")), backingFiles, + mirror == null ? null : mirror.getAttribute("job"), mirror != null && "yes".equals(mirror.getAttribute("ready")))); + } + } catch (Exception ex) { + throw new CloudRuntimeException(String.format("Failed to parse the XML of VM [%s] due to [%s].", domain.getName(), ex.getMessage()), ex); + } + return diskChains; + } - List grandChildren = mergeTreeTO.getGrandChildren().stream() - .map(snapshotTo -> new QemuImgFile(storagePool.getLocalPathFor(snapshotTo.getPath()), QemuImg.PhysicalDiskFormat.QCOW2)) - .collect(Collectors.toList()); + private Element getChildElement(Element parent, String name) { + NodeList children = parent.getChildNodes(); + for (int i = 0; i < children.getLength(); i++) { + Node child = children.item(i); + if (child.getNodeType() == Node.ELEMENT_NODE && name.equals(child.getNodeName())) { + return (Element) child; + } + } + return null; + } - logger.debug("Rebasing grandChildren [{}] into parent at [{}].", grandChildren, parentSnapshotLocalPath); - for (QemuImgFile grandChild : grandChildren) { - qemuImg.rebase(grandChild, parent, parent.getFormat().toString(), false); + private String getSourcePath(Element source) { + if (source == null) { + return null; + } + for (String attribute : List.of("file", "dev", "name")) { + if (StringUtils.isNotBlank(source.getAttribute(attribute))) { + return source.getAttribute(attribute); } } + return null; + } + + private static boolean samePath(String path, String otherPath) { + return path != null && otherPath != null && Path.of(path).normalize().equals(Path.of(otherPath).normalize()); + } + + private String getVolumeUuid(SnapshotMergeTreeTO snapshotMergeTreeTO) { + return ((SnapshotObjectTO) snapshotMergeTreeTO.getParent()).getVolume().getUuid(); + } + + protected QemuImg getQemuImg(long timeoutInMillis) throws LibvirtException, QemuImgException { + return new QemuImg(timeoutInMillis); + } + + protected long getPollIntervalInMillis() { + return POLL_INTERVAL_IN_MILLIS; + } + + protected static class RunningVmDiskMerge { + protected final String volumeUuid; + protected final String label; + protected final String basePath; + protected final String topPath; + protected final boolean active; + protected final List grandChildPaths; + protected boolean observedApplied; + + protected RunningVmDiskMerge(String volumeUuid, String label, String basePath, String topPath, boolean active, List grandChildPaths) { + this.volumeUuid = volumeUuid; + this.label = label; + this.basePath = basePath; + this.topPath = topPath; + this.active = active; + this.grandChildPaths = grandChildPaths; + } + } + + protected static class StoppedVmDiskMerge { + protected final String volumeUuid; + protected final String parentPath; + protected final String childPath; + protected final List grandChildPaths; + + protected StoppedVmDiskMerge(String volumeUuid, String parentPath, String childPath, List grandChildPaths) { + this.volumeUuid = volumeUuid; + this.parentPath = parentPath; + this.childPath = childPath; + this.grandChildPaths = grandChildPaths; + } + } + + protected static class DiskChain { + protected final String activeSource; + protected final List backingFiles; + protected final String mirrorJob; + protected final boolean mirrorReady; + + protected DiskChain(String activeSource, List backingFiles, String mirrorJob, boolean mirrorReady) { + this.activeSource = activeSource; + this.backingFiles = backingFiles; + this.mirrorJob = StringUtils.isBlank(mirrorJob) ? null : mirrorJob; + this.mirrorReady = mirrorReady; + } + + protected boolean isActiveCommitReady() { + return Objects.equals(mirrorJob, "active-commit") && mirrorReady; + } + + protected boolean contains(String path) { + return samePath(activeSource, path) || backingFiles.stream().anyMatch(backingFile -> samePath(backingFile, path)); + } - return new Answer(cmd, true, null); + @Override + public String toString() { + List chain = new ArrayList<>(); + chain.add(activeSource); + chain.addAll(backingFiles); + return StringUtils.join(chain, " -> ") + (mirrorJob == null ? "" : String.format(" (job [%s], ready [%s])", mirrorJob, mirrorReady)); + } } } diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapperTest.java new file mode 100644 index 000000000000..95e8a506229f --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapperTest.java @@ -0,0 +1,465 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package com.cloud.hypervisor.kvm.resource.wrapper; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.qemu.QemuImg; +import org.apache.cloudstack.utils.qemu.QemuImgException; +import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.junit.Before; +import org.junit.Test; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.DomainBlockJobInfo; +import org.libvirt.LibvirtException; + +import com.cloud.agent.api.Answer; +import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotAnswer; +import com.cloud.agent.api.storage.MergeDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.SnapshotMergeTreeTO; +import com.cloud.agent.api.to.DataObjectType; +import com.cloud.agent.api.to.DataTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.resource.LibvirtVMDef; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage; +import com.cloud.vm.VirtualMachine; + +public class LibvirtMergeDiskOnlyVMSnapshotCommandWrapperTest { + + private static final String VM_NAME = "i-2-10-VM"; + + private QemuImg qemuImg; + private LibvirtMergeDiskOnlyVMSnapshotCommandWrapper wrapper; + private LibvirtComputingResource resource; + private LibvirtUtilitiesHelper libvirtUtilitiesHelper; + private KVMStoragePoolManager storagePoolManager; + private KVMStoragePool storagePool; + private Domain domain; + private FakeBlockJobs fakeBlockJobs; + private Path poolDirectory; + + @Before + public void setup() throws Exception { + qemuImg = mock(QemuImg.class); + wrapper = new LibvirtMergeDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected QemuImg getQemuImg(long timeoutInMillis) { + return qemuImg; + } + + @Override + protected long getPollIntervalInMillis() { + return 10; + } + }; + + resource = mock(LibvirtComputingResource.class); + libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + storagePoolManager = mock(KVMStoragePoolManager.class); + storagePool = mock(KVMStoragePool.class); + domain = mock(Domain.class); + Connect connect = mock(Connect.class); + poolDirectory = Files.createTempDirectory("merge-pool-"); + fakeBlockJobs = new FakeBlockJobs(poolDirectory); + + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDomain(connect, VM_NAME)).thenReturn(domain); + when(resource.getDisks(connect, VM_NAME)).thenReturn(new ArrayList<>()); + when(resource.getStoragePoolMgr()).thenReturn(storagePoolManager); + when(resource.getQcow2DeltaMergeTimeout()).thenReturn(259200); + when(resource.getCmdsTimeout()).thenReturn(7200000); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(any(), any())).thenReturn(storagePool); + when(storagePoolManager.getStoragePool(any(), anyString())).thenReturn(storagePool); + when(storagePool.getLocalPathFor(anyString())).thenAnswer(invocation -> poolDirectory.resolve(invocation.getArgument(0, String.class)).toString()); + when(domain.getName()).thenReturn(VM_NAME); + fakeBlockJobs.attachTo(domain); + } + + @Test + public void testGetMergeTimeoutInSecondsLeavesAMarginBeforeTheCommandWait() { + assertEquals(259200, wrapper.getMergeTimeoutInSeconds(commandWithWait(0), 259200)); + assertEquals(15, wrapper.getMergeTimeoutInSeconds(commandWithWait(20), 259200)); + assertEquals(270, wrapper.getMergeTimeoutInSeconds(commandWithWait(300), 259200)); + assertEquals(1740, wrapper.getMergeTimeoutInSeconds(commandWithWait(1800), 259200)); + assertEquals(1, wrapper.getMergeTimeoutInSeconds(commandWithWait(3), 259200)); + assertEquals(100, wrapper.getMergeTimeoutInSeconds(commandWithWait(1800), 100)); + } + + @Test + public void testGetDiskChainsReadsActiveImageBackingChainAndMirror() throws Exception { + fakeBlockJobs.addDisk("vda", "top-a", "snap-a", "base-a"); + fakeBlockJobs.disk("vda").job = "active-commit"; + fakeBlockJobs.disk("vda").ready = true; + + LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.DiskChain diskChain = wrapper.getDiskChains(domain).get("vda"); + + assertNotNull(diskChain); + assertEquals(fakeBlockJobs.path("top-a"), diskChain.activeSource); + assertEquals(List.of(fakeBlockJobs.path("snap-a"), fakeBlockJobs.path("base-a")), diskChain.backingFiles); + assertTrue(diskChain.isActiveCommitReady()); + assertTrue(diskChain.contains(fakeBlockJobs.path("snap-a"))); + assertFalse(diskChain.contains(fakeBlockJobs.path("other"))); + } + + @Test + public void testRunningVmMergePivotsDisksOnlyOnceEveryDiskIsReady() throws Exception { + fakeBlockJobs.addDisk("vda", "top-a", "snap-a", "base-a"); + fakeBlockJobs.addDisk("vdb", "top-b", "snap-b", "base-b"); + fakeBlockJobs.disk("vdb").pollsUntilFinished = 3; + MergeDiskOnlyVmSnapshotCommand command = runningCommand(1800, activeTree("vol-a", "vda", "top-a", "snap-a"), activeTree("vol-b", "vdb", "top-b", "snap-b")); + + Answer answer = wrapper.execute(command, resource); + + assertTrue(answer.getDetails(), answer.getResult()); + assertEquals(List.of("vol-a", "vol-b"), ((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids()); + assertFalse("A disk was pivoted while another one was not ready yet.", fakeBlockJobs.pivotedBeforeEveryDiskWasReady); + assertEquals(fakeBlockJobs.path("snap-a"), fakeBlockJobs.disk("vda").chain.get(0)); + assertEquals(fakeBlockJobs.path("snap-b"), fakeBlockJobs.disk("vdb").chain.get(0)); + assertFalse(Files.exists(Path.of(fakeBlockJobs.path("top-a")))); + assertFalse(Files.exists(Path.of(fakeBlockJobs.path("top-b")))); + verify(domain, never()).blockJobAbort(anyString(), eq(0)); + } + + @Test + public void testRunningVmMergeCancelsEveryDiskWhenOneIsNotReadyInTime() throws Exception { + fakeBlockJobs.addDisk("vda", "top-a", "snap-a", "base-a"); + fakeBlockJobs.addDisk("vdb", "top-b", "snap-b", "base-b"); + fakeBlockJobs.disk("vdb").pollsUntilFinished = Integer.MAX_VALUE; + MergeDiskOnlyVmSnapshotCommand command = runningCommand(6, activeTree("vol-a", "vda", "top-a", "snap-a"), activeTree("vol-b", "vdb", "top-b", "snap-b")); + + Answer answer = wrapper.execute(command, resource); + + assertFalse(answer.getResult()); + assertTrue(((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids().isEmpty()); + verify(domain, never()).blockJobAbort(anyString(), eq(Domain.BlockJobAbortFlags.PIVOT)); + verify(domain).blockJobAbort("vda", 0); + verify(domain).blockJobAbort("vdb", 0); + assertEquals(List.of(fakeBlockJobs.path("top-a"), fakeBlockJobs.path("snap-a"), fakeBlockJobs.path("base-a")), fakeBlockJobs.disk("vda").chain); + assertEquals(List.of(fakeBlockJobs.path("top-b"), fakeBlockJobs.path("snap-b"), fakeBlockJobs.path("base-b")), fakeBlockJobs.disk("vdb").chain); + assertTrue(Files.exists(Path.of(fakeBlockJobs.path("top-a")))); + assertTrue(Files.exists(Path.of(fakeBlockJobs.path("top-b")))); + } + + @Test + public void testRunningVmMergeReportsPivotedDisksWhenALaterPivotFails() throws Exception { + fakeBlockJobs.addDisk("vda", "top-a", "snap-a", "base-a"); + fakeBlockJobs.addDisk("vdb", "top-b", "snap-b", "base-b"); + fakeBlockJobs.disk("vdb").pivotFails = true; + MergeDiskOnlyVmSnapshotCommand command = runningCommand(1800, activeTree("vol-a", "vda", "top-a", "snap-a"), activeTree("vol-b", "vdb", "top-b", "snap-b")); + + Answer answer = wrapper.execute(command, resource); + + assertFalse(answer.getResult()); + assertEquals(List.of("vol-a"), ((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids()); + verify(domain).blockJobAbort("vdb", 0); + assertFalse(Files.exists(Path.of(fakeBlockJobs.path("top-a")))); + assertTrue(Files.exists(Path.of(fakeBlockJobs.path("top-b")))); + assertEquals(fakeBlockJobs.path("top-b"), fakeBlockJobs.disk("vdb").chain.get(0)); + } + + @Test + public void testRunningVmMergeReportsOnlyTheNonActiveCommitsThatCompleted() throws Exception { + fakeBlockJobs.addDisk("vda", "top-a", "child-a", "parent-a", "base-a"); + fakeBlockJobs.addDisk("vdb", "top-b", "child-b", "parent-b", "base-b"); + fakeBlockJobs.disk("vdb").commitFails = true; + Files.createFile(Path.of(fakeBlockJobs.path("grandchild-a"))); + MergeDiskOnlyVmSnapshotCommand command = runningCommand(1800, nonActiveTree("vol-a", "vda", "child-a", "parent-a", "grandchild-a"), + nonActiveTree("vol-b", "vdb", "child-b", "parent-b")); + + Answer answer = wrapper.execute(command, resource); + + assertFalse(answer.getResult()); + assertEquals(List.of("vol-a"), ((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids()); + verify(domain, never()).blockJobAbort(anyString(), eq(Domain.BlockJobAbortFlags.PIVOT)); + verify(qemuImg).rebase(argThat(file -> file.getFileName().equals(fakeBlockJobs.path("grandchild-a"))), + argThat(file -> file.getFileName().equals(fakeBlockJobs.path("parent-a"))), anyString(), eq(false)); + assertFalse(Files.exists(Path.of(fakeBlockJobs.path("child-a")))); + assertTrue(Files.exists(Path.of(fakeBlockJobs.path("child-b")))); + assertEquals(List.of(fakeBlockJobs.path("top-b"), fakeBlockJobs.path("child-b"), fakeBlockJobs.path("parent-b"), fakeBlockJobs.path("base-b")), + fakeBlockJobs.disk("vdb").chain); + } + + @Test + public void testStoppedVmMergeDoesNotChangeAnyChainWhenACommitFails() throws Exception { + Files.createFile(poolDirectory.resolve("child-a")); + Files.createFile(poolDirectory.resolve("child-b")); + doThrow(new QemuImgException("commit failed")).when(qemuImg).commit(argThat(file -> file.getFileName().endsWith("child-b")), any(), eq(true)); + MergeDiskOnlyVmSnapshotCommand command = stoppedCommand(stoppedTree("vol-a", "child-a", "parent-a", "grandchild-a"), + stoppedTree("vol-b", "child-b", "parent-b")); + + Answer answer = wrapper.execute(command, resource); + + assertFalse(answer.getResult()); + assertTrue(((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids().isEmpty()); + verify(qemuImg, never()).rebase(any(QemuImgFile.class), any(QemuImgFile.class), anyString(), eq(false)); + assertTrue(Files.exists(poolDirectory.resolve("child-a"))); + assertTrue(Files.exists(poolDirectory.resolve("child-b"))); + } + + @Test + public void testStoppedVmMergeChangesTheChainsOnlyAfterEveryCommitSucceeded() throws Exception { + Files.createFile(poolDirectory.resolve("child-a")); + Files.createFile(poolDirectory.resolve("child-b")); + MergeDiskOnlyVmSnapshotCommand command = stoppedCommand(stoppedTree("vol-a", "child-a", "parent-a", "grandchild-a"), + stoppedTree("vol-b", "child-b", "parent-b")); + + Answer answer = wrapper.execute(command, resource); + + assertTrue(answer.getDetails(), answer.getResult()); + assertEquals(List.of("vol-a", "vol-b"), ((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids()); + verify(qemuImg).rebase(argThat(file -> file.getFileName().endsWith("grandchild-a")), argThat(file -> file.getFileName().endsWith("parent-a")), + anyString(), eq(false)); + assertFalse(Files.exists(poolDirectory.resolve("child-a"))); + assertFalse(Files.exists(poolDirectory.resolve("child-b"))); + } + + @Test + public void testStoppedVmMergeAbortsWithoutCommittingOnceTheDeadlineIsReached() throws Exception { + MergeDiskOnlyVmSnapshotCommand command = stoppedCommand(stoppedTree("vol-a", "child-a", "parent-a"), stoppedTree("vol-b", "child-b", "parent-b")); + command.setWait(6); + doAnswer(invocation -> { + Thread.sleep(1100); + return null; + }).when(qemuImg).commit(argThat(file -> file.getFileName().endsWith("child-a")), any(), eq(true)); + + Answer answer = wrapper.execute(command, resource); + + assertFalse(answer.getResult()); + assertTrue(((MergeDiskOnlyVmSnapshotAnswer) answer).getMergedVolumeUuids().isEmpty()); + verify(qemuImg, never()).commit(argThat(file -> file.getFileName().endsWith("child-b")), any(), eq(true)); + } + + private MergeDiskOnlyVmSnapshotCommand commandWithWait(int wait) { + MergeDiskOnlyVmSnapshotCommand command = new MergeDiskOnlyVmSnapshotCommand(new ArrayList<>(), VirtualMachine.State.Running, VM_NAME); + command.setWait(wait); + return command; + } + + private MergeDiskOnlyVmSnapshotCommand runningCommand(int wait, SnapshotMergeTreeTO... trees) { + MergeDiskOnlyVmSnapshotCommand command = new MergeDiskOnlyVmSnapshotCommand(List.of(trees), VirtualMachine.State.Running, VM_NAME); + command.setWait(wait); + return command; + } + + private MergeDiskOnlyVmSnapshotCommand stoppedCommand(SnapshotMergeTreeTO... trees) { + MergeDiskOnlyVmSnapshotCommand command = new MergeDiskOnlyVmSnapshotCommand(List.of(trees), VirtualMachine.State.Stopped, VM_NAME); + command.setWait(1800); + return command; + } + + private SnapshotMergeTreeTO activeTree(String volumeUuid, String label, String topPath, String basePath) { + VolumeObjectTO volume = volume(volumeUuid, label, topPath); + when(volume.getObjectType()).thenReturn(DataObjectType.VOLUME); + return new SnapshotMergeTreeTO(snapshot(basePath, volume), volume, new ArrayList<>()); + } + + private SnapshotMergeTreeTO nonActiveTree(String volumeUuid, String label, String topPath, String basePath, String... grandChildren) { + VolumeObjectTO volume = volume(volumeUuid, label, "top-" + label); + SnapshotObjectTO child = snapshot(topPath, volume); + when(child.getObjectType()).thenReturn(DataObjectType.SNAPSHOT); + return new SnapshotMergeTreeTO(snapshot(basePath, volume), child, dataTos(volume, grandChildren)); + } + + private SnapshotMergeTreeTO stoppedTree(String volumeUuid, String childPath, String parentPath, String... grandChildren) { + VolumeObjectTO volume = volume(volumeUuid, null, childPath); + SnapshotObjectTO parent = snapshot(parentPath, volume); + PrimaryDataStoreTO dataStore = mock(PrimaryDataStoreTO.class); + when(dataStore.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(dataStore.getUuid()).thenReturn("pool"); + when(parent.getDataStore()).thenReturn(dataStore); + return new SnapshotMergeTreeTO(parent, snapshot(childPath, volume), dataTos(volume, grandChildren)); + } + + private List dataTos(VolumeObjectTO volume, String... paths) { + List dataTos = new ArrayList<>(); + for (String path : paths) { + dataTos.add(snapshot(path, volume)); + } + return dataTos; + } + + private VolumeObjectTO volume(String uuid, String label, String path) { + VolumeObjectTO volume = mock(VolumeObjectTO.class); + when(volume.getUuid()).thenReturn(uuid); + when(volume.getPath()).thenReturn(path); + if (label != null) { + LibvirtVMDef.DiskDef diskDef = mock(LibvirtVMDef.DiskDef.class); + when(diskDef.getDiskLabel()).thenReturn(label); + when(resource.getDiskWithPathOfVolumeObjectTO(any(), eq(volume))).thenReturn(diskDef); + } + return volume; + } + + private SnapshotObjectTO snapshot(String path, VolumeObjectTO volume) { + SnapshotObjectTO snapshot = mock(SnapshotObjectTO.class); + when(snapshot.getPath()).thenReturn(path); + when(snapshot.getVolume()).thenReturn(volume); + return snapshot; + } + + /** + * Imitates how Libvirt handles block commits: active commits stay ready until pivoted or cancelled, non-active commits finish on their own, and + * cancelling any of them leaves the backing chain unchanged. + */ + private static class FakeBlockJobs { + private final Path poolDirectory; + private final Map disks = new LinkedHashMap<>(); + private boolean pivotedBeforeEveryDiskWasReady; + + private FakeBlockJobs(Path poolDirectory) { + this.poolDirectory = poolDirectory; + } + + private String path(String name) { + return poolDirectory.resolve(name).toString(); + } + + private void addDisk(String label, String... chain) throws Exception { + FakeDisk disk = new FakeDisk(); + for (String name : chain) { + disk.chain.add(path(name)); + Files.createFile(Path.of(path(name))); + } + disks.put(label, disk); + } + + private FakeDisk disk(String label) { + return disks.get(label); + } + + private void attachTo(Domain domain) throws LibvirtException { + when(domain.getXMLDesc(0)).thenAnswer(invocation -> { + disks.values().forEach(FakeDisk::advance); + return xml(); + }); + when(domain.getBlockJobInfo(anyString(), eq(0))).thenAnswer(invocation -> { + DomainBlockJobInfo blockJobInfo = new DomainBlockJobInfo(); + FakeDisk disk = disks.get(invocation.getArgument(0, String.class)); + if (disk != null && disk.job != null) { + blockJobInfo.type = 3; + blockJobInfo.end = 100; + blockJobInfo.cur = disk.ready ? 100 : 50; + } + return blockJobInfo; + }); + doAnswer(invocation -> { + FakeDisk disk = disks.get(invocation.getArgument(0, String.class)); + disk.base = invocation.getArgument(1, String.class); + disk.top = invocation.getArgument(2, String.class); + disk.job = (invocation.getArgument(4, Integer.class) & Domain.BlockCommitFlags.ACTIVE) != 0 ? "active-commit" : "commit"; + return null; + }).when(domain).blockCommit(anyString(), anyString(), anyString(), anyLong(), anyInt()); + doAnswer(invocation -> { + FakeDisk disk = disks.get(invocation.getArgument(0, String.class)); + int flags = invocation.getArgument(1, Integer.class); + if (flags == Domain.BlockJobAbortFlags.PIVOT) { + if (disk.pivotFails || !disk.ready) { + throw mock(LibvirtException.class); + } + pivotedBeforeEveryDiskWasReady |= disks.values().stream().anyMatch(other -> other.job != null && !other.ready); + disk.chain.subList(0, disk.chain.indexOf(disk.base)).clear(); + } + disk.job = null; + disk.ready = false; + return null; + }).when(domain).blockJobAbort(anyString(), anyInt()); + } + + private String xml() { + StringBuilder xml = new StringBuilder("" + VM_NAME + ""); + for (Map.Entry entry : disks.entrySet()) { + FakeDisk disk = entry.getValue(); + xml.append(""); + for (int i = 1; i < disk.chain.size(); i++) { + xml.append(""); + } + xml.append(""); + for (int i = 1; i < disk.chain.size(); i++) { + xml.append(""); + } + if ("active-commit".equals(disk.job)) { + xml.append(""); + } + xml.append(""); + } + return xml.append("").toString(); + } + } + + private static class FakeDisk { + private final List chain = new ArrayList<>(); + private String job; + private boolean ready; + private String base; + private String top; + private int pollsUntilFinished; + private boolean pivotFails; + private boolean commitFails; + + private void advance() { + if (job == null || ready) { + return; + } + if (pollsUntilFinished > 0) { + if (pollsUntilFinished != Integer.MAX_VALUE) { + pollsUntilFinished--; + } + return; + } + if ("active-commit".equals(job)) { + ready = true; + return; + } + if (!commitFails) { + chain.remove(top); + } + job = null; + } + } +}