diff --git a/.editorconfig b/.editorconfig new file mode 100644 index 000000000000..fffc67955b55 --- /dev/null +++ b/.editorconfig @@ -0,0 +1,26 @@ +# 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. +root = true + +[*] +charset = utf-8 +end_of_line = lf +# indent_size = 4 +indent_style = space +insert_final_newline = true +# max_line_length = 120 +# tab_width = 4 diff --git a/.github/linters/codespell.txt b/.github/linters/codespell.txt index 27761c70f446..e954402a57c3 100644 --- a/.github/linters/codespell.txt +++ b/.github/linters/codespell.txt @@ -187,6 +187,7 @@ environmnet equivalant erro erronous +errorprone everthing everytime excute diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 9670b6e7c13a..02d63811e36e 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -39,3 +39,17 @@ example.ver.1 > example.ver.2: which can now be attached to Instances. This is to prevent the Secondary Storage to grow to enormous sizes as Linux Distributions keep growing in size while a stripped down Linux should fit on a 2.88MB floppy. + +4.22.0.0 > 4.22.0.1: + * Disk-only instance snapshots for KVM UEFI VMs now include a sidecar copy of + the active NVRAM state so revert operations restore both disk and firmware + boot state consistently. + + * UEFI disk-only instance snapshots taken before this change do not contain an + NVRAM sidecar and cannot be safely reverted. Take a new snapshot after + upgrading before relying on revert for UEFI VMs. + + * Taking a disk-only instance snapshot for KVM UEFI VMs now briefly suspends + the guest while the NVRAM sidecar is copied, so that the captured firmware + state is consistent with the disk snapshot. Non-UEFI VMs are unaffected and + continue to snapshot live. diff --git a/agent/src/main/java/com/cloud/agent/mockvm/MockVmMgr.java b/agent/src/main/java/com/cloud/agent/mockvm/MockVmMgr.java index 54fdde3d3d28..d97207c9f30c 100644 --- a/agent/src/main/java/com/cloud/agent/mockvm/MockVmMgr.java +++ b/agent/src/main/java/com/cloud/agent/mockvm/MockVmMgr.java @@ -249,7 +249,7 @@ public void freeVncPort(int port) { public MockVm createVmFromSpec(VirtualMachineTO vmSpec) { String vmName = vmSpec.getName(); long ramSize = vmSpec.getMinRam(); - int utilizationPercent = randSeed.nextInt() % 100; + int utilizationPercent = randSeed.nextInt(100); MockVm vm = null; synchronized (this) { diff --git a/api/src/main/java/com/cloud/host/Host.java b/api/src/main/java/com/cloud/host/Host.java index b52348201516..a7b89b8a2b85 100644 --- a/api/src/main/java/com/cloud/host/Host.java +++ b/api/src/main/java/com/cloud/host/Host.java @@ -55,6 +55,7 @@ public static String[] toStrings(Host.Type... types) { } String HOST_UEFI_ENABLE = "host.uefi.enable"; + String HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM = "host.kvm.diskonlyvmsnapshot.nvram"; String HOST_VOLUME_ENCRYPTION = "host.volume.encryption"; String HOST_INSTANCE_CONVERSION = "host.instance.conversion"; String HOST_VDDK_SUPPORT = "host.vddk.support"; diff --git a/api/src/main/java/com/cloud/resource/ResourceService.java b/api/src/main/java/com/cloud/resource/ResourceService.java index 3cdf8fc64e99..202d7fe63c49 100644 --- a/api/src/main/java/com/cloud/resource/ResourceService.java +++ b/api/src/main/java/com/cloud/resource/ResourceService.java @@ -91,7 +91,7 @@ public interface ResourceService { DataCenter getZone(Long zoneId); - List getSupportedHypervisorTypes(long zoneId, boolean forVirtualRouter, Long podId); + List getSupportedHypervisorTypes(long zoneId, boolean forSystemVm, Long podId); boolean releaseHostReservation(Long hostId); diff --git a/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java b/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java index 5c78d6bedd64..c91f5b736514 100644 --- a/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java +++ b/api/src/main/java/com/cloud/vm/VirtualMachineProfile.java @@ -79,6 +79,7 @@ public static class Param { public static final Param PreserveNics = new Param("PreserveNics"); public static final Param ConsiderLastHost = new Param("ConsiderLastHost"); public static final Param ReturnAfterVolumePrepare = new Param("ReturnAfterVolumePrepare"); + public static final Param ResetPasswordOnRestore = new Param("ResetPasswordOnRestore"); private String name; diff --git a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java index a8ff00c40ff3..91beefde509b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/main/java/org/apache/cloudstack/api/ApiConstants.java @@ -452,6 +452,7 @@ public class ApiConstants { public static final String CURRENT_PASSWORD = "currentpassword"; public static final String SHOULD_UPDATE_PASSWORD = "update_passwd_on_host"; public static final String PASSWORD_ENABLED = "passwordenabled"; + public static final String RESET_PASSWORD = "resetpassword"; public static final String SSHKEY_ENABLED = "sshkeyenabled"; public static final String PATH = "path"; public static final String PATH_READY = "pathready"; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java index 9de06715ee74..fd77aeb47081 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/backup/UpdateBackupOfferingCmd.java @@ -83,8 +83,8 @@ public Boolean getAllowUserDrivenBackups() { public void execute() { try { if (StringUtils.isAllEmpty(getName(), getDescription()) && getAllowUserDrivenBackups() == null) { - throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated, at least one of the", - "following should be informed: name, description or allowUserDrivenBackups.", id)); + throw new InvalidParameterValueException(String.format("Can't update Backup Offering [id: %s] because there are no parameters to be updated," + + " at least one of the following should be passed: name, description or allowUserDrivenBackups.", id)); } BackupOffering result = backupManager.updateBackupOffering(this); diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java index 6192bfb4540a..7390d4d2da71 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vm/CreateVMFromBackupCmd.java @@ -70,6 +70,11 @@ public class CreateVMFromBackupCmd extends BaseDeployVMCmd { @Parameter(name = ApiConstants.PRESERVE_IP, type = CommandType.BOOLEAN, description = "Use the same IP/MAC addresses as stored in the backup metadata. Works only if the original Instance is deleted and the IP/MAC address is available.") private Boolean preserveIp; + @Parameter(name = ApiConstants.RESET_PASSWORD, type = CommandType.BOOLEAN, + description = "For a password enabled template, whether to generate a new password for the created Instance and return it in the response. " + + "If not specified, the zone setting `restore.vm.from.backup.reset.password` decides.", since = "4.22.1.0") + private Boolean resetPassword; + ///////////////////////////////////////////////////// /////////////////// Accessors /////////////////////// ///////////////////////////////////////////////////// @@ -90,6 +95,10 @@ public boolean getPreserveIp() { return (preserveIp != null) ? preserveIp : false; } + public Boolean getResetPassword() { + return resetPassword; + } + @Override public void create() { UserVm vm; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java index 6e1a7daf4c23..f2decf21a1db 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/vmsnapshot/CreateVMSnapshotCmd.java @@ -37,7 +37,8 @@ import com.cloud.vm.VirtualMachine; import com.cloud.vm.snapshot.VMSnapshot; -@APICommand(name = "createVMSnapshot", description = "Creates Snapshot for an Instance.", responseObject = VMSnapshotResponse.class, since = "4.2.0", entityType = {VMSnapshot.class}, +@APICommand(name = "createVMSnapshot", description = "Creates Snapshot for an Instance. Running KVM UEFI disk-only snapshots briefly suspend the Instance while copying NVRAM state.", + responseObject = VMSnapshotResponse.class, since = "4.2.0", entityType = {VMSnapshot.class}, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class CreateVMSnapshotCmd extends BaseAsyncCreateCmd { diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreDetailResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreDetailResponse.java index 0afef6166f8b..036f5f0d170b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreDetailResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ImageStoreDetailResponse.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.api.response; +import java.util.Objects; + import com.google.gson.annotations.SerializedName; import org.apache.cloudstack.api.BaseResponse; @@ -81,7 +83,7 @@ public boolean equals(Object obj) { return false; } else if (!oid.equals(other.getName())) return false; - else if (this.getValue().equals(other.getValue())) + else if (!Objects.equals(this.getValue(), other.getValue())) return false; return true; } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/NetworkOfferingResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/NetworkOfferingResponse.java index 87f960590283..740dd23604a9 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/NetworkOfferingResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/NetworkOfferingResponse.java @@ -91,6 +91,10 @@ public class NetworkOfferingResponse extends BaseResponseWithAnnotations { @Param(description = "The ID of the service offering used by virtual router provider") private String serviceOfferingId; + @SerializedName(ApiConstants.SERVICE_OFFERING_NAME) + @Param(description = "the name of the service offering used by virtual router provider") + private String serviceOfferingName; + @SerializedName(ApiConstants.SERVICE) @Param(description = "The list of supported services", responseObject = ServiceResponse.class) private List services; @@ -330,4 +334,12 @@ public String getRoutingMode() { public void setRoutingMode(String routingMode) { this.routingMode = routingMode; } + + public String getServiceOfferingName() { + return serviceOfferingName; + } + + public void setServiceOfferingName(String serviceOfferingName) { + this.serviceOfferingName = serviceOfferingName; + } } diff --git a/api/src/main/java/org/apache/cloudstack/api/response/ProjectResponse.java b/api/src/main/java/org/apache/cloudstack/api/response/ProjectResponse.java index 40f9405d0fc5..135dcf97affc 100644 --- a/api/src/main/java/org/apache/cloudstack/api/response/ProjectResponse.java +++ b/api/src/main/java/org/apache/cloudstack/api/response/ProjectResponse.java @@ -452,7 +452,7 @@ public void setNetworkAvailable(String networkAvailable) { @Override public void setVpcLimit(String vpcLimit) { - this.vpcLimit = networkLimit; + this.vpcLimit = vpcLimit; } @Override diff --git a/api/src/test/java/com/cloud/network/IsolationMethodTest.java b/api/src/test/java/com/cloud/network/IsolationMethodTest.java index 2db3ec55db24..dd08f46b5937 100644 --- a/api/src/test/java/com/cloud/network/IsolationMethodTest.java +++ b/api/src/test/java/com/cloud/network/IsolationMethodTest.java @@ -25,7 +25,7 @@ public class IsolationMethodTest { @After public void cleanTheRegistry() { - PhysicalNetwork.IsolationMethod.registeredIsolationMethods.removeAll(PhysicalNetwork.IsolationMethod.registeredIsolationMethods); + PhysicalNetwork.IsolationMethod.registeredIsolationMethods.clear(); } @Test diff --git a/api/src/test/java/org/apache/cloudstack/api/response/ImageStoreDetailResponseTest.java b/api/src/test/java/org/apache/cloudstack/api/response/ImageStoreDetailResponseTest.java new file mode 100644 index 000000000000..8550eac9fc1d --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/response/ImageStoreDetailResponseTest.java @@ -0,0 +1,45 @@ +// 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 org.apache.cloudstack.api.response; + +import org.junit.Assert; +import org.junit.Test; + +public class ImageStoreDetailResponseTest { + + @Test + public void equalsIsTrueForSameNameAndValue() { + ImageStoreDetailResponse a = new ImageStoreDetailResponse("key", "value"); + ImageStoreDetailResponse b = new ImageStoreDetailResponse("key", "value"); + Assert.assertEquals(a, b); + Assert.assertEquals(a.hashCode(), b.hashCode()); + } + + @Test + public void equalsIsFalseWhenValueDiffers() { + ImageStoreDetailResponse a = new ImageStoreDetailResponse("key", "value"); + ImageStoreDetailResponse c = new ImageStoreDetailResponse("key", "other"); + Assert.assertNotEquals(a, c); + } + + @Test + public void equalsIsFalseWhenNameDiffers() { + ImageStoreDetailResponse a = new ImageStoreDetailResponse("key", "value"); + ImageStoreDetailResponse d = new ImageStoreDetailResponse("other", "value"); + Assert.assertNotEquals(a, d); + } +} diff --git a/api/src/test/java/org/apache/cloudstack/api/response/ProjectResponseTest.java b/api/src/test/java/org/apache/cloudstack/api/response/ProjectResponseTest.java new file mode 100644 index 000000000000..a6f5caa4a76a --- /dev/null +++ b/api/src/test/java/org/apache/cloudstack/api/response/ProjectResponseTest.java @@ -0,0 +1,34 @@ +// 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 org.apache.cloudstack.api.response; + +import org.junit.Assert; +import org.junit.Test; +import org.springframework.test.util.ReflectionTestUtils; + +public class ProjectResponseTest { + + @Test + public void setVpcLimitStoresItsOwnParameterNotTheNetworkLimit() { + ProjectResponse response = new ProjectResponse(); + response.setNetworkLimit("5"); + response.setVpcLimit("10"); + + Assert.assertEquals("10", ReflectionTestUtils.getField(response, "vpcLimit")); + Assert.assertEquals("5", ReflectionTestUtils.getField(response, "networkLimit")); + } +} diff --git a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java index 4d61249c7cbc..ffa4eaff2963 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java @@ -27,13 +27,24 @@ public class CreateDiskOnlyVmSnapshotAnswer extends Answer { protected Map> mapVolumeToSnapshotSizeAndNewVolumePath; + private String nvramSnapshotPath; public CreateDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, Map> mapVolumeToSnapshotSizeAndNewVolumePath) { + this(command, success, details, mapVolumeToSnapshotSizeAndNewVolumePath, null); + } + + public CreateDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, Map> mapVolumeToSnapshotSizeAndNewVolumePath, + String nvramSnapshotPath) { super(command, success, details); this.mapVolumeToSnapshotSizeAndNewVolumePath = mapVolumeToSnapshotSizeAndNewVolumePath; + this.nvramSnapshotPath = nvramSnapshotPath; } public Map> getMapVolumeToSnapshotSizeAndNewVolumePath() { return mapVolumeToSnapshotSizeAndNewVolumePath; } + + public String getNvramSnapshotPath() { + return nvramSnapshotPath; + } } diff --git a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java index 952bf0c971de..7f328bab81db 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotCommand.java @@ -29,13 +29,30 @@ public class CreateDiskOnlyVmSnapshotCommand extends VMSnapshotBaseCommand { protected VirtualMachine.State vmState; + private final String vmUuid; + private final boolean uefiEnabled; public CreateDiskOnlyVmSnapshotCommand(String vmName, VMSnapshotTO snapshot, List volumeTOs, String guestOSType, VirtualMachine.State vmState) { + this(vmName, null, snapshot, volumeTOs, guestOSType, vmState, false); + } + + public CreateDiskOnlyVmSnapshotCommand(String vmName, String vmUuid, VMSnapshotTO snapshot, List volumeTOs, String guestOSType, + VirtualMachine.State vmState, boolean uefiEnabled) { super(vmName, snapshot, volumeTOs, guestOSType); + this.vmUuid = vmUuid; this.vmState = vmState; + this.uefiEnabled = uefiEnabled; } public VirtualMachine.State getVmState() { return vmState; } + + public String getVmUuid() { + return vmUuid; + } + + public boolean isUefiEnabled() { + return uefiEnabled; + } } diff --git a/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java index bf7bdd597360..1ac0a0ff719b 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/DeleteDiskOnlyVmSnapshotCommand.java @@ -19,24 +19,43 @@ package com.cloud.agent.api.storage; import com.cloud.agent.api.Command; - import com.cloud.agent.api.to.DataTO; - +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import java.util.List; public class DeleteDiskOnlyVmSnapshotCommand extends Command { - List snapshots; + private final List snapshots; + private final String nvramSnapshotPath; + private final PrimaryDataStoreTO primaryDataStore; public DeleteDiskOnlyVmSnapshotCommand(List snapshots) { + this(snapshots, null); + } + + public DeleteDiskOnlyVmSnapshotCommand(List snapshots, String nvramSnapshotPath) { + this(snapshots, nvramSnapshotPath, null); + } + + public DeleteDiskOnlyVmSnapshotCommand(List snapshots, String nvramSnapshotPath, PrimaryDataStoreTO primaryDataStore) { this.snapshots = snapshots; + this.nvramSnapshotPath = nvramSnapshotPath; + this.primaryDataStore = primaryDataStore; } public List getSnapshots() { return snapshots; } + public String getNvramSnapshotPath() { + return nvramSnapshotPath; + } + + public PrimaryDataStoreTO getPrimaryDataStore() { + return primaryDataStore; + } + @Override public boolean executeInSequence() { return false; diff --git a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java index 72bb92bcb10d..3c9859aa44af 100644 --- a/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java +++ b/core/src/main/java/com/cloud/agent/api/storage/RevertDiskOnlyVmSnapshotCommand.java @@ -27,11 +27,21 @@ public class RevertDiskOnlyVmSnapshotCommand extends Command { private List snapshotObjectTos; private String vmName; + private final String vmUuid; + private final boolean uefiEnabled; + private final String nvramSnapshotPath; public RevertDiskOnlyVmSnapshotCommand(List snapshotObjectTos, String vmName) { + this(snapshotObjectTos, vmName, null, false, null); + } + + public RevertDiskOnlyVmSnapshotCommand(List snapshotObjectTos, String vmName, String vmUuid, boolean uefiEnabled, String nvramSnapshotPath) { super(); this.snapshotObjectTos = snapshotObjectTos; this.vmName = vmName; + this.vmUuid = vmUuid; + this.uefiEnabled = uefiEnabled; + this.nvramSnapshotPath = nvramSnapshotPath; } public List getSnapshotObjectTos() { @@ -42,6 +52,18 @@ public String getVmName() { return vmName; } + public String getVmUuid() { + return vmUuid; + } + + public boolean isUefiEnabled() { + return uefiEnabled; + } + + public String getNvramSnapshotPath() { + return nvramSnapshotPath; + } + @Override public boolean executeInSequence() { return false; diff --git a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java index 83dfa2a62caa..64034e444eaf 100644 --- a/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java +++ b/core/src/main/java/com/cloud/agent/resource/virtualnetwork/facade/AbstractConfigItemFacade.java @@ -106,7 +106,7 @@ public abstract class AbstractConfigItemFacade { public static AbstractConfigItemFacade getInstance(final Class key) { if (!flyweight.containsKey(key)) { - throw new CloudRuntimeException("Unable to process the configuration for " + key.getClass().getName()); + throw new CloudRuntimeException("Unable to process the configuration for " + key.getName()); } final AbstractConfigItemFacade instance = flyweight.get(key); diff --git a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java index ff9546529bd8..1879e6ee2a6d 100644 --- a/core/src/main/java/com/cloud/network/HAProxyConfigurator.java +++ b/core/src/main/java/com/cloud/network/HAProxyConfigurator.java @@ -538,7 +538,7 @@ private List getRulesForPool(final LoadBalancerTO lbTO, final LoadBalanc } dstSubRule.add(sb.toString()); if (stickinessSubRule != null) { - sb.append(" cookie ").append(dest.getDestIp().replace(".", "_")).append('-').append(dest.getDestPort()).toString(); + sb.append(" cookie ").append(dest.getDestIp().replace(".", "_")).append('-').append(dest.getDestPort()); dstWithCookieSubRule.add(sb.toString()); } destsAvailable = true; diff --git a/core/src/main/java/com/cloud/resource/RequestWrapper.java b/core/src/main/java/com/cloud/resource/RequestWrapper.java index 54d8b289c8d6..3c375195239c 100644 --- a/core/src/main/java/com/cloud/resource/RequestWrapper.java +++ b/core/src/main/java/com/cloud/resource/RequestWrapper.java @@ -86,7 +86,7 @@ protected CommandWrapper retrieveCommands(final keepCommandClass = commandClass2; } catch (final ClassCastException e) { - throw new CommandNotSupported("No key found for '" + keepCommandClass.getClass() + "' in the Map!"); + throw new CommandNotSupported("No key found for '" + keepCommandClass + "' in the Map!"); } catch (final NullPointerException e) { // Will now traverse all the resource hierarchy. Returning null // is not a problem. diff --git a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java index 65d54fe81992..545169abd001 100644 --- a/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java +++ b/core/src/main/java/org/apache/cloudstack/agent/directdownload/DirectDownloadCommand.java @@ -53,7 +53,6 @@ protected DirectDownloadCommand (final String url, final Long templateId, final final Integer soTimeout, final Integer connectionRequestTimeout, final boolean followRedirects) { this.url = url; this.templateId = templateId; - this.destData = destData; this.destPool = destPool; this.checksum = checksum; this.headers = headers; diff --git a/core/src/test/java/com/cloud/resource/ServerResourceBaseTest.java b/core/src/test/java/com/cloud/resource/ServerResourceBaseTest.java index ed64e1482a62..68bfb5e9b3bf 100644 --- a/core/src/test/java/com/cloud/resource/ServerResourceBaseTest.java +++ b/core/src/test/java/com/cloud/resource/ServerResourceBaseTest.java @@ -182,36 +182,36 @@ public void tryToAutoDiscoverResourcePrivateNetworkInterfaceTestReturnNic() thro @Test public void defineResourceNetworkInterfacesTestUseXenbr0WhenPrivateNetworkInterfaceNotConfigured() { Map params = createParamsMap(null, "cloudbr1", "cloudbr2", "cloudbr3"); - try (MockedStatic ignored = Mockito.mockStatic(NetUtils.class)) { - Mockito.when(NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4); + try (MockedStatic mockedNetUtils = Mockito.mockStatic(NetUtils.class)) { + mockedNetUtils.when(() -> NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4); serverResourceBaseSpy.defineResourceNetworkInterfaces(params); - verifyAndAssertNetworkInterfaces("xenbr0", "cloudbr1", "cloudbr2", "cloudbr3"); + verifyAndAssertNetworkInterfaces(mockedNetUtils, "xenbr0", "cloudbr1", "cloudbr2", "cloudbr3"); } } @Test public void defineResourceNetworkInterfacesTestUseXenbr1WhenPublicNetworkInterfaceNotConfigured() { Map params = createParamsMap("cloudbr0", null, "cloudbr2", "cloudbr3"); - try (MockedStatic ignored = Mockito.mockStatic(NetUtils.class)) { - Mockito.when(NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4); + try (MockedStatic mockedNetUtils = Mockito.mockStatic(NetUtils.class)) { + mockedNetUtils.when(() -> NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4); serverResourceBaseSpy.defineResourceNetworkInterfaces(params); - verifyAndAssertNetworkInterfaces("cloudbr0", "xenbr1", "cloudbr2", "cloudbr3"); + verifyAndAssertNetworkInterfaces(mockedNetUtils, "cloudbr0", "xenbr1", "cloudbr2", "cloudbr3"); } } @Test public void defineResourceNetworkInterfacesTestUseConfiguredNetworkInterfaces() { Map params = createParamsMap("cloudbr0", "cloudbr1", "cloudbr2", "cloudbr3"); - try (MockedStatic ignored = Mockito.mockStatic(NetUtils.class)) { - Mockito.when(NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4); + try (MockedStatic mockedNetUtils = Mockito.mockStatic(NetUtils.class)) { + mockedNetUtils.when(() -> NetUtils.getNetworkInterface(Mockito.anyString())).thenReturn(networkInterfaceMock1, networkInterfaceMock2, networkInterfaceMock3, networkInterfaceMock4); serverResourceBaseSpy.defineResourceNetworkInterfaces(params); - verifyAndAssertNetworkInterfaces("cloudbr0", "cloudbr1", "cloudbr2", "cloudbr3"); + verifyAndAssertNetworkInterfaces(mockedNetUtils, "cloudbr0", "cloudbr1", "cloudbr2", "cloudbr3"); } } @@ -224,9 +224,8 @@ private Map createParamsMap(String... params) { return result; } - private void verifyAndAssertNetworkInterfaces(String... expectedResults) { - Mockito.verify(NetUtils.class, Mockito.times(4)); - NetUtils.getNetworkInterface(keyCaptor.capture()); + private void verifyAndAssertNetworkInterfaces(MockedStatic mockedNetUtils, String... expectedResults) { + mockedNetUtils.verify(() -> NetUtils.getNetworkInterface(keyCaptor.capture()), Mockito.times(4)); List keys = keyCaptor.getAllValues(); for (int i = 0; i < expectedResults.length; i++) { diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java index 0a6bbc876544..402bd2b6b9b9 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentAttache.java @@ -24,6 +24,7 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Random; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -361,7 +362,8 @@ public boolean equals(final Object obj) { return false; } AgentAttache that = (AgentAttache)obj; - return _id == that._id; + return Objects.equals(_uuid, that._uuid) && + Objects.equals(_name, that._name); } public void send(final Request req, final Listener listener) throws AgentUnavailableException { @@ -530,11 +532,12 @@ protected synchronized void sendNext(final long seq) { logger.debug(LOG_SEQ_FORMATTED_STRING, req.getSequence(), "Sending now. is current sequence."); try { send(req); + _currentSequence = req.getSequence(); } catch (AgentUnavailableException e) { logger.debug(LOG_SEQ_FORMATTED_STRING, req.getSequence(), "Unable to send the next sequence"); cancel(req.getSequence()); + sendNext(req.getSequence()); } - _currentSequence = req.getSequence(); } public void process(final Answer[] answers) { @@ -561,6 +564,11 @@ public void process(final Answer[] answers) { */ protected abstract boolean isClosed(); + @Override + public int hashCode() { + return Objects.hash(_uuid, _name); + } + protected class Alarm extends ManagedContextRunnable { long _seq; diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java index 8c69dcdc4828..ecb789a15bd5 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java @@ -108,10 +108,12 @@ import com.cloud.exception.UnsupportedVersionException; import com.cloud.ha.HighAvailabilityManager; import com.cloud.host.Host; +import com.cloud.host.DetailVO; import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.Status.Event; import com.cloud.host.dao.HostDao; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.hypervisor.HypervisorGuruManager; import com.cloud.org.Cluster; @@ -167,6 +169,8 @@ public class AgentManagerImpl extends ManagerBase implements AgentManager, Handl @Inject protected HostDao _hostDao = null; @Inject + protected HostDetailsDao _hostDetailsDao = null; + @Inject private ManagementServerHostDao _mshostDao; @Inject protected OutOfBandManagementDao outOfBandManagementDao; @@ -802,18 +806,24 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi ReadyAnswer readyAnswer = (ReadyAnswer)answer; Map detailsMap = readyAnswer.getDetailsMap(); if (detailsMap != null) { + _hostDao.loadDetails(host); + if (host.getDetails() == null) { + host.setDetails(new HashMap<>()); + } String uefiEnabled = detailsMap.get(Host.HOST_UEFI_ENABLE); + String diskOnlyVmSnapshotNvramSupport = detailsMap.get(Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM); String virtv2vVersion = detailsMap.get(Host.HOST_VIRTV2V_VERSION); String ovftoolVersion = detailsMap.get(Host.HOST_OVFTOOL_VERSION); String vddkSupport = detailsMap.get(Host.HOST_VDDK_SUPPORT); String vddkLibDir = detailsMap.get(Host.HOST_VDDK_LIB_DIR); String vddkVersion = detailsMap.get(Host.HOST_VDDK_VERSION); logger.debug("Got HOST_UEFI_ENABLE [{}] for host [{}]:", uefiEnabled, host); - if (ObjectUtils.anyNotNull(uefiEnabled, virtv2vVersion, ovftoolVersion, vddkSupport, vddkLibDir, vddkVersion)) { - _hostDao.loadDetails(host); + if (ObjectUtils.anyNotNull(uefiEnabled, diskOnlyVmSnapshotNvramSupport, virtv2vVersion, ovftoolVersion, vddkSupport, vddkLibDir, vddkVersion)) { boolean updateNeeded = false; - if (StringUtils.isNotBlank(uefiEnabled) && !uefiEnabled.equals(host.getDetails().get(Host.HOST_UEFI_ENABLE))) { - host.getDetails().put(Host.HOST_UEFI_ENABLE, uefiEnabled); + if (syncBooleanHostCapability(host, Host.HOST_UEFI_ENABLE, uefiEnabled)) { + updateNeeded = true; + } + if (syncBooleanHostCapability(host, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, diskOnlyVmSnapshotNvramSupport)) { updateNeeded = true; } if (StringUtils.isNotBlank(virtv2vVersion) && !virtv2vVersion.equals(host.getDetails().get(Host.HOST_VIRTV2V_VERSION))) { @@ -856,6 +866,26 @@ protected AgentAttache notifyMonitorsOfConnection(final AgentAttache attache, fi return attache; } + protected boolean syncBooleanHostCapability(HostVO host, String capabilityName, String advertisedValue) { + if (StringUtils.isNotBlank(advertisedValue)) { + if (!advertisedValue.equals(host.getDetails().get(capabilityName))) { + host.getDetails().put(capabilityName, advertisedValue); + return true; + } + return false; + } + + if (host.getDetails().containsKey(capabilityName)) { + host.getDetails().remove(capabilityName); + DetailVO hostDetail = _hostDetailsDao.findDetail(host.getId(), capabilityName); + if (hostDetail != null) { + _hostDetailsDao.remove(hostDetail.getId()); + } + return true; + } + return false; + } + @Override public boolean start() { ManagementServerHostVO msHost = _mshostDao.findByMsid(_nodeId); diff --git a/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java b/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java index ed18d1e82b7e..2edc9ad19bc3 100644 --- a/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java +++ b/engine/orchestration/src/main/java/com/cloud/agent/manager/DirectAgentAttache.java @@ -150,6 +150,11 @@ private synchronized void scheduleFromQueue() { } } + @Override + public int hashCode() { + return super.hashCode(); + } + protected class PingTask extends ManagedContextRunnable { @Override protected synchronized void runInContext() { diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 4bbb73014cf5..c98391a654db 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1630,9 +1630,10 @@ public void orchestrateStart(final String vmUuid, final Map> getVolumesToDisconnect(VirtualMachine vm) { return volumesToDisconnect; } - protected boolean sendStop(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final boolean force, final boolean checkBeforeCleanup) { + protected Pair sendStop(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final boolean force, final boolean checkBeforeCleanup) { final VirtualMachine vm = profile.getVirtualMachine(); Map vlanToPersistenceMap = getVlanToPersistenceMapForVM(vm.getId()); StopCommand stpCmd = new StopCommand(vm, getExecuteInSequence(vm.getHypervisorType()), checkBeforeCleanup); @@ -2241,7 +2242,7 @@ protected boolean sendStop(final VirtualMachineGuru guru, final VirtualMachinePr if (!answer.getResult()) { final String details = answer.getDetails(); logger.debug("Unable to stop VM due to {}", details); - return false; + return new Pair<>(false, details); } guru.finalizeStop(profile, answer); @@ -2254,21 +2255,23 @@ protected boolean sendStop(final VirtualMachineGuru guru, final VirtualMachinePr } } } else { - logger.error("Invalid answer received in response to a StopCommand for {}", vm.getInstanceName()); - return false; + String errorMsg = String.format("Invalid answer received in response to a StopCommand for %s", vm.getInstanceName()); + logger.error(errorMsg); + return new Pair<>(false, errorMsg); } } catch (final AgentUnavailableException | OperationTimedoutException e) { - logger.warn("Unable to stop {} due to [{}].", vm.toString(), e.getMessage(), e); + String errorMsg = String.format("Unable to stop %s due to [%s].", vm.toString(), e.getMessage()); + logger.warn(errorMsg, e); if (!force) { - return false; + return new Pair<>(false, errorMsg); } } - return true; + return new Pair<>(true, null); } - protected boolean cleanup(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final ItWorkVO work, final Event event, final boolean cleanUpEvenIfUnableToStop) { + protected Pair cleanup(final VirtualMachineGuru guru, final VirtualMachineProfile profile, final ItWorkVO work, final Event event, final boolean cleanUpEvenIfUnableToStop) { final VirtualMachine vm = profile.getVirtualMachine(); final State state = vm.getState(); logger.debug("Cleaning up resources for the vm {} in {} state", vm, state); @@ -2277,57 +2280,63 @@ protected boolean cleanup(final VirtualMachineGuru guru, final VirtualMachinePro if (work != null) { final Step step = work.getStep(); if (step == Step.Starting && !cleanUpEvenIfUnableToStop) { - logger.warn("Unable to cleanup vm {}; work state is incorrect: {}", vm, step); - return false; + String errorMsg = String.format("Unable to cleanup vm %s; work state is incorrect: %s", vm, step); + logger.warn(errorMsg); + return new Pair<>(false, errorMsg); } if (step == Step.Started || step == Step.Starting || step == Step.Release) { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Starting); - return false; + return result; } } } if (step != Step.Release && step != Step.Prepare && step != Step.Started && step != Step.Starting) { logger.debug("Cleanup is not needed for vm {}; work state is incorrect: {}", vm, step); - return true; + return new Pair<>(true, null); } } else { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Starting); - return false; + return result; } } } } else if (state == State.Stopping) { if (vm.getHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Stopping); - return false; + return result; } } } else if (state == State.Migrating) { if (vm.getHostId() != null || vm.getLastHostId() != null) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Migrating); - return false; + return result; } } } else if (state == State.Running) { - if (!sendStop(guru, profile, cleanUpEvenIfUnableToStop, false)) { + Pair result = sendStop(guru, profile, cleanUpEvenIfUnableToStop, false); + if (!result.first()) { logger.warn("Failed to stop vm {} in {} state as a part of cleanup process", vm, State.Running); - return false; + return result; } } } finally { releaseVmResources(profile, cleanUpEvenIfUnableToStop); } - return true; + return new Pair<>(true, null); } protected void releaseVmResources(final VirtualMachineProfile profile, final boolean forced) { @@ -2509,7 +2518,8 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl logger.warn("Unable to transition the state but we're moving on because it's forced stop", e1); if (doCleanup) { - if (cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.StopRequested, cleanUpEvenIfUnableToStop)) { + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.StopRequested, cleanUpEvenIfUnableToStop); + if (cleanupResult.first()) { try { if (work != null) { logger.debug("Updating work item to Done, id: {}", work.getId()); @@ -2524,7 +2534,8 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl } } else { logger.debug("Failed to cleanup VM: {}", vm); - throw new CloudRuntimeException("Failed to cleanup " + vm + " , current state " + vm.getState()); + String errorDetails = cleanupResult.second() != null ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("Failed to cleanup " + vm + " , current state " + vm.getState() + errorDetails); } } } @@ -2545,6 +2556,7 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl boolean stopped = false; Answer answer = null; + String agentExceptionDetail = null; try { answer = _agentMgr.send(vm.getHostId(), stop); if (answer != null) { @@ -2572,6 +2584,7 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl } } catch (AgentUnavailableException | OperationTimedoutException e) { + agentExceptionDetail = e.getMessage(); logger.warn("Unable to stop {} due to [{}].", profile.toString(), e.toString(), e); } finally { if (!stopped) { @@ -2582,7 +2595,9 @@ private void advanceStop(final VMInstanceVO vm, final boolean cleanUpEvenIfUnabl } catch (final NoTransitionException e) { logger.warn("Unable to transition the state " + vm, e); } - throw new CloudRuntimeException("Unable to stop " + vm); + String errorDetail = (answer != null && answer.getDetails() != null) ? answer.getDetails() : agentExceptionDetail; + String errorDetails = errorDetail != null ? " due to " + errorDetail : ""; + throw new CloudRuntimeException("Unable to stop " + vm + errorDetails); } else { logger.warn("Unable to actually stop {} but continue with release because it's a force stop", vm); vmGuru.finalizeStop(profile, answer); @@ -3261,8 +3276,9 @@ protected void migrate(final VMInstanceVO vm, final long srcHostId, final Deploy } catch (final AgentUnavailableException e) { logger.error("AgentUnavailableException while cleanup on source host: {}", fromHost, e); } - cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); - throw new CloudRuntimeException("Unable to complete migration for " + vm); + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); + String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("Unable to complete migration for " + vm + errorDetails); } } catch (final OperationTimedoutException e) { logger.warn("Error while checking the vm {} on host {}", vm, dest.getHost(), e); @@ -3718,8 +3734,9 @@ private void orchestrateMigrateWithStorage(final String vmUuid, final long srcHo } catch (final AgentUnavailableException e) { logger.error("AgentUnavailableException while cleanup on source host: {}", srcHost, e); } - cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); - throw new CloudRuntimeException("VM not found on destination host. Unable to complete migration for " + vm); + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); + String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("VM not found on destination host. Unable to complete migration for " + vm + errorDetails); } } catch (final OperationTimedoutException e) { logger.error("Error while checking the vm {} is on host {}", vm, destHost, e); @@ -5003,8 +5020,9 @@ private void orchestrateMigrateForScale(final String vmUuid, final long srcHostI } catch (final AgentUnavailableException e) { logger.error("Unable to cleanup source host [{}] due to [{}].", fromHost, e.getMessage(), e); } - cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); - throw new CloudRuntimeException("Unable to complete migration for " + vm); + Pair cleanupResult = cleanup(vmGuru, new VirtualMachineProfileImpl(vm), work, Event.AgentReportStopped, true); + String errorDetails = (cleanupResult.second() != null) ? " due to " + cleanupResult.second() : ""; + throw new CloudRuntimeException("Unable to complete migration for " + vm + errorDetails); } } catch (final OperationTimedoutException e) { logger.debug("Error while checking the {} on {}", vm, dstHost, e); @@ -5465,7 +5483,8 @@ private void handlePowerOffReportWithNoPendingJobsOnVM(final VMInstanceVO vm) { if (PowerState.PowerOff.equals(vm.getPowerState())) { final VirtualMachineGuru vmGuru = getVmGuru(vm); final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); - if (!sendStop(vmGuru, profile, true, true)) { + Pair result = sendStop(vmGuru, profile, true, true); + if (!result.first()) { return; } else { // Release resources on StopCommand success @@ -5938,14 +5957,25 @@ public Outcome removeNicFromVmThroughJobQueue( final VirtualMachine vm, final Nic nic) { Long vmId = vm.getId(); String commandName = VmWorkRemoveNicFromVm.class.getName(); - Pair pendingWorkJob = retrievePendingWorkJob(vmId, commandName); - VmWorkJobVO workJob = pendingWorkJob.first(); + // The nic uuid must be part of the pending-job lookup key. Without it, a concurrent request + // to remove a different nic from the same vm matches this still-pending job and joins it + // instead of submitting its own, so only one nic is removed while both callers wait on the + // single job and both receive its success. Mirrors the symmetric addVmToNetworkThroughJobQueue. + final List pendingWorkJobs = _workJobDao.listPendingWorkJobs( + VirtualMachine.Type.Instance, vmId, commandName, nic.getUuid()); - if (workJob == null) { + VmWorkJobVO workJob; + if (pendingWorkJobs != null && pendingWorkJobs.size() > 0) { + if (pendingWorkJobs.size() > 1) { + throw new CloudRuntimeException(String.format("The number of jobs to remove nic %s from vm %s are %d", nic.getUuid(), vm.getInstanceName(), pendingWorkJobs.size())); + } + workJob = pendingWorkJobs.get(0); + } else { Pair newVmWorkJobAndInfo = createWorkJobAndWorkInfo(commandName, vmId); workJob = newVmWorkJobAndInfo.first(); + workJob.setSecondaryObjectIdentifier(nic.getUuid()); VmWorkRemoveNicFromVm workInfo = new VmWorkRemoveNicFromVm(newVmWorkJobAndInfo.second(), nic.getId()); setCmdInfoAndSubmitAsyncJob(workJob, workInfo, vmId); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 55079173dc98..c6968cddd2f5 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -1767,7 +1767,7 @@ protected boolean reprogramNetworkRules(final long networkId, final Account call final List firewallEgressRulesToApply = _firewallDao.listByNetworkPurposeTrafficType(networkId, Purpose.Firewall, FirewallRule.TrafficType.Egress); final NetworkOfferingVO offering = _networkOfferingDao.findById(network.getNetworkOfferingId()); final DataCenter zone = _dcDao.findById(network.getDataCenterId()); - if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) && _networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) + if (_networkModel.areServicesSupportedInNetwork(network.getId(), Service.Firewall) && (network.getGuestType() == Network.GuestType.Isolated || network.getGuestType() == Network.GuestType.Shared && zone.getNetworkType() == NetworkType.Advanced)) { // add default egress rule to accept the traffic _firewallMgr.applyDefaultEgressFirewallRule(network.getId(), offering.isEgressDefaultPolicy(), true); diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentAttacheSendNextTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentAttacheSendNextTest.java new file mode 100644 index 000000000000..5b504b57133c --- /dev/null +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentAttacheSendNextTest.java @@ -0,0 +1,85 @@ +// 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.manager; + +import org.junit.Assert; +import org.junit.Test; +import org.mockito.Mockito; + +import com.cloud.agent.transport.Request; +import com.cloud.exception.AgentUnavailableException; +import com.cloud.host.Status; +import com.cloud.hypervisor.Hypervisor; + +public class AgentAttacheSendNextTest { + + /** + * Minimal concrete AgentAttache: send() fails for one designated sequence and succeeds otherwise, + * recording the sequence that was actually dispatched. + */ + static class TestAgentAttache extends AgentAttache { + Long sentSeq; + final long failSeq; + + TestAgentAttache(long failSeq) { + super(null, 1L, "uuid-1", "host-1", Hypervisor.HypervisorType.KVM, false); + this.failSeq = failSeq; + } + + @Override + public void send(Request req) throws AgentUnavailableException { + if (req.getSequence() == failSeq) { + throw new AgentUnavailableException("simulated transient link failure", _id); + } + sentSeq = req.getSequence(); + } + + @Override + public void disconnect(Status state) { + } + + @Override + protected boolean isClosed() { + return false; + } + } + + @Test + public void sendNextAdvancesPastAFailedCommandToTheNextQueued() { + long failSeq = 100L; + long goodSeq = 200L; + + Request failing = Mockito.mock(Request.class); + Mockito.when(failing.getSequence()).thenReturn(failSeq); + Request good = Mockito.mock(Request.class); + Mockito.when(good.getSequence()).thenReturn(goodSeq); + + TestAgentAttache attache = new TestAgentAttache(failSeq); + attache._requests.add(failing); + attache._requests.add(good); + + attache.sendNext(1L); + + // A command whose send() failed (and was cancelled) must NOT become _currentSequence: no answer + // will ever arrive for it, so every later in-sequence command to this host would queue behind it + // and time out. sendNext must move on and dispatch the next queued command instead. + Assert.assertEquals("the next queued command should have been dispatched", Long.valueOf(goodSeq), attache.sentSeq); + Assert.assertEquals("current sequence must be the successfully sent command, not the failed one", + Long.valueOf(goodSeq), attache._currentSequence); + Assert.assertTrue("the request queue should be drained", attache._requests.isEmpty()); + } +} diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java index 43d83a672c0f..2377bbaefbc8 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/AgentManagerImplTest.java @@ -18,14 +18,17 @@ import com.cloud.agent.Listener; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.ReadyAnswer; import com.cloud.agent.api.ReadyCommand; import com.cloud.agent.api.StartupCommand; import com.cloud.agent.api.StartupRoutingCommand; import com.cloud.exception.ConnectionException; +import com.cloud.host.DetailVO; import com.cloud.host.Host; import com.cloud.host.HostVO; import com.cloud.host.Status; import com.cloud.host.dao.HostDao; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.utils.Pair; import org.junit.Assert; @@ -34,10 +37,13 @@ import org.mockito.Mockito; import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; public class AgentManagerImplTest { private HostDao hostDao; + private HostDetailsDao hostDetailsDao; private Listener storagePoolMonitor; private AgentAttache attache; private AgentManagerImpl mgr = Mockito.spy(new AgentManagerImpl()); @@ -46,15 +52,18 @@ public class AgentManagerImplTest { @Before public void setUp() throws Exception { - host = new HostVO("some-Uuid"); + host = Mockito.spy(new HostVO("some-Uuid")); + Mockito.when(host.getId()).thenReturn(1L); host.setDataCenterId(1L); cmds = new StartupCommand[]{new StartupRoutingCommand()}; attache = new ConnectedAgentAttache(null, 1L, "uuid", "kvm-attache", Hypervisor.HypervisorType.KVM, null, false); hostDao = Mockito.mock(HostDao.class); + hostDetailsDao = Mockito.mock(HostDetailsDao.class); storagePoolMonitor = Mockito.mock(Listener.class); mgr._hostDao = hostDao; + mgr._hostDetailsDao = hostDetailsDao; mgr._hostMonitors = new ArrayList<>(); mgr._hostMonitors.add(new Pair<>(0, storagePoolMonitor)); } @@ -86,6 +95,32 @@ public void testNotifyMonitorsOfConnectionWhenStoragePoolConnectionHostFailure() Mockito.verify(mgr, Mockito.times(1)).handleDisconnectWithoutInvestigation(Mockito.any(attache.getClass()), Mockito.eq(Status.Event.AgentDisconnected), Mockito.eq(true), Mockito.eq(true)); } + @Test + public void testNotifyMonitorsOfConnectionClearsStaleNvramCapabilityOnReconnect() throws ConnectionException { + DetailVO staleNvramCapability = Mockito.mock(DetailVO.class); + ReadyAnswer readyAnswer = Mockito.mock(ReadyAnswer.class); + host.setDetails(new HashMap<>(Map.of(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString(), + Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString()))); + + Mockito.when(staleNvramCapability.getId()).thenReturn(11L); + Mockito.when(hostDao.findById(Mockito.anyLong())).thenReturn(host); + Mockito.doNothing().when(hostDao).loadDetails(host); + Mockito.when(hostDetailsDao.findDetail(host.getId(), Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(staleNvramCapability); + Mockito.doNothing().when(storagePoolMonitor).processConnect(Mockito.eq(host), Mockito.eq(cmds[0]), Mockito.eq(false)); + Mockito.doReturn(true).when(mgr).handleDisconnectWithoutInvestigation(Mockito.any(attache.getClass()), Mockito.any(Status.Event.class), Mockito.anyBoolean(), Mockito.anyBoolean()); + Mockito.when(readyAnswer.getResult()).thenReturn(true); + Mockito.when(readyAnswer.getDetailsMap()).thenReturn(Map.of(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + Mockito.doReturn(readyAnswer).when(mgr).easySend(Mockito.anyLong(), Mockito.any(ReadyCommand.class)); + Mockito.doReturn(true).when(mgr).agentStatusTransitTo(Mockito.eq(host), Mockito.eq(Status.Event.Ready), Mockito.anyLong()); + + final AgentAttache agentAttache = mgr.notifyMonitorsOfConnection(attache, cmds, false); + + Assert.assertTrue(agentAttache.isReady()); + Assert.assertFalse(host.getDetails().containsKey(Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)); + Mockito.verify(hostDetailsDao).remove(11L); + Mockito.verify(hostDao).saveDetails(host); + } + @Test public void testGetTimeoutWithPositiveTimeout() { Commands commands = Mockito.mock(Commands.class); diff --git a/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java b/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java index 66e6bbae5e2c..72b217d0fdd9 100644 --- a/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java +++ b/engine/orchestration/src/test/java/com/cloud/agent/manager/ConnectedAgentAttacheTest.java @@ -61,12 +61,12 @@ public void testEqualsFalseDiffLink() throws Exception { } @Test - public void testEqualsFalseDiffId() throws Exception { + public void testEqualsFalseDiffUuid() throws Exception { Link link1 = mock(Link.class); - ConnectedAgentAttache agentAttache1 = new ConnectedAgentAttache(null, 1, "uuid", null, Hypervisor.HypervisorType.KVM, link1, false); - ConnectedAgentAttache agentAttache2 = new ConnectedAgentAttache(null, 2, "uuid", null, Hypervisor.HypervisorType.KVM, link1, false); + ConnectedAgentAttache agentAttache1 = new ConnectedAgentAttache(null, 1, "uuid1", null, Hypervisor.HypervisorType.KVM, link1, false); + ConnectedAgentAttache agentAttache2 = new ConnectedAgentAttache(null, 2, "uuid2", null, Hypervisor.HypervisorType.KVM, link1, false); assertFalse(agentAttache1.equals(agentAttache2)); } diff --git a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java index a07870d09af2..c9a404f9c89c 100644 --- a/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java +++ b/engine/orchestration/src/test/java/com/cloud/vm/VirtualMachineManagerImplTest.java @@ -70,6 +70,7 @@ import org.apache.cloudstack.framework.extensions.dao.ExtensionDetailsDao; import org.apache.cloudstack.framework.extensions.manager.ExtensionsManager; import org.apache.cloudstack.framework.extensions.vo.ExtensionDetailsVO; +import org.apache.cloudstack.framework.jobs.AsyncJobExecutionContext; import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao; import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; @@ -394,9 +395,9 @@ public void testSendStopWithOkAnswer() throws Exception { when(vm.getHostId()).thenReturn(1L); when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); - boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); + Pair actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); - Assert.assertTrue(actual); + Assert.assertTrue(actual.first()); } @Test @@ -409,9 +410,10 @@ public void testSendStopWithFailAnswer() throws Exception { when(vm.getHostId()).thenReturn(1L); when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(answer); - boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); + Pair actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); - assertFalse(actual); + assertFalse(actual.first()); + Assert.assertEquals("fail", actual.second()); } @Test @@ -421,11 +423,13 @@ public void testSendStopWithNullAnswer() throws Exception { VirtualMachineProfile profile = mock(VirtualMachineProfile.class); when(profile.getVirtualMachine()).thenReturn(vm); when(vm.getHostId()).thenReturn(1L); + when(vm.getInstanceName()).thenReturn("test-vm"); when(agentManagerMock.send(anyLong(), (Command)any())).thenReturn(null); - boolean actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); + Pair actual = virtualMachineManagerImpl.sendStop(guru, profile, false, false); - assertFalse(actual); + assertFalse(actual.first()); + Assert.assertNotNull(actual.second()); } @Test @@ -1865,6 +1869,97 @@ public void testUnmanagePendingWorkJobs() { virtualMachineManagerImpl.unmanage(vmMockUuid, null); } + /** + * A pending remove-nic job for a different nic on the same vm must not swallow the removal of + * this nic: the pending-job lookup has to be keyed on the nic uuid so that a new job is + * submitted for this nic instead of joining the other nic's job. Regression test for the + * concurrent-removeNic collapse. + */ + @Test + public void removeNicFromVmThroughJobQueueDoesNotJoinAnotherNicsPendingJob() { + String commandName = VmWorkRemoveNicFromVm.class.getName(); + String nicUuid = UUID.randomUUID().toString(); + + Nic nic = mock(Nic.class); + when(nic.getId()).thenReturn(42L); + when(nic.getUuid()).thenReturn(nicUuid); + + // A pending remove-nic job exists for the vm (e.g. for another nic); the nic-agnostic + // lookup would return it, but the nic-keyed lookup finds nothing for this nic. This stub is + // lenient because the fixed code never consults the nic-agnostic 3-arg lookup - only the + // buggy code does, where this job is what it wrongly joins. + Mockito.lenient().when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName)) + .thenReturn(Collections.singletonList(mock(VmWorkJobVO.class))); + when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName, nicUuid)) + .thenReturn(Collections.emptyList()); + + VmWorkJobVO newJob = mock(VmWorkJobVO.class); + when(newJob.getId()).thenReturn(100L); + doReturn(new Pair(newJob, mock(VmWork.class))) + .when(virtualMachineManagerImpl).createWorkJobAndWorkInfo(commandName, vmInstanceVoMockId); + doNothing().when(virtualMachineManagerImpl).setCmdInfoAndSubmitAsyncJob(any(), any(), anyLong()); + + AsyncJobExecutionContext execContext = mock(AsyncJobExecutionContext.class); + try (MockedStatic ignored = Mockito.mockStatic(AsyncJobExecutionContext.class)) { + when(AsyncJobExecutionContext.getCurrentExecutionContext()).thenReturn(execContext); + + virtualMachineManagerImpl.removeNicFromVmThroughJobQueue(vmInstanceMock, nic); + + // A new job must be created for this nic, stamped with the nic uuid, submitted and joined. + verify(virtualMachineManagerImpl, times(1)).createWorkJobAndWorkInfo(commandName, vmInstanceVoMockId); + verify(newJob, times(1)).setSecondaryObjectIdentifier(nicUuid); + verify(virtualMachineManagerImpl, times(1)).setCmdInfoAndSubmitAsyncJob(eq(newJob), any(), eq(vmInstanceVoMockId)); + verify(execContext, times(1)).joinJob(100L); + } + } + + /** + * When a pending remove-nic job already exists for this same nic, the request must join it + * rather than submit a duplicate: per-nic deduplication still holds. + */ + @Test + public void removeNicFromVmThroughJobQueueJoinsExistingJobForSameNic() { + String commandName = VmWorkRemoveNicFromVm.class.getName(); + String nicUuid = UUID.randomUUID().toString(); + + Nic nic = mock(Nic.class); + when(nic.getUuid()).thenReturn(nicUuid); + + VmWorkJobVO existingJob = mock(VmWorkJobVO.class); + when(existingJob.getId()).thenReturn(77L); + when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName, nicUuid)) + .thenReturn(Collections.singletonList(existingJob)); + + AsyncJobExecutionContext execContext = mock(AsyncJobExecutionContext.class); + try (MockedStatic ignored = Mockito.mockStatic(AsyncJobExecutionContext.class)) { + when(AsyncJobExecutionContext.getCurrentExecutionContext()).thenReturn(execContext); + + virtualMachineManagerImpl.removeNicFromVmThroughJobQueue(vmInstanceMock, nic); + + verify(virtualMachineManagerImpl, never()).createWorkJobAndWorkInfo(anyString(), anyLong()); + verify(virtualMachineManagerImpl, never()).setCmdInfoAndSubmitAsyncJob(any(), any(), anyLong()); + verify(execContext, times(1)).joinJob(77L); + } + } + + /** + * More than one pending remove-nic job for the same nic is an inconsistent state and must fail + * fast rather than pick one arbitrarily. Mirrors the guard in addVmToNetworkThroughJobQueue. + */ + @Test(expected = CloudRuntimeException.class) + public void removeNicFromVmThroughJobQueueThrowsWhenMultiplePendingJobsForSameNic() { + String commandName = VmWorkRemoveNicFromVm.class.getName(); + String nicUuid = UUID.randomUUID().toString(); + + Nic nic = mock(Nic.class); + when(nic.getUuid()).thenReturn(nicUuid); + + when(_workJobDao.listPendingWorkJobs(VirtualMachine.Type.Instance, vmInstanceVoMockId, commandName, nicUuid)) + .thenReturn(Arrays.asList(mock(VmWorkJobVO.class), mock(VmWorkJobVO.class))); + + virtualMachineManagerImpl.removeNicFromVmThroughJobQueue(vmInstanceMock, nic); + } + @Test public void testUnmanageHostNotFoundAfterTransaction() { when(vmInstanceMock.getHostId()).thenReturn(hostMockId); diff --git a/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java b/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java index c50451b03e4a..03716237cdb0 100644 --- a/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java +++ b/engine/schema/src/main/java/com/cloud/event/dao/EventDao.java @@ -19,6 +19,7 @@ import java.util.Date; import java.util.List; +import com.cloud.event.Event; import com.cloud.event.EventVO; import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDao; @@ -31,6 +32,18 @@ public interface EventDao extends GenericDao { EventVO findCompletedEvent(long startId); + /** + * Finds the last non-archived start event matching the specified criteria. + * Events are ordered by ID in descending order, returning the most recent one. + * + * @param type the event type to search for + * @param state the event state to search for (e.g., {@link Event.State#Scheduled}) + * @param resourceId the resource ID associated with the event + * @param resourceType the resource type associated with the event + * @return the most recent EventVO matching the criteria, or null if not found + */ + EventVO findLastEvent(String type, Event.State state, Long resourceId, String resourceType); + public List listToArchiveOrDeleteEvents(List ids, String type, Date startDate, Date endDate, List accountIds); public void archiveEvents(List events); diff --git a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java index e748e98900eb..b66da14292ef 100644 --- a/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/event/dao/EventDaoImpl.java @@ -19,7 +19,6 @@ import java.util.Date; import java.util.List; - import org.springframework.stereotype.Component; import com.cloud.event.Event.State; @@ -35,6 +34,7 @@ public class EventDaoImpl extends GenericDaoBase implements EventDao { protected final SearchBuilder CompletedEventSearch; protected final SearchBuilder ToArchiveOrDeleteEventSearch; + protected final SearchBuilder LastStartEventSearch; public EventDaoImpl() { CompletedEventSearch = createSearchBuilder(); @@ -51,6 +51,14 @@ public EventDaoImpl() { ToArchiveOrDeleteEventSearch.and("createdDateL", ToArchiveOrDeleteEventSearch.entity().getCreateDate(), Op.LTEQ); ToArchiveOrDeleteEventSearch.and("archived", ToArchiveOrDeleteEventSearch.entity().getArchived(), Op.EQ); ToArchiveOrDeleteEventSearch.done(); + + LastStartEventSearch = createSearchBuilder(); + LastStartEventSearch.and("type", LastStartEventSearch.entity().getType(), Op.EQ); + LastStartEventSearch.and("state", LastStartEventSearch.entity().getState(), Op.EQ); + LastStartEventSearch.and("resourceId", LastStartEventSearch.entity().getResourceId(), Op.EQ); + LastStartEventSearch.and("resourceType", LastStartEventSearch.entity().getResourceType(), Op.EQ); + LastStartEventSearch.and("archived", LastStartEventSearch.entity().getArchived(), Op.EQ); + LastStartEventSearch.done(); } @Override @@ -77,6 +85,17 @@ public EventVO findCompletedEvent(long startId) { return findOneIncludingRemovedBy(sc); } + @Override + public EventVO findLastEvent(String type, State state, Long resourceId, String resourceType) { + SearchCriteria sc = LastStartEventSearch.create(); + sc.setParameters("type", type); + sc.setParameters("state", state); + sc.setParameters("resourceId", resourceId); + sc.setParameters("resourceType", resourceType); + sc.setParameters("archived", false); + return findLastOneBy(sc); + } + @Override public List listToArchiveOrDeleteEvents(List ids, String type, Date startDate, Date endDate, List accountIds) { SearchCriteria sc = ToArchiveOrDeleteEventSearch.create(); diff --git a/engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java b/engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java index 904c8e646eb5..fc22e68985d2 100644 --- a/engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java +++ b/engine/schema/src/main/java/com/cloud/offerings/NetworkOfferingVO.java @@ -436,7 +436,7 @@ public NetworkOfferingVO(String name, Network.GuestType guestType, boolean speci true, Availability.Optional, null, - Network.GuestType.Isolated, + guestType, true, false, false, diff --git a/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java b/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java index 89b71f03289b..3e9355c76036 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java @@ -324,38 +324,33 @@ public void setUpdated(Date updated) { public static final Map NewTemplateMap = new HashMap<>(); - public static final Map RouterTemplateConfigurationNames = new HashMap<>() { - { - put(Hypervisor.HypervisorType.KVM, "router.template.kvm"); - put(Hypervisor.HypervisorType.VMware, "router.template.vmware"); - put(Hypervisor.HypervisorType.XenServer, "router.template.xenserver"); - put(Hypervisor.HypervisorType.Hyperv, "router.template.hyperv"); - put(Hypervisor.HypervisorType.LXC, "router.template.lxc"); - put(Hypervisor.HypervisorType.Ovm3, "router.template.ovm3"); - } - }; - - public static Map hypervisorGuestOsMap = new HashMap<>() { - { - put(Hypervisor.HypervisorType.KVM, LINUX_12_ID); - put(Hypervisor.HypervisorType.XenServer, OTHER_LINUX_ID); - put(Hypervisor.HypervisorType.VMware, OTHER_LINUX_ID); - put(Hypervisor.HypervisorType.Hyperv, LINUX_12_ID); - put(Hypervisor.HypervisorType.LXC, LINUX_12_ID); - put(Hypervisor.HypervisorType.Ovm3, LINUX_12_ID); - } - }; - - public static final Map hypervisorImageFormat = new HashMap() { - { - put(Hypervisor.HypervisorType.KVM, ImageFormat.QCOW2); - put(Hypervisor.HypervisorType.XenServer, ImageFormat.VHD); - put(Hypervisor.HypervisorType.VMware, ImageFormat.OVA); - put(Hypervisor.HypervisorType.Hyperv, ImageFormat.VHD); - put(Hypervisor.HypervisorType.LXC, ImageFormat.QCOW2); - put(Hypervisor.HypervisorType.Ovm3, ImageFormat.RAW); - } - }; + public static final Map RouterTemplateConfigurationNames = Map.of( + Hypervisor.HypervisorType.KVM, "router.template.kvm", + Hypervisor.HypervisorType.VMware, "router.template.vmware", + Hypervisor.HypervisorType.XenServer, "router.template.xenserver", + Hypervisor.HypervisorType.Hyperv, "router.template.hyperv", + Hypervisor.HypervisorType.LXC, "router.template.lxc", + Hypervisor.HypervisorType.Ovm3, "router.template.ovm3" + ); + + public static Map hypervisorGuestOsMap = new HashMap<>(); + static { + hypervisorGuestOsMap.put(Hypervisor.HypervisorType.KVM, LINUX_12_ID); + hypervisorGuestOsMap.put(Hypervisor.HypervisorType.XenServer, OTHER_LINUX_ID); + hypervisorGuestOsMap.put(Hypervisor.HypervisorType.VMware, OTHER_LINUX_ID); + hypervisorGuestOsMap.put(Hypervisor.HypervisorType.Hyperv, LINUX_12_ID); + hypervisorGuestOsMap.put(Hypervisor.HypervisorType.LXC, LINUX_12_ID); + hypervisorGuestOsMap.put(Hypervisor.HypervisorType.Ovm3, LINUX_12_ID); + } + + public static final Map hypervisorImageFormat = Map.of( + Hypervisor.HypervisorType.KVM, ImageFormat.QCOW2, + Hypervisor.HypervisorType.XenServer, ImageFormat.VHD, + Hypervisor.HypervisorType.VMware, ImageFormat.OVA, + Hypervisor.HypervisorType.Hyperv, ImageFormat.VHD, + Hypervisor.HypervisorType.LXC, ImageFormat.QCOW2, + Hypervisor.HypervisorType.Ovm3, ImageFormat.RAW + ); public boolean validateIfSeeded(TemplateDataStoreVO templDataStoreVO, String url, String path, String nfsVersion) { String filePath = null; diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java index 223d7a466376..f5775a56097e 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/DatabaseAccessObject.java @@ -128,7 +128,7 @@ public boolean indexExists(Connection conn, String tableName, String indexName) return true; } } catch (SQLException e) { - logger.debug(String.format("Index %s doesn't exist, ignoring exception:", indexName, e.getMessage())); + logger.debug("Index {} doesn't exist, ignoring exception: {}", indexName, e.getMessage()); } return false; } diff --git a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41500to41510.java b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41500to41510.java index c7295414326d..18d419d29f98 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41500to41510.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade41500to41510.java @@ -22,7 +22,6 @@ import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLException; -import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; @@ -98,49 +97,41 @@ public void updateSystemVmTemplates(final Connection conn) { throw new CloudRuntimeException("updateSystemVmTemplates:Exception while getting hypervisor types from clusters", e); } - final Map NewTemplateNameList = new HashMap() { - { - put(KVM, "systemvm-kvm-4.15.1"); - put(VMware, "systemvm-vmware-4.15.1"); - put(XenServer, "systemvm-xenserver-4.15.1"); - put(Hyperv, "systemvm-hyperv-4.15.1"); - put(LXC, "systemvm-lxc-4.15.1"); - put(Ovm3, "systemvm-ovm3-4.15.1"); - } - }; - - final Map routerTemplateConfigurationNames = new HashMap() { - { - put(KVM, "router.template.kvm"); - put(VMware, "router.template.vmware"); - put(XenServer, "router.template.xenserver"); - put(Hyperv, "router.template.hyperv"); - put(LXC, "router.template.lxc"); - put(Ovm3, "router.template.ovm3"); - } - }; - - final Map newTemplateUrl = new HashMap() { - { - put(KVM, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-kvm.qcow2.bz2"); - put(VMware, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-vmware.ova"); - put(XenServer, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-xen.vhd.bz2"); - put(Hyperv, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-hyperv.vhd.zip"); - put(LXC, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-kvm.qcow2.bz2"); - put(Ovm3, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-ovm.raw.bz2"); - } - }; - - final Map newTemplateChecksum = new HashMap() { - { - put(KVM, "0e9f9a7d0957c3e0a2088e41b2da2cec"); - put(XenServer, "86373992740b1eca8aff8b08ebf3aea5"); - put(VMware, "4006982765846d373eb3719b2fe4d720"); - put(Hyperv, "0b9514e4b6cba1f636fea2125f0f7a5f"); - put(LXC, "0e9f9a7d0957c3e0a2088e41b2da2cec"); - put(Ovm3, "ae3977e696b3e6c81bdcbb792d514d29"); - } - }; + final Map NewTemplateNameList = Map.of( + KVM, "systemvm-kvm-4.15.1", + VMware, "systemvm-vmware-4.15.1", + XenServer, "systemvm-xenserver-4.15.1", + Hyperv, "systemvm-hyperv-4.15.1", + LXC, "systemvm-lxc-4.15.1", + Ovm3, "systemvm-ovm3-4.15.1" + ); + + final Map routerTemplateConfigurationNames = Map.of( + KVM, "router.template.kvm", + VMware, "router.template.vmware", + XenServer, "router.template.xenserver", + Hyperv, "router.template.hyperv", + LXC, "router.template.lxc", + Ovm3, "router.template.ovm3" + ); + + final Map newTemplateUrl = Map.of( + KVM, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-kvm.qcow2.bz2", + VMware, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-vmware.ova", + XenServer, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-xen.vhd.bz2", + Hyperv, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-hyperv.vhd.zip", + LXC, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-kvm.qcow2.bz2", + Ovm3, "https://download.cloudstack.org/systemvm/4.15/systemvmtemplate-4.15.1-ovm.raw.bz2" + ); + + final Map newTemplateChecksum = Map.of( + KVM, "0e9f9a7d0957c3e0a2088e41b2da2cec", + XenServer, "86373992740b1eca8aff8b08ebf3aea5", + VMware, "4006982765846d373eb3719b2fe4d720", + Hyperv, "0b9514e4b6cba1f636fea2125f0f7a5f", + LXC, "0e9f9a7d0957c3e0a2088e41b2da2cec", + Ovm3, "ae3977e696b3e6c81bdcbb792d514d29" + ); for (final Map.Entry hypervisorAndTemplateName : NewTemplateNameList.entrySet()) { logger.debug("Updating " + hypervisorAndTemplateName.getKey() + " System Vms"); diff --git a/engine/schema/src/test/java/com/cloud/network/as/AutoScaleVmProfileVOTest.java b/engine/schema/src/test/java/com/cloud/network/as/AutoScaleVmProfileVOTest.java index 6813a2091576..4843a97f6d5d 100755 --- a/engine/schema/src/test/java/com/cloud/network/as/AutoScaleVmProfileVOTest.java +++ b/engine/schema/src/test/java/com/cloud/network/as/AutoScaleVmProfileVOTest.java @@ -44,8 +44,15 @@ public void testCounterParamsForUpdate() { AutoScaleVmProfileVO profile = new AutoScaleVmProfileVO(); Map> counterParamList = new LinkedHashMap<>(); - counterParamList.put("0", new LinkedHashMap<>() {{ put("name", "snmpcommunity"); put("value", "public"); }}); - counterParamList.put("1", new LinkedHashMap<>() {{ put("name", "snmpport"); put("value", "161"); }}); + LinkedHashMap param0 = new LinkedHashMap<>(); + param0.put("name", "snmpcommunity"); + param0.put("value", "public"); + counterParamList.put("0", param0); + + LinkedHashMap param1 = new LinkedHashMap<>(); + param1.put("name", "snmpport"); + param1.put("value", "161"); + counterParamList.put("1", param1); profile.setCounterParamsForUpdate(counterParamList); Assert.assertEquals("snmpcommunity=public&snmpport=161", profile.getCounterParamsString()); @@ -63,8 +70,15 @@ public void tstSetOtherDeployParamsForUpdate() { AutoScaleVmProfileVO profile = new AutoScaleVmProfileVO(); Map> otherDeployParamsMap = new HashMap<>(); - otherDeployParamsMap.put("0", new HashMap<>() {{ put("name", "serviceofferingid"); put("value", "a7fb50f6-01d9-11ed-8bc1-77f8f0228926"); }}); - otherDeployParamsMap.put("1", new HashMap<>() {{ put("name", "rootdisksize"); put("value", "10"); }}); + HashMap deployParam0 = new HashMap<>(); + deployParam0.put("name", "serviceofferingid"); + deployParam0.put("value", "a7fb50f6-01d9-11ed-8bc1-77f8f0228926"); + otherDeployParamsMap.put("0", deployParam0); + + HashMap deployParam1 = new HashMap<>(); + deployParam1.put("name", "rootdisksize"); + deployParam1.put("value", "10"); + otherDeployParamsMap.put("1", deployParam1); profile.setOtherDeployParamsForUpdate(otherDeployParamsMap); diff --git a/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java b/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java index 93be850f558e..7a5e1505c1bb 100644 --- a/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java +++ b/engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java @@ -363,6 +363,7 @@ public void testValidateTemplates_fileFailure() { systemVmTemplateRegistration.validateTemplates(list); } + @Test public void testValidateTemplates_downloadableFileNotFound() { CPU.CPUArch arch = SystemVmTemplateRegistration.DOWNLOADABLE_TEMPLATE_ARCH_TYPES.get(0); List> list = new ArrayList<>(); 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 003065e394f5..79e2a9722039 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 @@ -28,9 +28,14 @@ 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.configuration.Resource; import com.cloud.event.EventTypes; import com.cloud.event.UsageEventUtils; +import com.cloud.host.DetailVO; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDetailsDao; import com.cloud.hypervisor.Hypervisor; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Snapshot; @@ -46,9 +51,11 @@ import com.cloud.utils.fsm.NoTransitionException; import com.cloud.vm.UserVmVO; import com.cloud.vm.VirtualMachine; +import com.cloud.vm.dao.VMInstanceDetailsDao; import com.cloud.vm.snapshot.VMSnapshot; import com.cloud.vm.snapshot.VMSnapshotDetailsVO; import com.cloud.vm.snapshot.VMSnapshotVO; +import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.backup.BackupOfferingVO; import org.apache.cloudstack.backup.dao.BackupOfferingDao; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; @@ -59,10 +66,12 @@ import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.snapshot.SnapshotObject; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; import org.apache.commons.collections.CollectionUtils; +import org.apache.commons.lang3.StringUtils; import javax.inject.Inject; import java.util.ArrayList; @@ -76,6 +85,7 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStrategy { private static final List supportedStoragePoolTypes = List.of(Storage.StoragePoolType.Filesystem, Storage.StoragePoolType.NetworkFilesystem, Storage.StoragePoolType.SharedMountPoint); + private static final String KVM_FILE_BASED_STORAGE_SNAPSHOT_NVRAM = "kvmFileBasedStorageSnapshotNvram"; @Inject protected SnapshotDataStoreDao snapshotDataStoreDao; @@ -86,6 +96,15 @@ public class KvmFileBasedStorageVmSnapshotStrategy extends StorageVMSnapshotStra @Inject protected BackupOfferingDao backupOfferingDao; + @Inject + protected VMInstanceDetailsDao vmInstanceDetailsDao; + + @Inject + protected HostDetailsDao hostDetailsDao; + + @Inject + protected AlertManager alertManager; + @Override public VMSnapshot takeVMSnapshot(VMSnapshot vmSnapshot) { Map volumeInfoToSnapshotObjectMap = new HashMap<>(); @@ -116,7 +135,8 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { logger.info("Starting VM snapshot delete process for snapshot [{}].", vmSnapshot.getUuid()); UserVmVO userVm = userVmDao.findById(vmSnapshot.getVmId()); VMSnapshotVO vmSnapshotBeingDeleted = (VMSnapshotVO) vmSnapshot; - Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingDeleted.getVmId()); + Long hostId = pickHostForNvramSidecarCleanup(vmSnapshotBeingDeleted, userVm, "delete"); + validateHostSupportsNvramSidecarCleanup(vmSnapshotBeingDeleted, hostId, "delete"); long virtualSize = 0; boolean isCurrent = vmSnapshotBeingDeleted.getCurrent(); @@ -124,6 +144,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { List volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshotBeingDeleted.getVmId()); List snapshotChildren = vmSnapshotDao.listByParentAndStateIn(vmSnapshotBeingDeleted.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden); + PrimaryDataStoreTO nvramPrimaryDataStore = getPrimaryDataStoreForNvramCleanup(vmSnapshotBeingDeleted, volumeTOs); long realSize = getVMSnapshotRealSize(vmSnapshotBeingDeleted); int numberOfChildren = snapshotChildren.size(); @@ -157,6 +178,8 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) { return true; } + deleteNvramSnapshotIfNeeded(vmSnapshotBeingDeleted, hostId, nvramPrimaryDataStore); + transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.OperationSucceeded); vmSnapshotDetailsDao.removeDetails(vmSnapshotBeingDeleted.getId()); @@ -175,7 +198,8 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { } VMSnapshotVO vmSnapshotBeingReverted = (VMSnapshotVO) vmSnapshot; - Long hostId = vmSnapshotHelper.pickRunningHost(vmSnapshotBeingReverted.getVmId()); + Long hostId = pickHostForUefiNvramAwareDiskOnlySnapshot(userVm, "revert"); + validateHostSupportsUefiNvramAwareDiskOnlySnapshots(hostId, userVm, "revert"); transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.RevertRequested); @@ -184,7 +208,9 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { .map(snapshot -> (SnapshotObjectTO) snapshotDataFactory.getSnapshot(snapshot.getSnapshotId(), snapshot.getDataStoreId(), DataStoreRole.Primary).getTO()) .collect(Collectors.toList()); - RevertDiskOnlyVmSnapshotCommand revertDiskOnlyVMSnapshotCommand = new RevertDiskOnlyVmSnapshotCommand(volumeSnapshotTos, userVm.getName()); + RevertDiskOnlyVmSnapshotCommand revertDiskOnlyVMSnapshotCommand = + new RevertDiskOnlyVmSnapshotCommand(volumeSnapshotTos, userVm.getName(), userVm.getUuid(), isUefiVm(userVm), + getNvramSnapshotPath(vmSnapshotBeingReverted)); Answer answer = agentMgr.easySend(hostId, revertDiskOnlyVMSnapshotCommand); if (answer == null || !answer.getResult()) { @@ -204,6 +230,13 @@ public boolean revertVMSnapshot(VMSnapshot vmSnapshot) { publishUsageEvent(EventTypes.EVENT_VM_SNAPSHOT_REVERT, vmSnapshotBeingReverted, userVm, volumeObjectTo); } + if (isUefiVm(userVm) && !Objects.equals(userVm.getLastHostId(), hostId)) { + logger.debug("Updating last host of UEFI VM [{}] to [{}] after disk-only snapshot revert because the NVRAM state was restored on that host.", + userVm.getUuid(), hostId); + userVm.setLastHostId(hostId); + userVmDao.update(userVm.getId(), userVm); + } + transitStateWithoutThrow(vmSnapshotBeingReverted, VMSnapshot.Event.OperationSucceeded); VMSnapshotVO currentVmSnapshot = vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId()); @@ -248,6 +281,8 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen return; } + validateHostSupportsNvramSidecarCleanup(oldParent, hostId, "clean up"); + PrimaryDataStoreTO nvramPrimaryDataStore = getPrimaryDataStoreForNvramCleanup(oldParent, volumeTOs); List snapshotVos; if (oldParent.getCurrent()) { @@ -276,6 +311,8 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen snapshotDao.update(snapshotVO.getId(), snapshotVO); } + deleteNvramSnapshotIfNeeded(oldParent, hostId, nvramPrimaryDataStore); + vmSnapshotDetailsDao.removeDetails(oldParent.getId()); oldParent.setRemoved(DateUtil.now()); @@ -347,12 +384,13 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe } private List deleteSnapshot(VMSnapshotVO vmSnapshotVO, Long hostId) { + validateHostSupportsNvramSidecarCleanup(vmSnapshotVO, hostId, "delete"); List volumeSnapshots = getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshotVO); List volumeSnapshotTOList = volumeSnapshots.stream() .map(snapshotDataStoreVO -> snapshotDataFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), snapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO()) .collect(Collectors.toList()); - DeleteDiskOnlyVmSnapshotCommand deleteSnapshotCommand = new DeleteDiskOnlyVmSnapshotCommand(volumeSnapshotTOList); + DeleteDiskOnlyVmSnapshotCommand deleteSnapshotCommand = new DeleteDiskOnlyVmSnapshotCommand(volumeSnapshotTOList, getNvramSnapshotPath(vmSnapshotVO)); Answer answer = agentMgr.easySend(hostId, deleteSnapshotCommand); if (answer == null || !answer.getResult()) { logger.error("Failed to delete VM snapshot [{}] due to {}.", vmSnapshotVO.getUuid(), answer != null ? answer.getDetails() : "Communication failure"); @@ -368,6 +406,20 @@ private List deleteSnapshot(VMSnapshotVO vmSnapshotVO, Long hostId) return snapshotVOList; } + protected void deleteNvramSnapshotIfNeeded(VMSnapshotVO vmSnapshotVO, Long hostId, PrimaryDataStoreTO primaryDataStore) { + String nvramSnapshotPath = getNvramSnapshotPath(vmSnapshotVO); + if (StringUtils.isBlank(nvramSnapshotPath) || primaryDataStore == null) { + return; + } + + DeleteDiskOnlyVmSnapshotCommand deleteSnapshotCommand = new DeleteDiskOnlyVmSnapshotCommand(List.of(), nvramSnapshotPath, primaryDataStore); + Answer answer = agentMgr.easySend(hostId, deleteSnapshotCommand); + if (answer == null || !answer.getResult()) { + logger.warn("Failed to delete the NVRAM sidecar of VM snapshot [{}] due to {}.", vmSnapshotVO.getUuid(), + answer != null ? answer.getDetails() : "communication failure"); + } + } + private List mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO childSnapshot, UserVmVO userVm, List volumeObjectTOS, Long hostId) { logger.debug("Merging VM snapshot [{}] with its child [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid()); @@ -470,12 +522,13 @@ protected VMSnapshot takeVmSnapshotInternal(VMSnapshot vmSnapshot, Map volumeTOs = vmSnapshotHelper.getVolumeTOList(userVm.getId()); - transitStateWithoutThrow(vmSnapshot, VMSnapshot.Event.CreateRequested); - VMSnapshotTO parentSnapshotTo = null; VMSnapshotVO parentSnapshotVo = vmSnapshotDao.findCurrentSnapshotByVmId(userVm.getId()); if (parentSnapshotVo != null) { @@ -493,14 +546,18 @@ protected VMSnapshot takeVmSnapshotInternal(VMSnapshot vmSnapshot, Map volumeTOs) { + return (PrimaryDataStoreTO) volumeTOs.stream() + .filter(volumeObjectTO -> Volume.Type.ROOT.equals(volumeObjectTO.getVolumeType())) + .findFirst() + .orElseThrow(() -> new CloudRuntimeException("Failed to locate the root volume while handling the VM snapshot.")) + .getDataStore(); + } + + protected PrimaryDataStoreTO getRootVolumePrimaryDataStoreForCleanup(VMSnapshotVO vmSnapshot, List volumeTOs) { + try { + return getRootVolumePrimaryDataStore(volumeTOs); + } catch (CloudRuntimeException e) { + logger.warn("Failed to locate the root volume while cleaning up the NVRAM sidecar for VM snapshot [{}].", vmSnapshot.getUuid(), e); + return null; + } + } + + protected PrimaryDataStoreTO getPrimaryDataStoreForNvramCleanup(VMSnapshotVO vmSnapshot, List volumeTOs) { + PrimaryDataStoreTO rootSnapshotPrimaryDataStore = getRootSnapshotPrimaryDataStoreForCleanup(vmSnapshot); + return rootSnapshotPrimaryDataStore != null ? rootSnapshotPrimaryDataStore : getRootVolumePrimaryDataStoreForCleanup(vmSnapshot, volumeTOs); + } + + protected PrimaryDataStoreTO getRootSnapshotPrimaryDataStoreForCleanup(VMSnapshotVO vmSnapshot) { + try { + return (PrimaryDataStoreTO) getVolumeSnapshotsAssociatedWithVmSnapshot(vmSnapshot).stream() + .map(snapshotDataStoreVO -> (SnapshotObjectTO) snapshotDataFactory.getSnapshot(snapshotDataStoreVO.getSnapshotId(), + snapshotDataStoreVO.getDataStoreId(), DataStoreRole.Primary).getTO()) + .filter(snapshotObjectTO -> Volume.Type.ROOT.equals(snapshotObjectTO.getVolume().getVolumeType())) + .findFirst() + .orElseThrow(() -> new CloudRuntimeException("Failed to locate the root volume snapshot while handling the VM snapshot.")) + .getDataStore(); + } catch (CloudRuntimeException e) { + logger.warn("Failed to locate the root volume snapshot while cleaning up the NVRAM sidecar for VM snapshot [{}].", vmSnapshot.getUuid(), e); + return null; + } + } + + protected String getNvramSnapshotPath(VMSnapshotVO vmSnapshot) { + VMSnapshotDetailsVO nvramDetail = vmSnapshotDetailsDao.findDetail(vmSnapshot.getId(), KVM_FILE_BASED_STORAGE_SNAPSHOT_NVRAM); + return nvramDetail != null ? nvramDetail.getValue() : null; + } + + protected Long pickHostForUefiNvramAwareDiskOnlySnapshot(UserVm userVm, String operation) { + Long selectedHostId = vmSnapshotHelper.pickRunningHost(userVm.getId()); + if (!isUefiVm(userVm)) { + return selectedHostId; + } + + boolean isCreate = "create".equals(operation); + if (isCreate) { + validateUefiSnapshotCreateHostOwnsActiveNvram(userVm, selectedHostId); + } + + return pickHostWithRequiredCapabilities(userVm, selectedHostId, operation, !isCreate, + List.of(Host.HOST_UEFI_ENABLE, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)); + } + + protected void validateUefiSnapshotCreateHostOwnsActiveNvram(UserVm userVm, Long selectedHostId) { + if (VirtualMachine.State.Running.equals(userVm.getState())) { + return; + } + + Long lastHostId = userVm.getLastHostId(); + if (lastHostId == null || !Objects.equals(lastHostId, selectedHostId)) { + throw new CloudRuntimeException(String.format("Cannot create a disk-only snapshot for stopped UEFI VM [%s] on host [%s] because the active NVRAM " + + "state is expected on last host [%s]. Make the last host available or start the VM on a UEFI-capable KVM host before retrying.", + userVm.getUuid(), selectedHostId, lastHostId)); + } + } + + protected Long pickHostForNvramSidecarCleanup(VMSnapshotVO vmSnapshotVO, UserVm userVm, String operation) { + Long selectedHostId = vmSnapshotHelper.pickRunningHost(vmSnapshotVO.getVmId()); + if (StringUtils.isBlank(getNvramSnapshotPath(vmSnapshotVO))) { + return selectedHostId; + } + + return pickHostWithRequiredCapabilities(userVm, selectedHostId, operation, true, List.of(Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)); + } + + protected Long pickHostWithRequiredCapabilities(UserVm userVm, Long selectedHostId, String operation, boolean canFallbackFromSelectedHost, + List requiredCapabilities) { + if (hostSupportsCapabilities(selectedHostId, requiredCapabilities)) { + return selectedHostId; + } + + if (VirtualMachine.State.Running.equals(userVm.getState()) || !canFallbackFromSelectedHost) { + return selectedHostId; + } + + return listCandidateHostsForVmSnapshot(userVm).stream() + .filter(host -> hostSupportsCapabilities(host.getId(), requiredCapabilities)) + .findFirst() + .map(HostVO::getId) + .orElseThrow(() -> new CloudRuntimeException(String.format("Cannot %s disk-only snapshot state for VM [%s] because no Up and Enabled host in the " + + "VM storage scope advertises [%s].", operation, userVm.getUuid(), String.join(", ", requiredCapabilities)))); + } + + protected List listCandidateHostsForVmSnapshot(UserVm userVm) { + List volumes = volumeDao.findByInstance(userVm.getId()); + if (CollectionUtils.isEmpty(volumes)) { + throw new CloudRuntimeException(String.format("Cannot find a host for VM snapshot operation because VM [%s] has no volumes.", userVm.getUuid())); + } + + VolumeVO volume = volumes.stream() + .filter(volumeVO -> Volume.Type.ROOT.equals(volumeVO.getVolumeType())) + .findFirst() + .orElse(volumes.get(0)); + Long poolId = volume.getPoolId(); + if (poolId == null) { + throw new CloudRuntimeException(String.format("Cannot find a host for VM snapshot operation because volume [%s] has no pool.", volume.getUuid())); + } + + StoragePoolVO storagePoolVO = storagePool.findById(poolId); + if (storagePoolVO == null) { + throw new CloudRuntimeException(String.format("Cannot find a host for VM snapshot operation because storage pool [%s] was not found.", poolId)); + } + + List hosts = hostDao.listAllUpAndEnabledNonHAHosts(Host.Type.Routing, storagePoolVO.getClusterId(), storagePoolVO.getPodId(), + storagePoolVO.getDataCenterId(), null); + if (CollectionUtils.isEmpty(hosts)) { + throw new CloudRuntimeException(String.format("Cannot find a host for VM snapshot operation because no Up and Enabled host was found in storage pool [%s] scope.", + storagePoolVO.getUuid())); + } + return hosts; + } + + protected boolean hostSupportsCapabilities(Long hostId, List requiredCapabilities) { + if (hostId == null || CollectionUtils.isEmpty(requiredCapabilities)) { + return false; + } + return requiredCapabilities.stream().allMatch(capability -> isHostCapabilityEnabled(hostId, capability)); + } + + protected void validateHostSupportsUefiNvramAwareDiskOnlySnapshots(Long hostId, UserVm userVm, String operation) { + if (!isUefiVm(userVm)) { + return; + } + + if (!isHostCapabilityEnabled(hostId, Host.HOST_UEFI_ENABLE)) { + throw new CloudRuntimeException(String.format("Cannot %s a disk-only snapshot for UEFI VM [%s] on host [%s] because the host does not advertise " + + "UEFI support. Ensure the host is configured with UEFI support and retry.", operation, userVm.getUuid(), hostId)); + } + + if (!isHostCapabilityEnabled(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) { + throw new CloudRuntimeException(String.format("Cannot %s a disk-only snapshot for UEFI VM [%s] on host [%s] because the KVM agent does not advertise " + + "NVRAM-aware disk-only snapshot support. Upgrade the host and retry.", operation, userVm.getUuid(), hostId)); + } + } + + protected boolean isHostCapabilityEnabled(Long hostId, String capabilityName) { + DetailVO hostCapability = hostDetailsDao.findDetail(hostId, capabilityName); + return hostCapability != null && Boolean.parseBoolean(hostCapability.getValue()); + } + + protected void validateHostSupportsNvramSidecarCleanup(VMSnapshotVO vmSnapshotVO, Long hostId, String operation) { + if (StringUtils.isBlank(getNvramSnapshotPath(vmSnapshotVO))) { + return; + } + + if (!isHostCapabilityEnabled(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) { + throw new CloudRuntimeException(String.format("Cannot %s VM snapshot [%s] on host [%s] because the KVM agent does not advertise " + + "NVRAM-aware disk-only snapshot support and the snapshot has an NVRAM sidecar that must be cleaned up. Upgrade the host and retry.", + operation, vmSnapshotVO.getUuid(), hostId)); + } + } + + protected void notifyGuestRecoveryIssueIfNeeded(CreateDiskOnlyVmSnapshotAnswer answer, UserVm userVm, VMSnapshotVO vmSnapshot) { + if (StringUtils.isBlank(answer.getDetails())) { + return; + } + + String subject = String.format("Disk-only VM snapshot [%s] completed with guest recovery warnings", vmSnapshot.getUuid()); + String message = String.format("Disk-only VM snapshot [%s] for UEFI VM [%s] completed, but post-snapshot guest recovery reported: %s", + vmSnapshot.getUuid(), userVm.getUuid(), answer.getDetails()); + logger.error(message); + try { + alertManager.sendAlert(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT, userVm.getDataCenterId(), userVm.getPodIdToDeployIn(), subject, message); + } catch (Exception e) { + logger.warn("Failed to send post-snapshot guest recovery alert for VM snapshot [{}].", vmSnapshot.getUuid(), e); + } + } + /** * Given a list of VM snapshots, will remove any that are part of the current direct backing chain (all the direct ancestors of the current vm snapshot). * This is done because, when using virDomainBlockCommit}, Libvirt will maintain diff --git a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java index aced750bd320..a18d65748cf1 100644 --- a/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java +++ b/engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/ScaleIOVMSnapshotStrategy.java @@ -141,7 +141,7 @@ public StrategyPriority canHandle(Long vmId, Long rootPoolId, boolean snapshotMe for (VolumeObjectTO volumeTO : volumeTOs) { Long poolId = volumeTO.getPoolId(); Storage.StoragePoolType poolType = vmSnapshotHelper.getStoragePoolType(poolId); - if (poolType != Storage.StoragePoolType.PowerFlex || volumeTO.getFormat() != ImageFormat.RAW || poolId != rootPoolId) { + if (poolType != Storage.StoragePoolType.PowerFlex || volumeTO.getFormat() != ImageFormat.RAW || !poolId.equals(rootPoolId)) { return StrategyPriority.CANT_HANDLE; } } 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 new file mode 100644 index 000000000000..35b1f9ff65bf --- /dev/null +++ b/engine/storage/snapshot/src/test/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategyTest.java @@ -0,0 +1,550 @@ +/* + * 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 org.apache.cloudstack.storage.vmsnapshot; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.mockito.ArgumentMatchers.any; +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.doNothing; +import static org.mockito.Mockito.inOrder; +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.lang.reflect.Method; +import java.util.Collections; +import java.util.List; + +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotDataFactory; +import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotInfo; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +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.api.ApiConstants; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.InOrder; +import org.mockito.Mockito; + +import com.cloud.agent.AgentManager; +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.RevertDiskOnlyVmSnapshotAnswer; +import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand; +import com.cloud.alert.AlertManager; +import com.cloud.host.DetailVO; +import com.cloud.host.Host; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.host.dao.HostDetailsDao; +import com.cloud.storage.DataStoreRole; +import com.cloud.storage.Volume; +import com.cloud.storage.VolumeVO; +import com.cloud.storage.dao.VolumeDao; +import com.cloud.user.ResourceLimitService; +import com.cloud.uservm.UserVm; +import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.vm.VMInstanceDetailVO; +import com.cloud.vm.UserVmVO; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.dao.VMInstanceDetailsDao; +import com.cloud.vm.dao.UserVmDao; +import com.cloud.vm.snapshot.VMSnapshot; +import com.cloud.vm.snapshot.VMSnapshotDetailsVO; +import com.cloud.vm.snapshot.VMSnapshotVO; +import com.cloud.vm.snapshot.dao.VMSnapshotDao; +import com.cloud.vm.snapshot.dao.VMSnapshotDetailsDao; + +public class KvmFileBasedStorageVmSnapshotStrategyTest { + + private KvmFileBasedStorageVmSnapshotStrategy strategy; + private VMSnapshotDetailsDao vmSnapshotDetailsDao; + private VMSnapshotDao vmSnapshotDao; + private VMSnapshotHelper vmSnapshotHelper; + private AgentManager agentMgr; + private SnapshotDataStoreDao snapshotDataStoreDao; + private HostDetailsDao hostDetailsDao; + private HostDao hostDao; + private PrimaryDataStoreDao storagePoolDao; + + @Before + public void setup() { + strategy = Mockito.spy(new KvmFileBasedStorageVmSnapshotStrategy()); + vmSnapshotDetailsDao = mock(VMSnapshotDetailsDao.class); + vmSnapshotDao = mock(VMSnapshotDao.class); + vmSnapshotHelper = mock(VMSnapshotHelper.class); + agentMgr = mock(AgentManager.class); + snapshotDataStoreDao = mock(SnapshotDataStoreDao.class); + hostDetailsDao = mock(HostDetailsDao.class); + hostDao = mock(HostDao.class); + storagePoolDao = mock(PrimaryDataStoreDao.class); + + strategy.vmSnapshotDetailsDao = vmSnapshotDetailsDao; + strategy.vmSnapshotDao = vmSnapshotDao; + strategy.vmSnapshotHelper = vmSnapshotHelper; + strategy.agentMgr = agentMgr; + strategy.snapshotDataStoreDao = snapshotDataStoreDao; + strategy.hostDao = hostDao; + strategy.storagePool = storagePoolDao; + strategy.resourceLimitManager = mock(ResourceLimitService.class); + strategy.snapshotDataFactory = mock(SnapshotDataFactory.class); + strategy.userVmDao = mock(UserVmDao.class); + strategy.volumeDao = mock(VolumeDao.class); + strategy.vmInstanceDetailsDao = mock(VMInstanceDetailsDao.class); + strategy.hostDetailsDao = hostDetailsDao; + strategy.alertManager = mock(AlertManager.class); + doNothing().when(strategy).publishUsageEvent(anyString(), any(VMSnapshot.class), any(UserVm.class), anyLong(), anyLong()); + doNothing().when(strategy).publishUsageEvent(anyString(), any(VMSnapshot.class), any(UserVm.class), any(VolumeObjectTO.class)); + } + + @Test + public void testProcessCreateVmSnapshotAnswerPersistsNvramPath() throws Exception { + VMSnapshot vmSnapshot = mock(VMSnapshot.class); + CreateDiskOnlyVmSnapshotAnswer answer = mock(CreateDiskOnlyVmSnapshotAnswer.class); + UserVm userVm = mock(UserVm.class); + VMSnapshotVO vmSnapshotVO = mock(VMSnapshotVO.class); + + when(vmSnapshot.getId()).thenReturn(42L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(answer.getMapVolumeToSnapshotSizeAndNewVolumePath()).thenReturn(Collections.emptyMap()); + when(answer.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + + Method method = KvmFileBasedStorageVmSnapshotStrategy.class.getDeclaredMethod("processCreateVmSnapshotAnswer", VMSnapshot.class, java.util.Map.class, + CreateDiskOnlyVmSnapshotAnswer.class, UserVm.class, VMSnapshotVO.class, long.class, VMSnapshotVO.class); + method.setAccessible(true); + method.invoke(strategy, vmSnapshot, Collections.emptyMap(), answer, userVm, vmSnapshotVO, 0L, null); + + verify(vmSnapshotDetailsDao).addDetail(42L, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + } + + @Test + public void testRevertVMSnapshotPassesNvramPathToAgentCommand() { + long vmId = 10L; + long snapshotId = 20L; + long dataStoreId = 30L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotVO currentVmSnapshot = mock(VMSnapshotVO.class); + SnapshotDataStoreVO snapshotDataStoreVO = mock(SnapshotDataStoreVO.class); + SnapshotInfo snapshotInfo = mock(SnapshotInfo.class); + SnapshotObjectTO snapshotObjectTO = mock(SnapshotObjectTO.class); + VMSnapshotDetailsVO volumeSnapshotDetail = new VMSnapshotDetailsVO(snapshotId, "kvmFileBasedStorageSnapshot", String.valueOf(snapshotId), true); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(snapshotId, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(vmSnapshot.getId()).thenReturn(snapshotId); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(userVm.getName()).thenReturn("i-10-VM"); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getLastHostId()).thenReturn(39L); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(hostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + when(vmSnapshotDetailsDao.findDetails(snapshotId, "kvmFileBasedStorageSnapshot")).thenReturn(List.of(volumeSnapshotDetail)); + when(vmSnapshotDetailsDao.findDetail(snapshotId, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(snapshotId, DataStoreRole.Primary)).thenReturn(snapshotDataStoreVO); + when(snapshotDataStoreVO.getSnapshotId()).thenReturn(snapshotId); + when(snapshotDataStoreVO.getDataStoreId()).thenReturn(dataStoreId); + when(strategy.snapshotDataFactory.getSnapshot(snapshotId, dataStoreId, DataStoreRole.Primary)).thenReturn(snapshotInfo); + when(snapshotInfo.getTO()).thenReturn(snapshotObjectTO); + when(vmSnapshotDao.findCurrentSnapshotByVmId(vmId)).thenReturn(currentVmSnapshot); + when(agentMgr.easySend(eq(hostId), any())).thenAnswer(invocation -> + new RevertDiskOnlyVmSnapshotAnswer((RevertDiskOnlyVmSnapshotCommand) invocation.getArgument(1), Collections.emptyList())); + + ArgumentCaptor commandCaptor = ArgumentCaptor.forClass(RevertDiskOnlyVmSnapshotCommand.class); + + strategy.revertVMSnapshot(vmSnapshot); + + verify(agentMgr).easySend(eq(hostId), commandCaptor.capture()); + verify(userVm).setLastHostId(hostId); + verify(strategy.userVmDao).update(vmId, userVm); + assertEquals("vm-uuid", commandCaptor.getValue().getVmUuid()); + assertEquals(true, commandCaptor.getValue().isUefiEnabled()); + assertEquals("nvram/42.fd", commandCaptor.getValue().getNvramSnapshotPath()); + } + + @Test + public void testPickHostForUefiNvramAwareDiskOnlySnapshotUsesCapableCandidateForRevertWhenDefaultHostLacksSupport() { + long vmId = 10L; + long defaultHostId = 40L; + long capableHostId = 41L; + long poolId = 50L; + long clusterId = 60L; + long podId = 70L; + long dataCenterId = 80L; + + UserVmVO userVm = mock(UserVmVO.class); + VolumeVO rootVolume = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + HostVO defaultHost = mock(HostVO.class); + HostVO capableHost = mock(HostVO.class); + + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(defaultHostId); + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootVolume.getPoolId()).thenReturn(poolId); + when(strategy.volumeDao.findByInstance(vmId)).thenReturn(List.of(rootVolume)); + when(storagePoolDao.findById(poolId)).thenReturn(storagePool); + when(storagePool.getClusterId()).thenReturn(clusterId); + when(storagePool.getPodId()).thenReturn(podId); + when(storagePool.getDataCenterId()).thenReturn(dataCenterId); + when(defaultHost.getId()).thenReturn(defaultHostId); + when(capableHost.getId()).thenReturn(capableHostId); + when(hostDao.listAllUpAndEnabledNonHAHosts(Host.Type.Routing, clusterId, podId, dataCenterId, null)) + .thenReturn(List.of(defaultHost, capableHost)); + when(hostDetailsDao.findDetail(defaultHostId, Host.HOST_UEFI_ENABLE)).thenReturn(null); + when(hostDetailsDao.findDetail(defaultHostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + when(hostDetailsDao.findDetail(capableHostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(capableHostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(capableHostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(capableHostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + + Long hostId = strategy.pickHostForUefiNvramAwareDiskOnlySnapshot(userVm, "revert"); + + assertEquals(Long.valueOf(capableHostId), hostId); + } + + @Test(expected = CloudRuntimeException.class) + public void testPickHostForUefiNvramAwareDiskOnlySnapshotFailsCreateWhenSelectedHostIsNotLastHost() { + long vmId = 10L; + long lastHostId = 39L; + long selectedHostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(userVm.getLastHostId()).thenReturn(lastHostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(selectedHostId); + when(hostDetailsDao.findDetail(selectedHostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(selectedHostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(selectedHostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(selectedHostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + + strategy.pickHostForUefiNvramAwareDiskOnlySnapshot(userVm, "create"); + } + + @Test + public void testDeleteNvramSnapshotIfNeededPassesPrimaryDataStoreToAgentCommand() { + long hostId = 40L; + + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(20L, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + PrimaryDataStoreTO primaryDataStore = mock(PrimaryDataStoreTO.class); + + when(vmSnapshot.getId()).thenReturn(20L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshotDetailsDao.findDetail(20L, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + when(agentMgr.easySend(eq(hostId), any())).thenReturn(new Answer(null, true, null)); + + strategy.deleteNvramSnapshotIfNeeded(vmSnapshot, hostId, primaryDataStore); + + verify(agentMgr).easySend(eq(hostId), argThat(command -> { + if (!(command instanceof DeleteDiskOnlyVmSnapshotCommand)) { + return false; + } + + DeleteDiskOnlyVmSnapshotCommand deleteCommand = (DeleteDiskOnlyVmSnapshotCommand) command; + return deleteCommand.getSnapshots().isEmpty() + && "nvram/42.fd".equals(deleteCommand.getNvramSnapshotPath()) + && deleteCommand.getPrimaryDataStore() == primaryDataStore; + })); + } + + @Test(expected = CloudRuntimeException.class) + public void testValidateHostSupportsNvramSidecarCleanupFailsWhenHostLacksNvramAwareCleanupCapability() { + long hostId = 40L; + + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(20L, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + + when(vmSnapshot.getId()).thenReturn(20L); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshotDetailsDao.findDetail(20L, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + try { + strategy.validateHostSupportsNvramSidecarCleanup(vmSnapshot, hostId, "delete"); + } finally { + verify(agentMgr, never()).easySend(eq(hostId), any()); + } + } + + @Test + public void testGetRootVolumePrimaryDataStoreForCleanupReturnsNullWhenRootVolumeIsMissing() { + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VolumeObjectTO dataVolume = mock(VolumeObjectTO.class); + + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(dataVolume.getVolumeType()).thenReturn(Volume.Type.DATADISK); + + PrimaryDataStoreTO primaryDataStore = strategy.getRootVolumePrimaryDataStoreForCleanup(vmSnapshot, List.of(dataVolume)); + + assertNull(primaryDataStore); + } + + @Test + public void testGetPrimaryDataStoreForNvramCleanupPrefersRootSnapshotPrimaryDataStore() { + long vmSnapshotId = 20L; + long rootSnapshotId = 30L; + long dataStoreId = 40L; + + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VolumeObjectTO rootVolume = mock(VolumeObjectTO.class); + PrimaryDataStoreTO liveRootVolumePrimaryDataStore = mock(PrimaryDataStoreTO.class); + PrimaryDataStoreTO rootSnapshotPrimaryDataStore = mock(PrimaryDataStoreTO.class); + SnapshotDataStoreVO rootSnapshotDataStore = mock(SnapshotDataStoreVO.class); + SnapshotInfo rootSnapshotInfo = mock(SnapshotInfo.class); + SnapshotObjectTO rootSnapshotObjectTo = mock(SnapshotObjectTO.class); + VolumeObjectTO rootSnapshotVolume = mock(VolumeObjectTO.class); + VMSnapshotDetailsVO volumeSnapshotDetail = new VMSnapshotDetailsVO(vmSnapshotId, "kvmFileBasedStorageSnapshot", String.valueOf(rootSnapshotId), true); + + when(vmSnapshot.getId()).thenReturn(vmSnapshotId); + when(vmSnapshot.getUuid()).thenReturn("vm-snapshot"); + when(vmSnapshotDetailsDao.findDetails(vmSnapshotId, "kvmFileBasedStorageSnapshot")).thenReturn(List.of(volumeSnapshotDetail)); + when(snapshotDataStoreDao.findOneBySnapshotAndDatastoreRole(rootSnapshotId, DataStoreRole.Primary)).thenReturn(rootSnapshotDataStore); + when(rootSnapshotDataStore.getSnapshotId()).thenReturn(rootSnapshotId); + when(rootSnapshotDataStore.getDataStoreId()).thenReturn(dataStoreId); + when(strategy.snapshotDataFactory.getSnapshot(rootSnapshotId, dataStoreId, DataStoreRole.Primary)).thenReturn(rootSnapshotInfo); + when(rootSnapshotInfo.getTO()).thenReturn(rootSnapshotObjectTo); + when(rootSnapshotObjectTo.getVolume()).thenReturn(rootSnapshotVolume); + when(rootSnapshotVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootSnapshotObjectTo.getDataStore()).thenReturn(rootSnapshotPrimaryDataStore); + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootVolume.getDataStore()).thenReturn(liveRootVolumePrimaryDataStore); + + PrimaryDataStoreTO primaryDataStore = strategy.getPrimaryDataStoreForNvramCleanup(vmSnapshot, List.of(rootVolume)); + + assertSame(rootSnapshotPrimaryDataStore, primaryDataStore); + } + + @Test(expected = CloudRuntimeException.class) + public void testTakeVmSnapshotInternalFailsWhenHostLacksNvramAwareSnapshotCapabilityForUefiVm() throws Exception { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Running); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(hostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + strategy.takeVmSnapshotInternal(vmSnapshot, Collections.emptyMap()); + } + + @Test(expected = CloudRuntimeException.class) + public void testTakeVMSnapshotMarksSnapshotFailedWhenHostCapabilityValidationFails() throws Exception { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Running); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(hostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + try { + strategy.takeVMSnapshot(vmSnapshot); + } finally { + InOrder inOrder = inOrder(vmSnapshotHelper); + inOrder.verify(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.CreateRequested); + inOrder.verify(vmSnapshotHelper).vmSnapshotStateTransitTo(vmSnapshot, VMSnapshot.Event.OperationFailed); + verify(agentMgr, never()).easySend(eq(hostId), any()); + } + } + + @Test(expected = CloudRuntimeException.class) + public void testDeleteVMSnapshotFailsWhenHostLacksNvramAwareCleanupCapabilityForSidecarSnapshot() { + long vmId = 10L; + long vmSnapshotId = 20L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + VMSnapshotDetailsVO nvramDetail = new VMSnapshotDetailsVO(vmSnapshotId, "kvmFileBasedStorageSnapshotNvram", "nvram/42.fd", false); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(vmSnapshot.getId()).thenReturn(vmSnapshotId); + 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(hostId); + when(vmSnapshotDetailsDao.findDetail(vmSnapshotId, "kvmFileBasedStorageSnapshotNvram")).thenReturn(nvramDetail); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + strategy.deleteVMSnapshot(vmSnapshot); + } + + @Test(expected = CloudRuntimeException.class) + public void testTakeVmSnapshotInternalFailsWhenHostLacksUefiCapabilityForUefiVm() throws Exception { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Running); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)).thenReturn(null); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + + strategy.takeVmSnapshotInternal(vmSnapshot, Collections.emptyMap()); + } + + @Test(expected = CloudRuntimeException.class) + public void testRevertVMSnapshotFailsWhenHostLacksNvramAwareSnapshotCapabilityForUefiVm() { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + mockCandidateHostScope(vmId, hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)) + .thenReturn(new DetailVO(hostId, Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString())); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)).thenReturn(null); + + strategy.revertVMSnapshot(vmSnapshot); + } + + @Test(expected = CloudRuntimeException.class) + public void testRevertVMSnapshotFailsWhenHostLacksUefiCapabilityForUefiVm() { + long vmId = 10L; + long hostId = 40L; + + UserVmVO userVm = mock(UserVmVO.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(vmSnapshot.getVmId()).thenReturn(vmId); + when(userVm.getId()).thenReturn(vmId); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getState()).thenReturn(VirtualMachine.State.Stopped); + when(strategy.userVmDao.findById(vmId)).thenReturn(userVm); + when(vmSnapshotHelper.pickRunningHost(vmId)).thenReturn(hostId); + mockCandidateHostScope(vmId, hostId); + when(strategy.vmInstanceDetailsDao.findDetail(vmId, ApiConstants.BootType.UEFI.toString())) + .thenReturn(new VMInstanceDetailVO(vmId, ApiConstants.BootType.UEFI.toString(), "SECURE", true)); + when(hostDetailsDao.findDetail(hostId, Host.HOST_UEFI_ENABLE)).thenReturn(null); + when(hostDetailsDao.findDetail(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM)) + .thenReturn(new DetailVO(hostId, Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString())); + + strategy.revertVMSnapshot(vmSnapshot); + } + + @Test + public void testNotifyGuestRecoveryIssueIfNeededSendsAlert() { + CreateDiskOnlyVmSnapshotAnswer answer = mock(CreateDiskOnlyVmSnapshotAnswer.class); + UserVm userVm = mock(UserVm.class); + VMSnapshotVO vmSnapshot = mock(VMSnapshotVO.class); + + when(answer.getDetails()).thenReturn("VM could not be thawed"); + when(userVm.getUuid()).thenReturn("vm-uuid"); + when(userVm.getDataCenterId()).thenReturn(1L); + when(userVm.getPodIdToDeployIn()).thenReturn(2L); + when(vmSnapshot.getUuid()).thenReturn("snapshot-uuid"); + + strategy.notifyGuestRecoveryIssueIfNeeded(answer, userVm, vmSnapshot); + + verify(strategy.alertManager).sendAlert(eq(AlertManager.AlertType.ALERT_TYPE_VM_SNAPSHOT), eq(1L), eq(2L), anyString(), anyString()); + } + + private void mockCandidateHostScope(long vmId, long hostId) { + long poolId = 50L; + long clusterId = 60L; + long podId = 70L; + long dataCenterId = 80L; + + VolumeVO rootVolume = mock(VolumeVO.class); + StoragePoolVO storagePool = mock(StoragePoolVO.class); + HostVO host = mock(HostVO.class); + + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootVolume.getPoolId()).thenReturn(poolId); + when(strategy.volumeDao.findByInstance(vmId)).thenReturn(List.of(rootVolume)); + when(storagePoolDao.findById(poolId)).thenReturn(storagePool); + when(storagePool.getClusterId()).thenReturn(clusterId); + when(storagePool.getPodId()).thenReturn(podId); + when(storagePool.getDataCenterId()).thenReturn(dataCenterId); + when(host.getId()).thenReturn(hostId); + when(hostDao.listAllUpAndEnabledNonHAHosts(Host.Type.Routing, clusterId, podId, dataCenterId, null)).thenReturn(List.of(host)); + } +} diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java index 061d18dc3769..c80267a4d36e 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/endpoint/DefaultEndPointSelector.java @@ -25,7 +25,6 @@ import java.sql.SQLException; import java.util.ArrayList; import java.util.Collections; -import java.util.Iterator; import java.util.List; import javax.inject.Inject; @@ -198,12 +197,7 @@ private void moveDedicatedHostsToLowerPriority(StringBuilder sbuilder, List, Integer> hostIds = dedicatedResourceDao.searchDedicatedHosts(null, null, account.getId(), null, null); List accountDedicatedHosts = hostIds.first(); for (DedicatedResourceVO accountDedicatedResource: accountDedicatedHosts){ - Iterator dedicatedHostsIterator = dedicatedHosts.iterator(); - while (dedicatedHostsIterator.hasNext()) { - if (dedicatedHostsIterator.next() == accountDedicatedResource.getHostId()) { - dedicatedHostsIterator.remove(); - } - } + dedicatedHosts.removeIf(hostId -> hostId.equals(accountDedicatedResource.getHostId())); } } } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java index 426d157b13a6..58807bdc6a65 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/volume/VolumeServiceImpl.java @@ -329,7 +329,7 @@ protected Void createVolumeCallback(AsyncCallbackDispatcher subGroup = key.subGroup(); ConfigurationSubGroupVO subGroupVO = _configSubGroupDao.findByNameAndGroup(subGroup.first(), groupId); if (subGroupVO == null) { - subGroupVO = new ConfigurationSubGroupVO(); + subGroupVO = new ConfigurationSubGroupVO(subGroup.first(), null, subGroup.second()); + subGroupVO.setGroupId(groupId); subGroupVO = _configSubGroupDao.persist(subGroupVO); } subGroupId = subGroupVO.getId(); diff --git a/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java b/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java index f4012b43fa8d..97f9b1765a7b 100644 --- a/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java +++ b/framework/config/src/test/java/org/apache/cloudstack/framework/config/impl/ConfigDepotImplTest.java @@ -19,6 +19,7 @@ package org.apache.cloudstack.framework.config.impl; import java.util.Collections; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -31,6 +32,7 @@ import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.Mockito; @@ -47,9 +49,32 @@ public class ConfigDepotImplTest { @Mock ConfigurationSubGroupDao configSubGroupDao; + @Mock + ConfigurationSubGroupDao _configSubGroupDao; + @InjectMocks private ConfigDepotImpl configDepotImpl = new ConfigDepotImpl(); + @Test + public void createConfigObjectPersistsSubGroupWithNameAndGroupId() { + ConfigKey key = Mockito.mock(ConfigKey.class); + Mockito.when(key.group()).thenReturn(null); + Mockito.when(key.subGroup()).thenReturn(new Pair<>("ConsoleProxy VM", 5L)); + Mockito.when(key.key()).thenReturn("consoleproxy.capacity.standby"); + Mockito.when(_configSubGroupDao.findByNameAndGroup("ConsoleProxy VM", 1L)).thenReturn(null); + Mockito.when(_configSubGroupDao.persist(Mockito.any(ConfigurationSubGroupVO.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + Mockito.when(_configDao.findById("consoleproxy.capacity.standby")).thenReturn(Mockito.mock(ConfigurationVO.class)); + + ArgumentCaptor captor = ArgumentCaptor.forClass(ConfigurationSubGroupVO.class); + ReflectionTestUtils.invokeMethod(configDepotImpl, "createOrupdateConfigObject", + new Date(), "components", key, "someValue"); + + Mockito.verify(_configSubGroupDao).persist(captor.capture()); + Assert.assertEquals("ConsoleProxy VM", captor.getValue().getName()); + Assert.assertEquals(Long.valueOf(1L), captor.getValue().getGroupId()); + } + @Test public void createEmptyScopeLevelMappingsTest() { configDepotImpl.createEmptyScopeLevelMappings(); diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index dcd863465d1b..3fb5728545d6 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -929,7 +929,7 @@ public Class getEntityBeanType() { protected T findOneIncludingRemovedBy(final SearchCriteria sc) { Filter filter = new Filter(1, true); List results = searchIncludingRemoved(sc, filter, null, false); - assert results.size() <= 1 : "Didn't the limiting worked?"; + assert results.size() <= 1 : "Didn't the limiting work?"; return results.size() == 0 ? null : results.get(0); } @@ -949,6 +949,15 @@ public T findOneBy(SearchCriteria sc, final Filter filter) { return results.isEmpty() ? null : results.get(0); } + @DB() + protected T findLastOneBy(SearchCriteria sc) { + sc = checkAndSetRemovedIsNull(sc); + Filter filter = new Filter(_entityBeanType, "id", Boolean.FALSE, 0L, 1L); + List results = searchIncludingRemoved(sc, filter, null, false); + assert results.size() <= 1 : "Didn't the limiting work?"; + return results.size() == 0 ? null : results.get(0); + } + @DB() public List listBy(SearchCriteria sc, final Filter filter) { sc = checkAndSetRemovedIsNull(sc); @@ -2072,8 +2081,8 @@ protected void setField(final Object entity, final ResultSet rs, ResultSetMetaDa } } if(attr == null) { - logger.warn(String.format("Failed to find attribute in the entity %s to map column %s.%s (%s)", - ClassUtils.getUserClass(entity).getSimpleName(), tableName, columnName)); + logger.warn("Failed to find attribute in the entity {} to map column {}.{}", + ClassUtils.getUserClass(entity).getSimpleName(), tableName, columnName); } else { setField(entity, attr.field, rs, index); } diff --git a/framework/ipc/src/main/java/org/apache/cloudstack/framework/serializer/OnwireClassRegistry.java b/framework/ipc/src/main/java/org/apache/cloudstack/framework/serializer/OnwireClassRegistry.java index 3cc643956e93..93f5e0f9b5c0 100644 --- a/framework/ipc/src/main/java/org/apache/cloudstack/framework/serializer/OnwireClassRegistry.java +++ b/framework/ipc/src/main/java/org/apache/cloudstack/framework/serializer/OnwireClassRegistry.java @@ -55,7 +55,7 @@ public OnwireClassRegistry(String packageName) { } public OnwireClassRegistry(List packages) { - packages.addAll(packages); + this.packages.addAll(packages); } public List getPackages() { diff --git a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java index 79075e5c4b64..1cb1cb4e309f 100644 --- a/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java +++ b/framework/jobs/src/main/java/org/apache/cloudstack/framework/jobs/impl/AsyncJobManagerImpl.java @@ -995,6 +995,9 @@ public void reallyRun() { } logger.trace("End cleanup expired async-jobs"); + + cleanupNetworksStuckInImplementing(); + } catch (Throwable e) { logger.error("Unexpected exception when trying to execute queue item, ", e); } @@ -1002,6 +1005,15 @@ public void reallyRun() { }; } + private void cleanupNetworksStuckInImplementing() { + // Cleanup orphaned networks stuck in Implementing state without async jobs + try { + cleanupOrphanedNetworks(); + } catch (Throwable e) { + logger.error("Unexpected exception when trying to cleanup orphaned networks", e); + } + } + @DB protected void expungeAsyncJob(final AsyncJobVO job) { Transaction.execute(new TransactionCallbackNoReturn() { @@ -1339,6 +1351,74 @@ private void cleanupFailedSnapshotsCreatedWithDefaultStrategy(final long msid) { } } + /** + * Cleanup networks that are stuck in Implementing state without associated async jobs. + * This only processes networks that have been stuck for longer than the job expiration threshold. + */ + private void cleanupOrphanedNetworks() { + try { + SearchCriteria sc = networkDao.createSearchCriteria(); + sc.addAnd("state", SearchCriteria.Op.EQ, Network.State.Implementing); + sc.addAnd("removed", SearchCriteria.Op.NULL); + List implementingNetworks = networkDao.search(sc, null); + + if (implementingNetworks == null || implementingNetworks.isEmpty()) { + return; + } + + logger.debug("Found {} networks in Implementing state, checking for orphaned networks", implementingNetworks.size()); + + final long expireMinutes = JobExpireMinutes.value(); + final Date cutoffTime = new Date(System.currentTimeMillis() - (expireMinutes * 60 * 1000)); + + for (NetworkVO network : implementingNetworks) { + if (network.getCreated().after(cutoffTime)) { + logger.trace("Network {} in Implementing state is only {} minutes old (threshold: {} minutes), skipping cleanup", + network.getId(), + (System.currentTimeMillis() - network.getCreated().getTime()) / 60000, + expireMinutes); + continue; + } + + List jobs = _jobDao.findInstancePendingAsyncJobs("Network", network.getAccountId()); + boolean hasActiveJob = false; + for (AsyncJobVO job : jobs) { + if (job.getInstanceId() != null && job.getInstanceId().equals(network.getId())) { + hasActiveJob = true; + break; + } + } + + if (hasActiveJob) { + logger.debug("Network {} in Implementing state has active async job, skipping cleanup", network.getId()); + continue; + } + + logger.warn("Found orphaned network {} in Implementing state without async job. " + + "Network created: {}, age: {} minutes, expiration threshold: {} minutes. Transitioning to Shutdown state.", + network.getId(), network.getCreated(), + (System.currentTimeMillis() - network.getCreated().getTime()) / 60000, + expireMinutes); + updateNetworkState(network); + + } + } catch (Exception e) { + logger.error("Error while cleaning up orphaned networks", e); + } + } + + private void updateNetworkState(NetworkVO network) { + try { + networkOrchestrationService.stateTransitTo(network, Network.Event.OperationFailed); + logger.info("Successfully transitioned orphaned network {} to Shutdown state using state machine", network.getId()); + } catch (final NoTransitionException e) { + logger.debug("State transition failed for orphaned network {}, forcing state update", network.getId()); + network.setState(Network.State.Shutdown); + networkDao.update(network.getId(), network); + logger.info("Successfully forced orphaned network {} to Shutdown state", network.getId()); + } + } + @Override public void onManagementNodeJoined(List nodeList, long selfNodeId) { } diff --git a/framework/spring/module/src/test/java/org/apache/cloudstack/spring/module/factory/ModuleBasedContextFactoryTest.java b/framework/spring/module/src/test/java/org/apache/cloudstack/spring/module/factory/ModuleBasedContextFactoryTest.java index 884665efed2d..b6f55f6b9e37 100644 --- a/framework/spring/module/src/test/java/org/apache/cloudstack/spring/module/factory/ModuleBasedContextFactoryTest.java +++ b/framework/spring/module/src/test/java/org/apache/cloudstack/spring/module/factory/ModuleBasedContextFactoryTest.java @@ -164,11 +164,12 @@ protected void testBeansInContext(ModuleDefinitionSet set, String name, int orde public static class InstantiationCounter { public static Integer count = 0; + private static final Object countLock = new Object(); int myCount; public InstantiationCounter() { - synchronized (count) { + synchronized (countLock) { myCount = count + 1; count = myCount; } diff --git a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java index fb1b78eb963b..c9c85624acbf 100644 --- a/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java +++ b/plugins/backup/nas/src/main/java/org/apache/cloudstack/backup/NASBackupProvider.java @@ -290,9 +290,14 @@ public boolean restoreVMFromBackup(VirtualMachine vm, Backup backup) { } private Pair restoreVMBackup(VirtualMachine vm, Backup backup) { - List backedVolumes = backup.getBackedUpVolumes(); - List backedVolumesUUIDs = backedVolumes.stream() + // Sort once and derive every per-volume list from that same ordering. The UUID list and the + // backup file list are consumed index-by-index on the agent side, so they have to agree; + // sorting only one of them leaves the two out of step whenever the stored order of the + // backed up volumes differs from their device id order. + List backedVolumes = backup.getBackedUpVolumes().stream() .sorted(Comparator.comparingLong(Backup.VolumeInfo::getDeviceId)) + .collect(Collectors.toList()); + List backedVolumesUUIDs = backedVolumes.stream() .map(Backup.VolumeInfo::getUuid) .collect(Collectors.toList()); diff --git a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java index 25c45ed2a102..395b637d4d47 100644 --- a/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java +++ b/plugins/ca/root-ca/src/main/java/org/apache/cloudstack/ca/provider/RootCAProvider.java @@ -440,7 +440,7 @@ private boolean setupCA() { @Override public boolean start() { managementCertificateCustomSAN = CAManager.CertManagementCustomSubjectAlternativeName.value(); - return loadRootCAKeyPair() && loadRootCAKeyPair() && loadManagementKeyStore(); + return loadRootCAKeyPair() && loadManagementKeyStore(); } @Override diff --git a/plugins/hypervisors/hyperv/src/test/java/com/cloud/hypervisor/hyperv/test/HypervDirectConnectResourceTest.java b/plugins/hypervisors/hyperv/src/test/java/com/cloud/hypervisor/hyperv/test/HypervDirectConnectResourceTest.java index d2e92bd5a2b6..6610985d5841 100644 --- a/plugins/hypervisors/hyperv/src/test/java/com/cloud/hypervisor/hyperv/test/HypervDirectConnectResourceTest.java +++ b/plugins/hypervisors/hyperv/src/test/java/com/cloud/hypervisor/hyperv/test/HypervDirectConnectResourceTest.java @@ -40,6 +40,7 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import com.google.common.io.Files; @@ -248,6 +249,7 @@ public final void testGetVmStatsCommand() { Assert.assertTrue(ans.getDetails(), ans.getResult()); } + @Ignore public final void testStartupCommand() { StartupRoutingCommand defaultStartRoutCmd = new StartupRoutingCommand(0, 0, 0, 0, null, Hypervisor.HypervisorType.Hyperv, RouterPrivateIpStrategy.HostLocal); @@ -278,6 +280,7 @@ public final void testStartupCommand() { } // @Test + @Ignore public final void testJson() { StartupStorageCommand sscmd = null; com.cloud.agent.api.StoragePoolInfo pi = new com.cloud.agent.api.StoragePoolInfo("test123", "192.168.0.1", "c:\\", "c:\\", StoragePoolType.Filesystem, 100L, 50L); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java index da60f6fd7177..26fc096eda40 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java @@ -190,9 +190,16 @@ protected boolean isValidProtocolAndVnetId(final String vNetId, final String pro protected String createStorageVnetBridgeIfNeeded(NicTO nic, String trafficLabel, String storageBrName) throws InternalErrorException { - if (!Networks.BroadcastDomainType.Storage.equals(nic.getBroadcastType()) || nic.getBroadcastUri() == null) { + if (nic.getBroadcastUri() == null) { return storageBrName; } + + boolean isStorageBroadcast = Networks.BroadcastDomainType.Storage.equals(nic.getBroadcastType()) || + Networks.BroadcastDomainType.Storage.equals(Networks.BroadcastDomainType.getSchemeValue(nic.getBroadcastUri())); + if (!isStorageBroadcast) { + return storageBrName; + } + String vNetId = Networks.BroadcastDomainType.getValue(nic.getBroadcastUri()); String protocol = Networks.BroadcastDomainType.Vlan.scheme(); if (!isValidProtocolAndVnetId(vNetId, protocol)) { 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 80d9a51cb855..a9ed0c62cab8 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 @@ -17,6 +17,7 @@ package com.cloud.hypervisor.kvm.resource; import static com.cloud.host.Host.HOST_INSTANCE_CONVERSION; +import static com.cloud.host.Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM; import static com.cloud.host.Host.HOST_OVFTOOL_VERSION; import static com.cloud.host.Host.HOST_VDDK_LIB_DIR; import static com.cloud.host.Host.HOST_VDDK_SUPPORT; @@ -4265,6 +4266,7 @@ public StartupCommand[] initialize() { privateIp = cmd.getPrivateIpAddress(); cmd.getHostDetails().putAll(getVersionStrings()); cmd.getHostDetails().put(KeyStoreUtils.SECURED, String.valueOf(isHostSecured()).toLowerCase()); + cmd.getHostDetails().put(HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString()); cmd.setPool(pool); cmd.setCluster(clusterId); cmd.setGatewayIpAddress(localGateway); @@ -4348,12 +4350,12 @@ protected void calculateHostCpuMaxCapacity(int cpuCores, Long cpuSpeed) { LOGGER.info(String.format("Host uses control group [%s].", output)); if (!CGROUP_V2.equals(output)) { - LOGGER.info(String.format("Setting host CPU max capacity to 0, as it uses cgroup v1.", getHostCpuMaxCapacity())); + LOGGER.info("Setting host CPU max capacity: {} to 0, as it uses cgroup v1.", getHostCpuMaxCapacity()); setHostCpuMaxCapacity(0); return; } - LOGGER.info(String.format("Calculating the max shares of the host.")); + LOGGER.info("Calculating the max shares of the host."); setHostCpuMaxCapacity(cpuCores * cpuSpeed.intValue()); LOGGER.info(String.format("The max shares of the host is [%d].", getHostCpuMaxCapacity())); } @@ -5880,7 +5882,7 @@ public List> cleanVMSnapshotMetadata(Domain dm) } for (String snapshotName: snapshotNames) { if (LOGGER.isDebugEnabled()) { - LOGGER.debug(String.format("Cleaning snapshot [%s] of VM [%s] metadata.", snapshotNames, dm.getName())); + LOGGER.debug("Cleaning snapshot {} of VM {} metadata.", Arrays.toString(snapshotNames), dm.getName()); } DomainSnapshot snapshot = dm.snapshotLookupByName(snapshotName); snapshot.delete(flags); // clean metadata of vm snapshot @@ -6334,6 +6336,15 @@ public String getSnapshotTemporaryPath(String diskPath, String snapshotName) { return String.join(File.separator, diskPathSplitted); } + public String getUefiNvramPath(String vmUuid) { + String nvramDirectory = uefiProperties.getProperty(LibvirtVMDef.GuestDef.GUEST_NVRAM_PATH); + if (StringUtils.isBlank(nvramDirectory) || StringUtils.isBlank(vmUuid)) { + return null; + } + + return nvramDirectory + vmUuid + ".fd"; + } + public static String generateSecretUUIDFromString(String seed) { return UuidUtils.nameUUIDFromBytes(seed.getBytes()).toString(); } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java index 84d17a1a1161..46a7da70c61f 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCreateDiskOnlyVMSnapshotCommandWrapper.java @@ -19,6 +19,7 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import com.cloud.agent.api.Answer; +import com.cloud.agent.api.FreezeThawVMCommand; import com.cloud.agent.api.VMSnapshotTO; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotCommand; @@ -32,24 +33,33 @@ import com.cloud.vm.VirtualMachine; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.qemu.QemuCommand; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.commons.lang3.StringUtils; import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.LibvirtException; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.UUID; +import com.cloud.storage.Volume; + @ResourceWrapper(handles = CreateDiskOnlyVmSnapshotCommand.class) public class LibvirtCreateDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper { + protected static final String NVRAM_SNAPSHOT_DIR = ".cloudstack-vm-snapshot-nvram"; private static final String SNAPSHOT_XML = "\n" + "%s\n" + @@ -79,6 +89,11 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma logger.info("Taking disk-only VM snapshot of running VM [{}].", vmName); Domain dm = null; + String nvramSnapshotPath = null; + boolean suspendedByThisWrapper = false; + boolean filesystemsFrozenByThisWrapper = false; + CreateDiskOnlyVmSnapshotAnswer answer = null; + String postSnapshotCleanupIssue = null; try { LibvirtUtilitiesHelper libvirtUtilitiesHelper = resource.getLibvirtUtilitiesHelper(); Connect conn = libvirtUtilitiesHelper.getConnection(); @@ -88,26 +103,45 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma dm = resource.getDomain(conn, vmName); if (dm == null) { - return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, String.format("Creation of disk-only VM Snapshot failed as we could not find the VM [%s].", vmName), null); + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, false, + String.format("Creation of disk-only VM Snapshot failed as we could not find the VM [%s].", vmName), null, null); + return answer; } VMSnapshotTO target = cmd.getTarget(); Pair>> snapshotXmlAndVolumeToNewPathMap = createSnapshotXmlAndNewVolumePathMap(volumeObjectTOS, disks, target, resource); + if (shouldFreezeVmFilesystemsForSnapshot(cmd)) { + // The guest-agent freeze flushes guest filesystems; suspend below prevents concurrent UEFI NVRAM writes. + freezeVmFilesystems(dm, vmName); + filesystemsFrozenByThisWrapper = true; + verifyVmFilesystemsFrozen(dm, vmName); + } + if (shouldSuspendVmForSnapshot(cmd)) { + suspendedByThisWrapper = suspendVmIfNeeded(dm); + } + nvramSnapshotPath = backupNvramIfNeeded(cmd, resource); - dm.snapshotCreateXML(snapshotXmlAndVolumeToNewPathMap.first(), getFlagsToUseForRunningVmSnapshotCreation(target)); + dm.snapshotCreateXML(snapshotXmlAndVolumeToNewPathMap.first(), getFlagsToUseForRunningVmSnapshotCreation(target, filesystemsFrozenByThisWrapper)); - return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, snapshotXmlAndVolumeToNewPathMap.second()); - } catch (LibvirtException e) { + postSnapshotCleanupIssue = recoverVmAfterSnapshot(dm, vmName, suspendedByThisWrapper, filesystemsFrozenByThisWrapper, postSnapshotCleanupIssue); + filesystemsFrozenByThisWrapper = false; + suspendedByThisWrapper = false; + + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, snapshotXmlAndVolumeToNewPathMap.second(), nvramSnapshotPath); + } catch (LibvirtException | IOException e) { String errorMsg = String.format("Creation of disk-only VM snapshot for VM [%s] failed due to %s.", vmName, e.getMessage()); logger.error(errorMsg, e); - if (e.getMessage().contains("QEMU guest agent is not connected")) { + cleanupNvramSnapshotIfNeeded(cmd, resource, nvramSnapshotPath); + if (StringUtils.contains(e.getMessage(), "QEMU guest agent is not connected")) { errorMsg = "QEMU guest agent is not connected. If the VM has been recently started, it might connect soon. Otherwise the VM does not have the" + " guest agent installed; thus the QuiesceVM parameter is not supported."; - return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null); + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, false, errorMsg, null, null); + } else { + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, false, e.getMessage(), null, null); } - return new CreateDiskOnlyVmSnapshotAnswer(cmd, false, e.getMessage(), null); } finally { if (dm != null) { + postSnapshotCleanupIssue = recoverVmAfterSnapshot(dm, vmName, suspendedByThisWrapper, filesystemsFrozenByThisWrapper, postSnapshotCleanupIssue); try { dm.free(); } catch (LibvirtException l) { @@ -115,6 +149,12 @@ protected Answer takeDiskOnlyVmSnapshotOfRunningVm(CreateDiskOnlyVmSnapshotComma } } } + + if (answer != null && StringUtils.isNotBlank(postSnapshotCleanupIssue)) { + answer = new CreateDiskOnlyVmSnapshotAnswer(cmd, answer.getResult(), + appendSnapshotOperationIssue(answer.getDetails(), postSnapshotCleanupIssue), answer.getMapVolumeToSnapshotSizeAndNewVolumePath(), answer.getNvramSnapshotPath()); + } + return answer; } protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { @@ -122,10 +162,12 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma logger.info("Taking disk-only VM snapshot of stopped VM [{}].", vmName); Map> mapVolumeToSnapshotSizeAndNewVolumePath = new HashMap<>(); + String nvramSnapshotPath = null; List volumeObjectTos = cmd.getVolumeTOs(); KVMStoragePoolManager storagePoolMgr = resource.getStoragePoolMgr(); try { + nvramSnapshotPath = backupNvramIfNeeded(cmd, resource); for (VolumeObjectTO volumeObjectTO : volumeObjectTos) { PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) volumeObjectTO.getDataStore(); KVMStoragePool kvmStoragePool = storagePoolMgr.getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); @@ -144,7 +186,7 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma mapVolumeToSnapshotSizeAndNewVolumePath.put(volumeObjectTO.getUuid(), new Pair<>(getFileSize(currentDeltaFullPath), snapshotPath)); } - } catch (LibvirtException | QemuImgException e) { + } catch (LibvirtException | QemuImgException | IOException e) { logger.error("Exception while creating disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e); for (VolumeObjectTO volumeObjectTO : volumeObjectTos) { Pair volSizeAndNewPath = mapVolumeToSnapshotSizeAndNewVolumePath.get(volumeObjectTO.getUuid()); @@ -160,14 +202,15 @@ protected Answer takeDiskOnlyVmSnapshotOfStoppedVm(CreateDiskOnlyVmSnapshotComma logger.warn("Tried to delete leftover snapshot at [{}] failed.", volSizeAndNewPath.second(), ex); } } + cleanupNvramSnapshotIfNeeded(cmd, resource, nvramSnapshotPath); return new Answer(cmd, e); } - return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, mapVolumeToSnapshotSizeAndNewVolumePath); + return new CreateDiskOnlyVmSnapshotAnswer(cmd, true, null, mapVolumeToSnapshotSizeAndNewVolumePath, nvramSnapshotPath); } - protected int getFlagsToUseForRunningVmSnapshotCreation(VMSnapshotTO target) { - int flags = target.getQuiescevm() ? Domain.SnapshotCreateFlags.QUIESCE : 0; + protected int getFlagsToUseForRunningVmSnapshotCreation(VMSnapshotTO target, boolean filesystemsFrozenByThisWrapper) { + int flags = target.getQuiescevm() && !filesystemsFrozenByThisWrapper ? Domain.SnapshotCreateFlags.QUIESCE : 0; flags += Domain.SnapshotCreateFlags.DISK_ONLY + Domain.SnapshotCreateFlags.ATOMIC + Domain.SnapshotCreateFlags.NO_METADATA; @@ -195,4 +238,193 @@ protected Pair>> createSnapshotXmlAndNewV protected long getFileSize(String path) { return new File(path).length(); } + + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws IOException, LibvirtException { + if (!cmd.isUefiEnabled()) { + return null; + } + + String activeNvramPath = resource.getUefiNvramPath(cmd.getVmUuid()); + if (StringUtils.isBlank(activeNvramPath) || !Files.exists(Path.of(activeNvramPath))) { + throw new IOException(String.format("Unable to find the active UEFI NVRAM file for VM [%s].", cmd.getVmName())); + } + + VolumeObjectTO rootVolume = getRootVolume(cmd.getVolumeTOs()); + PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) rootVolume.getDataStore(); + KVMStoragePool storagePool = resource.getStoragePoolMgr().getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); + + String nvramSnapshotPath = getNvramSnapshotRelativePath(cmd.getTarget().getId()); + Path targetPath = Path.of(storagePool.getLocalPathFor(nvramSnapshotPath)); + Files.createDirectories(targetPath.getParent()); + Files.copy(Path.of(activeNvramPath), targetPath, StandardCopyOption.REPLACE_EXISTING); + return nvramSnapshotPath; + } + + protected void cleanupNvramSnapshotIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, String nvramSnapshotPath) { + if (StringUtils.isBlank(nvramSnapshotPath)) { + return; + } + + try { + VolumeObjectTO rootVolume = getRootVolume(cmd.getVolumeTOs()); + PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) rootVolume.getDataStore(); + KVMStoragePool storagePool = resource.getStoragePoolMgr().getStoragePool(primaryDataStoreTO.getPoolType(), primaryDataStoreTO.getUuid()); + Files.deleteIfExists(Path.of(storagePool.getLocalPathFor(nvramSnapshotPath))); + } catch (Exception e) { + logger.warn("Failed to clean up temporary NVRAM snapshot [{}] for VM [{}].", nvramSnapshotPath, cmd.getVmName(), e); + } + } + + protected String getNvramSnapshotRelativePath(Long vmSnapshotId) { + return String.format("%s/%s.fd", NVRAM_SNAPSHOT_DIR, vmSnapshotId); + } + + protected VolumeObjectTO getRootVolume(List volumeObjectTos) { + return volumeObjectTos.stream() + .filter(volumeObjectTO -> Volume.Type.ROOT.equals(volumeObjectTO.getVolumeType())) + .findFirst() + .orElseThrow(() -> new IllegalStateException("Unable to locate the root volume while handling the VM snapshot.")); + } + + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return cmd.isUefiEnabled(); + } + + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return cmd.isUefiEnabled() && cmd.getTarget().getQuiescevm(); + } + + protected boolean suspendVmIfNeeded(Domain domain) throws LibvirtException { + if (domain.getInfo().state == org.libvirt.DomainInfo.DomainState.VIR_DOMAIN_PAUSED) { + return false; + } + + domain.suspend(); + return true; + } + + protected void freezeVmFilesystems(Domain domain, String vmName) throws LibvirtException, IOException { + String result = getResultOfQemuCommand(FreezeThawVMCommand.FREEZE, domain); + if (isQemuAgentErrorResponse(result)) { + throw new IOException(String.format("Failed to freeze VM [%s] filesystems before taking the disk-only VM snapshot. Result: %s", vmName, result)); + } + } + + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) throws LibvirtException, IOException { + String status = getResultOfQemuCommand(FreezeThawVMCommand.STATUS, domain); + if (StringUtils.isBlank(status)) { + throw new IOException(String.format("Failed to verify VM [%s] filesystem freeze state before taking the disk-only VM snapshot. Result: %s", vmName, status)); + } + + JsonObject statusObject; + try { + JsonElement statusElement = new JsonParser().parse(status); + if (!statusElement.isJsonObject()) { + throw new IOException(String.format("Failed to verify VM [%s] filesystem freeze state before taking the disk-only VM snapshot. Result: %s", vmName, status)); + } + statusObject = statusElement.getAsJsonObject(); + } catch (RuntimeException e) { + throw new IOException(String.format("Failed to verify VM [%s] filesystem freeze state before taking the disk-only VM snapshot. Result: %s", vmName, status), e); + } + + if (statusObject.has("error")) { + throw new IOException(String.format("Failed to verify VM [%s] filesystem freeze state before taking the disk-only VM snapshot. Result: %s", vmName, status)); + } + + JsonElement returnElement = statusObject.get("return"); + if (returnElement == null || !returnElement.isJsonPrimitive() || !returnElement.getAsJsonPrimitive().isString()) { + throw new IOException(String.format("Failed to verify VM [%s] filesystem freeze state before taking the disk-only VM snapshot. Result: %s", vmName, status)); + } + + String statusResult = returnElement.getAsString(); + if (!FreezeThawVMCommand.FREEZE.equals(statusResult)) { + throw new IOException(String.format("Failed to freeze VM [%s] filesystems before taking the disk-only VM snapshot. Status: %s", vmName, statusResult)); + } + } + + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName, boolean filesystemsFrozenByThisWrapper) { + if (!filesystemsFrozenByThisWrapper) { + return true; + } + return thawVmFilesystemsIfNeeded(domain, vmName); + } + + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + try { + String result = getResultOfQemuCommand(FreezeThawVMCommand.THAW, domain); + if (isQemuAgentErrorResponse(result)) { + logger.warn("Failed to thaw VM [{}] filesystems after taking the disk-only VM snapshot. Result: {}", vmName, result); + return false; + } + return true; + } catch (LibvirtException e) { + logger.warn("Failed to thaw VM [{}] filesystems after taking the disk-only VM snapshot.", vmName, e); + return false; + } + } + + protected boolean isQemuAgentErrorResponse(String result) { + if (StringUtils.isBlank(result) || result.startsWith("error")) { + return true; + } + + try { + JsonElement resultElement = new JsonParser().parse(result); + return resultElement.isJsonObject() && resultElement.getAsJsonObject().has("error"); + } catch (RuntimeException e) { + return false; + } + } + + protected String getResultOfQemuCommand(String cmd, Domain domain) throws LibvirtException { + if (cmd.equals(FreezeThawVMCommand.FREEZE)) { + return domain.qemuAgentCommand(QemuCommand.buildQemuCommand(QemuCommand.AGENT_FREEZE, null), 10, 0); + } else if (cmd.equals(FreezeThawVMCommand.THAW)) { + return domain.qemuAgentCommand(QemuCommand.buildQemuCommand(QemuCommand.AGENT_THAW, null), 10, 0); + } else if (cmd.equals(FreezeThawVMCommand.STATUS)) { + return domain.qemuAgentCommand(QemuCommand.buildQemuCommand(QemuCommand.AGENT_FREEZE_STATUS, null), 10, 0); + } + return null; + } + + protected boolean resumeVmIfNeeded(Domain domain, String vmName, boolean suspendedByThisWrapper) { + if (!suspendedByThisWrapper) { + return true; + } + return resumeVmIfNeeded(domain, vmName); + } + + protected boolean resumeVmIfNeeded(Domain domain, String vmName) { + try { + if (domain.getInfo().state == org.libvirt.DomainInfo.DomainState.VIR_DOMAIN_PAUSED) { + domain.resume(); + } + return true; + } catch (LibvirtException e) { + logger.warn("Failed to resume VM [{}] after taking the disk-only VM snapshot.", vmName, e); + return false; + } + } + + protected String recoverVmAfterSnapshot(Domain domain, String vmName, boolean suspendedByThisWrapper, boolean filesystemsFrozenByThisWrapper, String currentIssue) { + if (suspendedByThisWrapper && !resumeVmIfNeeded(domain, vmName)) { + currentIssue = appendSnapshotOperationIssue(currentIssue, + String.format("VM [%s] could not be resumed after taking the disk-only snapshot. Guest may still be paused.", vmName)); + } + if (filesystemsFrozenByThisWrapper && !thawVmFilesystemsIfNeeded(domain, vmName)) { + currentIssue = appendSnapshotOperationIssue(currentIssue, + String.format("VM [%s] filesystems could not be thawed after taking the disk-only snapshot. Guest may still be frozen.", vmName)); + } + return currentIssue; + } + + protected String appendSnapshotOperationIssue(String currentIssue, String newIssue) { + if (StringUtils.isBlank(newIssue)) { + return currentIssue; + } + if (StringUtils.isBlank(currentIssue)) { + return newIssue; + } + return currentIssue + " " + newIssue; + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java index 15df8627a8a7..62c7c5013374 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper.java @@ -27,12 +27,16 @@ import com.cloud.resource.CommandWrapper; import com.cloud.resource.ResourceWrapper; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.commons.lang3.StringUtils; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.List; +import com.cloud.storage.Volume; + @ResourceWrapper(handles = DeleteDiskOnlyVmSnapshotCommand.class) public class LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper { @@ -53,6 +57,40 @@ public Answer execute(DeleteDiskOnlyVmSnapshotCommand command, LibvirtComputingR return new Answer(command, e); } } + + deleteNvramSnapshotIfNeeded(command, resource, storagePoolMgr, snapshotsToDelete); return new Answer(command, true, null); } + + protected void deleteNvramSnapshotIfNeeded(DeleteDiskOnlyVmSnapshotCommand command, LibvirtComputingResource resource, KVMStoragePoolManager storagePoolMgr, + List snapshotsToDelete) { + if (StringUtils.isBlank(command.getNvramSnapshotPath())) { + return; + } + + try { + KVMStoragePool storagePool; + if (command.getPrimaryDataStore() != null) { + PrimaryDataStoreTO dataStore = command.getPrimaryDataStore(); + storagePool = storagePoolMgr.getStoragePool(dataStore.getPoolType(), dataStore.getUuid()); + } else { + SnapshotObjectTO rootVolumeSnapshot = snapshotsToDelete.stream() + .map(SnapshotObjectTO.class::cast) + .filter(snapshotObjectTO -> Volume.Type.ROOT.equals(snapshotObjectTO.getVolume().getVolumeType())) + .findFirst() + .orElse(null); + + if (rootVolumeSnapshot == null) { + logger.warn("Unable to locate the root volume snapshot while deleting NVRAM snapshot [{}].", command.getNvramSnapshotPath()); + return; + } + + storagePool = resource.getLibvirtUtilitiesHelper().getPrimaryPoolFromDataTo(rootVolumeSnapshot, storagePoolMgr); + } + + Files.deleteIfExists(Path.of(storagePool.getLocalPathFor(command.getNvramSnapshotPath()))); + } catch (Exception e) { + logger.warn("Failed to delete the UEFI NVRAM snapshot [{}]. It will be left behind on storage.", command.getNvramSnapshotPath(), e); + } + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java index 5a7d6d2c203a..1ff6d7851f20 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtReadyCommandWrapper.java @@ -43,6 +43,7 @@ public final class LibvirtReadyCommandWrapper extends CommandWrapper hostDetails = new HashMap(); + hostDetails.put(Host.HOST_KVM_DISK_ONLY_VM_SNAPSHOT_NVRAM, Boolean.TRUE.toString()); if (hostSupportsUefi(libvirtComputingResource.isUbuntuOrDebianHost()) && libvirtComputingResource.isUefiPropertiesFileLoaded()) { hostDetails.put(Host.HOST_UEFI_ENABLE, Boolean.TRUE.toString()); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java index 9ebb180b055f..417a075a8567 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapper.java @@ -20,13 +20,16 @@ package com.cloud.hypervisor.kvm.resource.wrapper; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; import org.apache.cloudstack.backup.BackupAnswer; @@ -98,11 +101,11 @@ public Answer execute(RestoreBackupCommand command, LibvirtComputingResource ser newVolumeId = getVolumeUuidFromPath(volumePath, volumePool); Long size = command.getRestoreVolumeSizes().get(0); restoreVolume(storagePoolMgr, backupPath, volumePool, volumePath, diskType, backupFile, size, - new Pair<>(vmName, command.getVmState()), mountDirectory, timeout); + new Pair<>(vmName, command.getVmState()), mountDirectory, timeout, mountTimeout); } else if (Boolean.TRUE.equals(vmExists)) { - restoreVolumesOfExistingVM(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backedVolumeUUIDs, backupPath, backupFiles, mountDirectory, timeout); + restoreVolumesOfExistingVM(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backedVolumeUUIDs, backupPath, backupFiles, mountDirectory, timeout, mountTimeout); } else { - restoreVolumesOfDestroyedVMs(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backupPath, backupFiles, mountDirectory, timeout); + restoreVolumesOfDestroyedVMs(storagePoolMgr, restoreVolumePools, restoreVolumePaths, backupPath, backupFiles, mountDirectory, timeout, mountTimeout); } } catch (CloudRuntimeException e) { String errorMessage = e.getMessage() != null ? e.getMessage() : ""; @@ -123,14 +126,41 @@ private void verifyBackupFile(String backupPath, String volUuid) { private void restoreVolumesOfExistingVM(KVMStoragePoolManager storagePoolMgr, List restoreVolumePools, List restoreVolumePaths, List backedVolumesUUIDs, - String backupPath, List backupFiles, String mountDirectory, int timeout) { + String backupPath, List backupFiles, String mountDirectory, int timeout, Integer mountTimeout) { String diskType = "root"; try { - for (int idx = 0; idx < restoreVolumePaths.size(); idx++) { - PrimaryDataStoreTO restoreVolumePool = restoreVolumePools.get(idx); - String restoreVolumePath = restoreVolumePaths.get(idx); - String backupFile = backupFiles.get(idx); + // Match each backed up volume to the instance's volume with the SAME UUID. Both lists + // arrive ordered by device id, but device ids are not stable across restores, so relying + // on the position within the list can write a backup into a different volume than the one + // it was taken from. + Map targetIndexByVolumeUuid = new HashMap<>(); + for (int i = 0; i < restoreVolumePaths.size(); i++) { + targetIndexByVolumeUuid.put(getVolumeUuidFromPath(restoreVolumePaths.get(i), restoreVolumePools.get(i)), i); + } + + // Creating an instance from a backup gives it brand new volumes, so none of the uuids + // recorded in the backup can match. Only then fall back to the device id ordering both + // lists already carry; when some of them do match, a missing one really is a volume that + // was detached, and writing its backup into another volume would be wrong. + boolean restoringIntoNewVolumes = backedVolumesUUIDs.stream().noneMatch(targetIndexByVolumeUuid::containsKey); + if (restoringIntoNewVolumes && backedVolumesUUIDs.size() != restoreVolumePaths.size()) { + throw new CloudRuntimeException(String.format("Unable to restore backup: it holds %d volumes but the instance has %d.", + backedVolumesUUIDs.size(), restoreVolumePaths.size())); + } + if (restoringIntoNewVolumes) { + logger.debug("None of the backed up volumes belong to this instance; restoring into its volumes in device id order."); + } + + for (int idx = 0; idx < backedVolumesUUIDs.size(); idx++) { String backupVolumeUuid = backedVolumesUUIDs.get(idx); + Integer targetIdx = restoringIntoNewVolumes ? Integer.valueOf(idx) : targetIndexByVolumeUuid.get(backupVolumeUuid); + if (targetIdx == null) { + throw new CloudRuntimeException(String.format("Unable to restore backup: volume [%s] recorded in the backup" + + " is not attached to the instance any more.", backupVolumeUuid)); + } + PrimaryDataStoreTO restoreVolumePool = restoreVolumePools.get(targetIdx); + String restoreVolumePath = restoreVolumePaths.get(targetIdx); + String backupFile = backupFiles.get(idx); String fullPath = getBackupPath(mountDirectory, backupPath, backupFile, diskType); diskType = "datadisk"; @@ -140,13 +170,13 @@ private void restoreVolumesOfExistingVM(KVMStoragePoolManager storagePoolMgr, Li } } } finally { - unmountBackupDirectory(mountDirectory); + unmountBackupDirectory(mountDirectory, mountTimeout); deleteTemporaryDirectory(mountDirectory); } } private void restoreVolumesOfDestroyedVMs(KVMStoragePoolManager storagePoolMgr, List volumePools, - List volumePaths, String backupPath, List backupFiles, String mountDirectory, int timeout) { + List volumePaths, String backupPath, List backupFiles, String mountDirectory, int timeout, Integer mountTimeout) { String diskType = "root"; try { for (int i = 0; i < volumePaths.size(); i++) { @@ -162,13 +192,13 @@ private void restoreVolumesOfDestroyedVMs(KVMStoragePoolManager storagePoolMgr, } } } finally { - unmountBackupDirectory(mountDirectory); + unmountBackupDirectory(mountDirectory, mountTimeout); deleteTemporaryDirectory(mountDirectory); } } private void restoreVolume(KVMStoragePoolManager storagePoolMgr, String backupPath, PrimaryDataStoreTO volumePool, String volumePath, String diskType, String backupFile, - Long size, Pair vmNameAndState, String mountDirectory, int timeout) { + Long size, Pair vmNameAndState, String mountDirectory, int timeout, Integer mountTimeout) { String bkpPath; String volumeUuid; try { @@ -185,7 +215,7 @@ private void restoreVolume(KVMStoragePoolManager storagePoolMgr, String backupPa } } } finally { - unmountBackupDirectory(mountDirectory); + unmountBackupDirectory(mountDirectory, mountTimeout); deleteTemporaryDirectory(mountDirectory); } } @@ -201,6 +231,7 @@ private String mountBackupDirectory(String backupRepoAddress, String backupRepoT logger.error("Failed to create the tmp mount directory {} for restore", mountDirectory, e); throw new CloudRuntimeException("Failed to create the tmp mount directory for restore on the KVM host"); } + int exitValue; try { String mountPath = Script.getExecutableAbsolutePath("mount"); List mountCmd = new ArrayList<>(); @@ -221,23 +252,42 @@ private String mountBackupDirectory(String backupRepoAddress, String backupRepoT mountCmd.add("-o"); mountCmd.add(mountOptions); } - Script.executeCommand(mountCmd.toArray(new String[0])); + exitValue = Script.executeCommandForExitValue(mountTimeout, mountCmd.toArray(new String[0])); } catch (Exception e) { logger.error("Failed to mount repository {} of type {} to the directory {}", backupRepoAddress, backupRepoType, mountDirectory, e); throw new CloudRuntimeException("Failed to mount the backup repository on the KVM host"); } + if (exitValue != 0) { + logger.error("Failed to mount repository {} of type {} to the directory {}, mount exited with {}", backupRepoAddress, + backupRepoType, mountDirectory, exitValue); + removeTemporaryDirectoryQuietly(mountDirectory); + throw new CloudRuntimeException("Failed to mount the backup repository on the KVM host"); + } return mountDirectory; } - private void unmountBackupDirectory(String backupDirectory) { + private void unmountBackupDirectory(String backupDirectory, Integer mountTimeout) { + int exitValue; try { String umountPath = Script.getExecutableAbsolutePath("umount"); String[] umountCmd = new String[] { "sudo", umountPath, backupDirectory }; - Script.executeCommand(umountCmd); + exitValue = Script.executeCommandForExitValue(mountTimeout, umountCmd); } catch (Exception e) { logger.error("Failed to unmount backup directory {}", backupDirectory, e); throw new CloudRuntimeException("Failed to unmount the backup directory"); } + if (exitValue != 0) { + logger.error("Failed to unmount backup directory {}, umount exited with {}", backupDirectory, exitValue); + throw new CloudRuntimeException("Failed to unmount the backup directory"); + } + } + + private void removeTemporaryDirectoryQuietly(String backupDirectory) { + try { + Files.deleteIfExists(Paths.get(backupDirectory)); + } catch (IOException e) { + logger.warn("Failed to remove the temporary mount directory {} after the mount failed.", backupDirectory, e); + } } private void deleteTemporaryDirectory(String backupDirectory) { @@ -276,7 +326,7 @@ private boolean replaceVolumeWithBackup(KVMStoragePoolManager storagePoolMgr, Pr } String[] rsyncCmd = new String[] { Script.getExecutableAbsolutePath("rsync"), "-az", backupPath, volumePath }; - int exitValue = Script.executeCommandForExitValue(rsyncCmd); + int exitValue = Script.executeCommandForExitValue(timeout, rsyncCmd); return exitValue == 0; } @@ -340,38 +390,68 @@ private boolean replaceBlockDeviceWithBackup(KVMStoragePoolManager storagePoolMg private boolean attachVolumeToVm(KVMStoragePoolManager storagePoolMgr, String vmName, PrimaryDataStoreTO volumePool, String volumePath) { String deviceToAttachDiskTo = getDeviceToAttachDisk(vmName); + if (Storage.StoragePoolType.RBD.equals(volumePool.getPoolType())) { + return attachRbdVolumeToVm(storagePoolMgr, vmName, volumePool, volumePath, deviceToAttachDiskTo); + } List virshCmd = new ArrayList<>(); virshCmd.add(Script.getExecutableAbsolutePath("virsh")); - if (volumePool.getPoolType() == Storage.StoragePoolType.RBD) { - String xmlForRbdDisk = getXmlForRbdDisk(storagePoolMgr, volumePool, volumePath, deviceToAttachDiskTo); - logger.debug("RBD disk xml to attach: {}", xmlForRbdDisk); - virshCmd.add("attach-device"); - virshCmd.add(vmName); - virshCmd.add("/dev/stdin"); - virshCmd.add("< result = Script.executePipedCommands(Arrays.asList(domblkCmd, tailCmd, headCmd, awkCmd), 0); - String currentDevice = result.second(); + // executePipedCommands appends a line separator to every line it reads, so the device + // name has to be trimmed before the last character can be incremented. + String currentDevice = result.second() == null ? "" : result.second().trim(); + if (result.first() == null || result.first() != 0 || StringUtils.isBlank(currentDevice)) { + throw new CloudRuntimeException(String.format("Failed to determine the device to attach the restored volume to on VM [%s].", vmName)); + } char lastChar = currentDevice.charAt(currentDevice.length() - 1); char incrementedChar = (char) (lastChar + 1); return currentDevice.substring(0, currentDevice.length() - 1) + incrementedChar; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java index 1aa79d48eec2..e4cd527331bd 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRevertDiskOnlyVMSnapshotCommandWrapper.java @@ -31,15 +31,20 @@ import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; +import org.apache.commons.lang3.StringUtils; import org.libvirt.LibvirtException; import java.io.IOException; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import com.cloud.storage.Volume; + @ResourceWrapper(handles = RevertDiskOnlyVmSnapshotCommand.class) public class LibvirtRevertDiskOnlyVMSnapshotCommandWrapper extends CommandWrapper { @@ -55,6 +60,8 @@ public Answer execute(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResou HashMap snapshotToNewDeltaPath = new HashMap<>(); try { + SnapshotObjectTO rootVolumeSnapshot = getRootVolumeSnapshot(snapshotObjectTos); + validateNvramRevertState(cmd, resource, rootVolumeSnapshot, storagePoolMgr); for (SnapshotObjectTO snapshotObjectTo : snapshotObjectTos) { KVMStoragePool kvmStoragePool = libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(snapshotObjectTo, storagePoolMgr); @@ -71,7 +78,8 @@ public Answer execute(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResou qemuImg.create(newDelta, currentDelta); snapshotToNewDeltaPath.put(snapshotObjectTo, deltaPath); } - } catch (LibvirtException | QemuImgException e) { + restoreNvramIfNeeded(cmd, resource, rootVolumeSnapshot, storagePoolMgr); + } catch (LibvirtException | QemuImgException | IOException e) { logger.error("Exception while reverting disk-only VM snapshot for VM [{}]. Deleting leftover deltas.", vmName, e); for (SnapshotObjectTO snapshotObjectTo : snapshotObjectTos) { String newPath = snapshotToNewDeltaPath.get(snapshotObjectTo); @@ -108,4 +116,88 @@ public Answer execute(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResou return new RevertDiskOnlyVmSnapshotAnswer(cmd, volumeObjectTos); } + + protected SnapshotObjectTO getRootVolumeSnapshot(List snapshotObjectTos) { + return snapshotObjectTos.stream() + .filter(snapshotObjectTO -> Volume.Type.ROOT.equals(snapshotObjectTO.getVolume().getVolumeType())) + .findFirst() + .orElse(null); + } + + protected void validateNvramRevertState(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, SnapshotObjectTO rootVolumeSnapshot, + KVMStoragePoolManager storagePoolMgr) throws IOException, LibvirtException { + String activeNvramPath = resource.getUefiNvramPath(cmd.getVmUuid()); + if (StringUtils.isBlank(cmd.getNvramSnapshotPath())) { + if (cmd.isUefiEnabled()) { + throw new IOException(String.format("Cannot safely revert disk-only VM snapshot for UEFI VM [%s] because the snapshot does not contain NVRAM state.", + cmd.getVmName())); + } + return; + } + + if (StringUtils.isBlank(activeNvramPath)) { + throw new IOException(String.format("Unable to determine the active UEFI NVRAM path for VM [%s].", cmd.getVmName())); + } + + Path snapshotNvramPath = getNvramSnapshotAbsolutePath(cmd.getNvramSnapshotPath(), rootVolumeSnapshot, resource, storagePoolMgr); + if (!Files.exists(snapshotNvramPath)) { + throw new IOException(String.format("Unable to find the UEFI NVRAM snapshot [%s] for VM [%s].", cmd.getNvramSnapshotPath(), cmd.getVmName())); + } + } + + protected void restoreNvramIfNeeded(RevertDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, SnapshotObjectTO rootVolumeSnapshot, + KVMStoragePoolManager storagePoolMgr) throws IOException, LibvirtException { + if (StringUtils.isBlank(cmd.getNvramSnapshotPath())) { + return; + } + + String activeNvramPath = resource.getUefiNvramPath(cmd.getVmUuid()); + if (StringUtils.isBlank(activeNvramPath)) { + throw new IOException(String.format("Unable to determine the active UEFI NVRAM path for VM [%s].", cmd.getVmName())); + } + + Path snapshotNvramPath = getNvramSnapshotAbsolutePath(cmd.getNvramSnapshotPath(), rootVolumeSnapshot, resource, storagePoolMgr); + if (!Files.exists(snapshotNvramPath)) { + throw new IOException(String.format("Unable to find the UEFI NVRAM snapshot [%s] for VM [%s].", cmd.getNvramSnapshotPath(), cmd.getVmName())); + } + + replaceNvramAtomically(snapshotNvramPath, Path.of(activeNvramPath)); + } + + protected void replaceNvramAtomically(Path snapshotNvramPath, Path activeNvramPath) throws IOException { + Path targetDirectory = activeNvramPath.getParent(); + if (targetDirectory != null) { + Files.createDirectories(targetDirectory); + } + + Path temporaryNvramPath = Files.createTempFile(targetDirectory, activeNvramPath.getFileName().toString(), ".tmp"); + try { + copyNvramSnapshotToTemporaryPath(snapshotNvramPath, temporaryNvramPath); + moveTemporaryNvramIntoPlace(temporaryNvramPath, activeNvramPath); + } finally { + Files.deleteIfExists(temporaryNvramPath); + } + } + + protected void copyNvramSnapshotToTemporaryPath(Path snapshotNvramPath, Path temporaryNvramPath) throws IOException { + Files.copy(snapshotNvramPath, temporaryNvramPath, StandardCopyOption.REPLACE_EXISTING); + } + + protected void moveTemporaryNvramIntoPlace(Path temporaryNvramPath, Path activeNvramPath) throws IOException { + try { + Files.move(temporaryNvramPath, activeNvramPath, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + Files.move(temporaryNvramPath, activeNvramPath, StandardCopyOption.REPLACE_EXISTING); + } + } + + protected Path getNvramSnapshotAbsolutePath(String nvramSnapshotPath, SnapshotObjectTO rootVolumeSnapshot, LibvirtComputingResource resource, + KVMStoragePoolManager storagePoolMgr) throws IOException, LibvirtException { + if (rootVolumeSnapshot == null) { + throw new IOException("Unable to locate the root volume snapshot while handling the UEFI NVRAM state."); + } + + KVMStoragePool storagePool = resource.getLibvirtUtilitiesHelper().getPrimaryPoolFromDataTo(rootVolumeSnapshot, storagePoolMgr); + return Path.of(storagePool.getLocalPathFor(nvramSnapshotPath)); + } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java index 8a9d69c97954..b3b55f484e4d 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java @@ -54,9 +54,9 @@ public static String RBDStringBuilder(KVMStoragePool storagePool, String image) rbdOpts += ":mon_host=" + composeOptionForMonHosts(monHost, monPort); if (authUserName == null) { - rbdOpts += ":auth_supported=none"; + rbdOpts += ":auth_client_required=none"; } else { - rbdOpts += ":auth_supported=cephx"; + rbdOpts += ":auth_client_required=cephx"; rbdOpts += ":id=" + authUserName; rbdOpts += ":key=" + authSecret; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index bc82744dd857..1fba9f3e96f0 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -864,7 +864,7 @@ private Answer createTemplateFromVolumeOrSnapshot(CopyCommand cmd) { if (path == null) { path = srcData.getPath(); if (path == null) { - new CloudRuntimeException("The 'path' or 'iqn' field must be specified."); + throw new CloudRuntimeException("The 'path' or 'iqn' field must be specified."); } } } @@ -2339,33 +2339,36 @@ private CreateObjectAnswer takeClvmVolumeSnapshotOfStoppedVm(KVMPhysicalDisk dis * barriers properly (>2.6.32) this won't be any different then pulling the power * cord out of a running machine. */ - private Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhysicalDisk disk, String snapshotName) { + protected Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMPhysicalDisk disk, String snapshotName) { Long snapshotSize = null; + Rados r = null; + IoCTX io = null; + Rbd rbd = null; + RbdImage image = null; try { - Rados r = radosConnect(primaryPool); + r = radosConnect(primaryPool); - final IoCTX io = r.ioCtxCreate(primaryPool.getSourceDir()); - final Rbd rbd = new Rbd(io); - final RbdImage image = rbd.open(disk.getName()); + io = r.ioCtxCreate(primaryPool.getSourceDir()); + rbd = new Rbd(io); + image = rbd.open(disk.getName()); logger.debug("Attempting to create RBD snapshot {}@{}", disk.getName(), snapshotName); image.snapCreate(snapshotName); - image.snapCreate(snapshotName); long rbdSnapshotSize = getRbdSnapshotSize(primaryPool.getSourceDir(), disk.getName(), snapshotName, primaryPool.getSourceHost(), primaryPool.getAuthUserName(), primaryPool.getAuthSecret()); if (rbdSnapshotSize > 0) { snapshotSize = rbdSnapshotSize; } - - rbd.close(image); - r.ioCtxDestroy(io); } catch (final Exception e) { logger.error("A RBD snapshot operation on [{}] failed. The error was: {}", disk.getName(), e.getMessage(), e); + } finally { + closeRbdImage(rbd, image, disk.getName()); + destroyRadosIoCtx(r, io, disk.getName()); } return snapshotSize; } - private long getRbdSnapshotSize(String poolPath, String diskName, String snapshotName, String rbdMonitor, String authUser, String authSecret) { + protected long getRbdSnapshotSize(String poolPath, String diskName, String snapshotName, String rbdMonitor, String authUser, String authSecret) { logger.debug("Get RBD snapshot size for {}/{}@{}", poolPath, diskName, snapshotName); //cmd: rbd du /@ --format json --mon-host --id --key 2>/dev/null String snapshotDetailsInJson = Script.runSimpleBashScript(String.format("rbd du %s/%s@%s --format json --mon-host %s --id %s --key %s 2>/dev/null", poolPath, diskName, snapshotName, rbdMonitor, authUser, authSecret)); @@ -2652,7 +2655,7 @@ protected boolean isAvailablePoolSizeDividedByDiskSizeLesserThanMinRate(long ava return ((availablePoolSize * 1d) / (diskSize * 1d)) < MIN_RATE_BETWEEN_AVAILABLE_POOL_AND_DISK_SIZE_TO_TAKE_DISK_SNAPSHOT; } - private Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException { + protected Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException { Rados r = new Rados(primaryPool.getAuthUserName()); r.confSet(CEPH_MON_HOST, primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort()); r.confSet(CEPH_AUTH_KEY, primaryPool.getAuthSecret()); @@ -2662,6 +2665,50 @@ private Rados radosConnect(final KVMStoragePool primaryPool) throws RadosExcepti return r; } + /** + * Closes an RBD image if it was opened; never throws. An image left open keeps this client's RBD + * exclusive-lock, which later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS and keeps + * the image busy so it cannot be removed. + */ + protected void closeRbdImage(Rbd rbd, RbdImage image, String imageName) { + if (image == null) { + return; + } + try { + rbd.close(image); + } catch (final Exception e) { + logger.warn("Failed to close RBD image [{}]. The error was: {}", imageName, e.getMessage(), e); + } + } + + /** Destroys a RADOS IO context if it was created; never throws. */ + protected void destroyRadosIoCtx(Rados r, IoCTX io, String contextDescription) { + if (io == null) { + return; + } + try { + r.ioCtxDestroy(io); + } catch (final Exception e) { + logger.warn("Failed to destroy the RADOS IO context used for [{}]. The error was: {}", contextDescription, e.getMessage(), e); + } + } + + /** + * Unprotects an RBD snapshot if it was protected; never throws. A snapshot left protected cannot + * be deleted, and neither can its volume. + */ + protected void unprotectRbdSnapshot(RbdImage image, String snapshotName, boolean snapProtected) { + if (!snapProtected) { + return; + } + try { + image.snapUnprotect(snapshotName); + } catch (final Exception e) { + logger.error("Failed to unprotect RBD snapshot [{}]; it and its volume cannot be deleted until this is resolved manually. The error was: {}", + snapshotName, e.getMessage(), e); + } + } + @Override public Answer deleteVolume(final DeleteCommand cmd) { final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData(); @@ -2785,7 +2832,7 @@ private KVMPhysicalDisk createVolumeFromSnapshotOnNFS(CopyCommand cmd, PrimaryDa if (path == null) { path = details != null ? details.get(DiskTO.IQN) : null; if (path == null) { - new CloudRuntimeException("The 'path' or 'iqn' field must be specified."); + logger.warn("The 'path' or 'iqn' field must be specified."); } } } @@ -2811,17 +2858,24 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S disk.setSize(size > volume.getVirtualSize() ? size : volume.getVirtualSize()); disk.setVirtualSize(size > volume.getVirtualSize() ? size : disk.getSize()); + Rados r = null; + IoCTX io = null; + Rbd rbd = null; + RbdImage srcImage = null; + RbdImage diskImage = null; + boolean snapProtected = false; + try { - Rados r = new Rados(srcPool.getAuthUserName()); + r = new Rados(srcPool.getAuthUserName()); r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); r.confSet("key", srcPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); r.connect(); - IoCTX io = r.ioCtxCreate(srcPool.getSourceDir()); - Rbd rbd = new Rbd(io); - RbdImage srcImage = rbd.open(volume.getName()); + io = r.ioCtxCreate(srcPool.getSourceDir()); + rbd = new Rbd(io); + srcImage = rbd.open(volume.getName()); List snaps = srcImage.snapList(); boolean snapFound = false; @@ -2837,23 +2891,26 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S return null; } srcImage.snapProtect(snapshotName); + snapProtected = true; logger.debug(String.format("Try to clone snapshot %s on RBD", snapshotName)); rbd.clone(volume.getName(), snapshotName, io, disk.getName(), LibvirtStorageAdaptor.RBD_FEATURES, 0); - RbdImage diskImage = rbd.open(disk.getName()); + diskImage = rbd.open(disk.getName()); if (disk.getVirtualSize() > volume.getVirtualSize()) { diskImage.resize(disk.getVirtualSize()); } diskImage.flatten(); - rbd.close(diskImage); - - srcImage.snapUnprotect(snapshotName); - rbd.close(srcImage); - r.ioCtxDestroy(io); } catch (RadosException | RbdException e) { logger.error(String.format("Failed due to %s", e.getMessage()), e); disk = null; + } finally { + // Every handle has to be released on all paths, including the "snapshot not found" return and + // any failure of clone/resize/flatten. + closeRbdImage(rbd, diskImage, newUuid); + unprotectRbdSnapshot(srcImage, snapshotName, snapProtected); + closeRbdImage(rbd, srcImage, volume.getName()); + destroyRadosIoCtx(r, io, snapshotName); } return disk; diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index 6c21065340cd..059f4f8b67af 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -1348,6 +1348,8 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, */ KVMStoragePool srcPool = template.getPool(); + Map destDetails = destPool.getDetails(); + String dataPool = (destDetails == null) ? null : destDetails.get(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL); KVMPhysicalDisk disk = null; String newUuid = name; @@ -1396,6 +1398,10 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, r.confSet("mon_host", srcPool.getSourceHost() + ":" + srcPool.getSourcePort()); r.confSet("key", srcPool.getAuthSecret()); r.confSet("client_mount_timeout", "30"); + if (dataPool != null) { + logger.debug("Setting RBD data pool to " + dataPool + " for the new image " + disk.getName()); + r.confSet(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL, dataPool); + } r.connect(); logger.debug("Successfully connected to Ceph cluster at " + r.confGet("mon_host")); @@ -1474,6 +1480,10 @@ private KVMPhysicalDisk createDiskFromTemplateOnRBD(KVMPhysicalDisk template, rDest.confSet("mon_host", destPool.getSourceHost() + ":" + destPool.getSourcePort()); rDest.confSet("key", destPool.getAuthSecret()); rDest.confSet("client_mount_timeout", "30"); + if (dataPool != null) { + logger.debug("Setting RBD data pool to " + dataPool + " on the destination cluster for the new image " + disk.getName()); + rDest.confSet(KVMPhysicalDisk.RBD_DEFAULT_DATA_POOL, dataPool); + } rDest.connect(); logger.debug("Successfully connected to source Ceph cluster at " + rDest.confGet("mon_host")); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java index 514e412559cc..13dab6942f8b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java @@ -228,21 +228,21 @@ private boolean connectPhysicalDisk(AddressInfo address, KVMStoragePool pool, Ma @Override public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool) { - if (LOGGER.isDebugEnabled()) LOGGER.debug(String.format("disconnectPhysicalDisk(volumePath,pool) called with args (%s, %s) START", volumePath, pool.getUuid())); + if (LOGGER.isDebugEnabled()) LOGGER.debug("disconnectPhysicalDisk(volumePath,pool) called with args ({}, {}) START", volumePath, pool.getUuid()); AddressInfo address = this.parseAndValidatePath(volumePath); if (address.getAddress() == null) { - if (LOGGER.isDebugEnabled()) LOGGER.debug(String.format("disconnectPhysicalDisk(volumePath,pool) returning FALSE, volume path has no address field", volumePath, pool.getUuid())); + if (LOGGER.isDebugEnabled()) LOGGER.debug("disconnectPhysicalDisk(volumePath,pool) called with args ({}, {}) returning FALSE, volume path has no address field", volumePath, pool.getUuid()); return false; } ScriptResult result = runScript(disconnectScript, 60000L, address.getAddress().toLowerCase()); if (result.getExitCode() != 0) { - LOGGER.warn(String.format("Disconnect failed for path [%s] with return code [%s]", address.getAddress().toLowerCase(), result.getExitCode())); + LOGGER.warn("Disconnect failed for path {} with return code {}", address.getAddress().toLowerCase(), result.getExitCode()); } if (LOGGER.isDebugEnabled()) { - LOGGER.debug("multipath flush output: " + result.getResult()); - LOGGER.debug(String.format("disconnectPhysicalDisk(volumePath,pool) called with args (%s, %s) COMPLETE [rc=%s]", volumePath, pool.getUuid(), result.getResult())); + LOGGER.debug("multipath flush output: {}", result.getResult()); + LOGGER.debug("disconnectPhysicalDisk(volumePath,pool) called with args ({}, {}) COMPLETE [rc={}]", volumePath, pool.getUuid(), result.getResult()); } return (result.getExitCode() == 0); @@ -250,7 +250,7 @@ public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool) { @Override public boolean disconnectPhysicalDisk(Map volumeToDisconnect) { - LOGGER.debug(String.format("disconnectPhysicalDisk(volumeToDisconnect) called with arg bag [not implemented]:") + " " + volumeToDisconnect); + LOGGER.debug("disconnectPhysicalDisk(volumeToDisconnect) called with arg bag [not implemented]: {}", volumeToDisconnect); return false; } diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java index db665c75cc13..d8e5bed2ef09 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/linux/KVMHostInfo.java @@ -140,21 +140,21 @@ private static long getCpuSpeedFromCommandLscpu() { long speed = 0L; LOGGER.info("Fetching CPU speed from command \"lscpu\"."); try { - String command = "lscpu | grep -i 'Model name' | head -n 1 | egrep -o '[[:digit:]].[[:digit:]]+GHz' | sed 's/GHz//g'"; - if(isHostS390x()) { - command = "lscpu | grep 'CPU dynamic MHz' | cut -d ':' -f 2 | tr -d ' ' | awk '{printf \"%.1f\\n\", $1 / 1000}'"; - } + String command = "lscpu | grep -i 'CPU max MHz' | head -n 1 | sed 's/^.*: //' | xargs"; String result = Script.runSimpleBashScript(command); - speed = (long) (Float.parseFloat(result) * 1000); + speed = (long) (Float.parseFloat(result)); LOGGER.info(String.format("Command [%s] resulted in the value [%s] for CPU speed.", command, speed)); return speed; } catch (NullPointerException | NumberFormatException e) { LOGGER.error(String.format("Unable to retrieve the CPU speed from lscpu."), e); } try { - String command = "lscpu | grep -i 'CPU max MHz' | head -n 1 | sed 's/^.*: //' | xargs"; + String command = "lscpu | grep -i 'Model name' | head -n 1 | egrep -o '[[:digit:]].[[:digit:]]+GHz' | sed 's/GHz//g'"; + if(isHostS390x()) { + command = "lscpu | grep 'CPU dynamic MHz' | cut -d ':' -f 2 | tr -d ' ' | awk '{printf \"%.1f\\n\", $1 / 1000}'"; + } String result = Script.runSimpleBashScript(command); - speed = (long) (Float.parseFloat(result)); + speed = (long) (Float.parseFloat(result) * 1000); LOGGER.info(String.format("Command [%s] resulted in the value [%s] for CPU speed.", command, speed)); return speed; } catch (NullPointerException | NumberFormatException e) { diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java index aaebd8c7aebc..c59ebe0f4197 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java @@ -83,6 +83,7 @@ import org.joda.time.Duration; import org.junit.Assert; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.libvirt.Connect; @@ -5642,6 +5643,7 @@ public void testAddExtraConfigComponentNotEmptyExtraConfig() { Mockito.verify(vmDef, times(1)).addComp(any()); } + @Ignore public void validateGetCurrentMemAccordingToMemBallooningWithoutMemBalooning(){ VirtualMachineTO vmTo = Mockito.mock(VirtualMachineTO.class); Mockito.when(vmTo.getType()).thenReturn(Type.User); @@ -5727,6 +5729,7 @@ public void validateCountDomainRunningVcpus() throws LibvirtException{ Assert.assertEquals(valueExpected, result); } + @Ignore public void setDiskIoDriverTestIoUring() { DiskDef diskDef = configureAndTestSetDiskIoDriverTest(HYPERVISOR_LIBVIRT_VERSION_SUPPORTS_IOURING, HYPERVISOR_QEMU_VERSION_SUPPORTS_IOURING); Assert.assertEquals(IoDriverPolicy.IO_URING, diskDef.getIoDriver()); diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDiskOnlyVMSnapshotCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDiskOnlyVMSnapshotCommandWrapperTest.java new file mode 100644 index 000000000000..e447e6f6cfdb --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtDiskOnlyVMSnapshotCommandWrapperTest.java @@ -0,0 +1,816 @@ +/* + * 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.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +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.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.UUID; + +import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; +import org.apache.cloudstack.storage.to.SnapshotObjectTO; +import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.junit.Test; +import org.libvirt.Domain; +import org.libvirt.DomainInfo; + +import com.cloud.agent.api.VMSnapshotTO; +import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.CreateDiskOnlyVmSnapshotAnswer; +import com.cloud.agent.api.storage.DeleteDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.storage.RevertDiskOnlyVmSnapshotCommand; +import com.cloud.agent.api.to.DataTO; +import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; +import com.cloud.storage.Storage; +import com.cloud.storage.Volume; +import com.cloud.utils.Pair; + +public class LibvirtDiskOnlyVMSnapshotCommandWrapperTest { + + @Test + public void testBackupNvramIfNeededCopiesActiveNvram() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO vmSnapshotTO = mock(VMSnapshotTO.class); + PrimaryDataStoreTO dataStoreTO = mock(PrimaryDataStoreTO.class); + VolumeObjectTO rootVolume = mock(VolumeObjectTO.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + Files.writeString(activeNvram, "snapshot-nvram"); + Path poolDirectory = Files.createTempDirectory("pool-"); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getTarget()).thenReturn(vmSnapshotTO); + when(vmSnapshotTO.getId()).thenReturn(42L); + when(command.getVolumeTOs()).thenReturn(List.of(rootVolume)); + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(rootVolume.getDataStore()).thenReturn(dataStoreTO); + when(dataStoreTO.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(dataStoreTO.getUuid()).thenReturn("pool-uuid"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getStoragePoolMgr()).thenReturn(storagePoolManager); + when(storagePoolManager.getStoragePool(Storage.StoragePoolType.NetworkFilesystem, "pool-uuid")).thenReturn(storagePool); + when(storagePool.getLocalPathFor(anyString())).thenAnswer(invocation -> poolDirectory.resolve(invocation.getArgument(0, String.class)).toString()); + + String nvramSnapshotPath = wrapper.backupNvramIfNeeded(command, resource); + + assertEquals(".cloudstack-vm-snapshot-nvram/42.fd", nvramSnapshotPath); + assertEquals("snapshot-nvram", Files.readString(poolDirectory.resolve(nvramSnapshotPath))); + } + + @Test(expected = IOException.class) + public void testBackupNvramIfNeededFailsWhenUefiNvramIsMissing() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn("/tmp/" + UUID.randomUUID() + ".fd"); + + wrapper.backupNvramIfNeeded(command, resource); + } + + @Test(expected = IOException.class) + public void testValidateNvramRevertStateFailsForLegacySnapshotsOnUefiVms() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.isUefiEnabled()).thenReturn(true); + when(command.getNvramSnapshotPath()).thenReturn(null); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + + wrapper.validateNvramRevertState(command, resource, null, mock(KVMStoragePoolManager.class)); + } + + @Test(expected = IOException.class) + public void testValidateNvramRevertStateFailsForLegacySnapshotsOnFallbackHostsForUefiVms() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.isUefiEnabled()).thenReturn(true); + when(command.getNvramSnapshotPath()).thenReturn(null); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(Path.of("/tmp", UUID.randomUUID() + ".fd").toString()); + + wrapper.validateNvramRevertState(command, resource, null, mock(KVMStoragePoolManager.class)); + } + + @Test + public void testValidateNvramRevertStateAllowsFallbackHostsWithoutLocalNvram() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.isUefiEnabled()).thenReturn(true); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(poolDirectory.resolve("missing").resolve("vm-uuid.fd").toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.validateNvramRevertState(command, resource, rootSnapshot, storagePoolManager); + } + + @Test + public void testRestoreNvramIfNeededRestoresSnapshotBytes() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + Files.writeString(activeNvram, "current"); + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.restoreNvramIfNeeded(command, resource, rootSnapshot, storagePoolManager); + + assertEquals("snapshot", Files.readString(activeNvram)); + } + + @Test + public void testRestoreNvramIfNeededCreatesMissingActiveNvramFile() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Path activeNvram = poolDirectory.resolve("target").resolve("vm-uuid.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.restoreNvramIfNeeded(command, resource, rootSnapshot, storagePoolManager); + + assertEquals("snapshot", Files.readString(activeNvram)); + } + + @Test + public void testRestoreNvramIfNeededPreservesActiveNvramWhenCopyFails() throws Exception { + LibvirtRevertDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtRevertDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected void copyNvramSnapshotToTemporaryPath(Path snapshotNvramPath, Path temporaryNvramPath) throws IOException { + Files.writeString(temporaryNvramPath, "partial"); + throw new IOException("copy failed"); + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + RevertDiskOnlyVmSnapshotCommand command = mock(RevertDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + + Path activeNvram = Files.createTempFile("active-", ".fd"); + Files.writeString(activeNvram, "current"); + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getVmUuid()).thenReturn("vm-uuid"); + when(command.getVmName()).thenReturn("vm-name"); + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(resource.getUefiNvramPath("vm-uuid")).thenReturn(activeNvram.toString()); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + try { + wrapper.restoreNvramIfNeeded(command, resource, rootSnapshot, storagePoolManager); + fail("Expected restore to fail when the snapshot copy fails."); + } catch (IOException expected) { + assertEquals("current", Files.readString(activeNvram)); + } + } + + @Test + public void testDeleteNvramSnapshotIfNeededDeletesSidecar() throws Exception { + LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + DeleteDiskOnlyVmSnapshotCommand command = mock(DeleteDiskOnlyVmSnapshotCommand.class); + SnapshotObjectTO rootSnapshot = mock(SnapshotObjectTO.class); + VolumeObjectTO rootVolume = mock(VolumeObjectTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(rootSnapshot.getVolume()).thenReturn(rootVolume); + when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(rootSnapshot, storagePoolManager)).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.deleteNvramSnapshotIfNeeded(command, resource, storagePoolManager, List.of(rootSnapshot)); + + assertFalse(Files.exists(snapshotNvram)); + assertTrue(Files.exists(poolDirectory)); + } + + @Test + public void testDeleteNvramSnapshotIfNeededDeletesSidecarUsingPrimaryDataStore() throws Exception { + LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtDeleteDiskOnlyVMSnapshotCommandWrapper(); + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + KVMStoragePoolManager storagePoolManager = mock(KVMStoragePoolManager.class); + KVMStoragePool storagePool = mock(KVMStoragePool.class); + DeleteDiskOnlyVmSnapshotCommand command = mock(DeleteDiskOnlyVmSnapshotCommand.class); + PrimaryDataStoreTO primaryDataStoreTO = mock(PrimaryDataStoreTO.class); + + Path poolDirectory = Files.createTempDirectory("pool-"); + Path snapshotNvram = poolDirectory.resolve("nvram/42.fd"); + Files.createDirectories(snapshotNvram.getParent()); + Files.writeString(snapshotNvram, "snapshot"); + + when(command.getNvramSnapshotPath()).thenReturn("nvram/42.fd"); + when(command.getPrimaryDataStore()).thenReturn(primaryDataStoreTO); + when(primaryDataStoreTO.getPoolType()).thenReturn(Storage.StoragePoolType.NetworkFilesystem); + when(primaryDataStoreTO.getUuid()).thenReturn("pool-uuid"); + when(storagePoolManager.getStoragePool(Storage.StoragePoolType.NetworkFilesystem, "pool-uuid")).thenReturn(storagePool); + when(storagePool.getLocalPathFor("nvram/42.fd")).thenReturn(snapshotNvram.toString()); + + wrapper.deleteNvramSnapshotIfNeeded(command, resource, storagePoolManager, List.of()); + + assertFalse(Files.exists(snapshotNvram)); + } + + @Test + public void testResumeVmIfNeededOnlyResumesWhenWrapperSuspendedVm() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + Domain domain = mock(Domain.class); + + wrapper.resumeVmIfNeeded(domain, "vm-name", false); + + verify(domain, never()).resume(); + verify(domain, never()).getInfo(); + } + + @Test + public void testSuspendVmIfNeededSkipsAlreadyPausedVm() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + Domain domain = mock(Domain.class); + DomainInfo domainInfo = new DomainInfo(); + domainInfo.state = DomainInfo.DomainState.VIR_DOMAIN_PAUSED; + when(domain.getInfo()).thenReturn(domainInfo); + + assertFalse(wrapper.suspendVmIfNeeded(domain)); + verify(domain, never()).suspend(); + } + + @Test + public void testShouldSuspendVmForSnapshotWhenUefiAndNotQuiesced() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getTarget()).thenReturn(target); + when(target.getQuiescevm()).thenReturn(false); + + assertTrue(wrapper.shouldSuspendVmForSnapshot(command)); + } + + @Test + public void testShouldSuspendVmForSnapshotWhenQuiesceIsRequested() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getTarget()).thenReturn(target); + when(target.getQuiescevm()).thenReturn(true); + + assertTrue(wrapper.shouldSuspendVmForSnapshot(command)); + } + + @Test + public void testShouldFreezeVmFilesystemsForSnapshotWhenQuiesceIsRequested() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.isUefiEnabled()).thenReturn(true); + when(command.getTarget()).thenReturn(target); + when(target.getQuiescevm()).thenReturn(true); + + assertTrue(wrapper.shouldFreezeVmFilesystemsForSnapshot(command)); + } + + @Test + public void testGetFlagsToUseForRunningVmSnapshotCreationOmitsLibvirtQuiesceWhenAlreadyFrozen() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper(); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(target.getQuiescevm()).thenReturn(true); + + int flags = wrapper.getFlagsToUseForRunningVmSnapshotCreation(target, true); + + assertEquals(0, flags & Domain.SnapshotCreateFlags.QUIESCE); + assertTrue((flags & Domain.SnapshotCreateFlags.DISK_ONLY) != 0); + assertTrue((flags & Domain.SnapshotCreateFlags.ATOMIC) != 0); + assertTrue((flags & Domain.SnapshotCreateFlags.NO_METADATA) != 0); + } + + @Test + public void testFreezeAndVerifyVmFilesystemsSucceedsWhenGuestAgentReportsFrozen() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected String getResultOfQemuCommand(String cmd, Domain domain) { + if ("status".equals(cmd)) { + return "{\"return\":\"frozen\"}"; + } + return "{\"return\":0}"; + } + }; + + Domain domain = mock(Domain.class); + wrapper.freezeVmFilesystems(domain, "vm-name"); + wrapper.verifyVmFilesystemsFrozen(domain, "vm-name"); + } + + @Test + public void testFreezeVmFilesystemsFailsForQemuErrorResponse() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected String getResultOfQemuCommand(String cmd, Domain domain) { + return "{\"error\":{\"class\":\"GenericError\",\"desc\":\"guest agent failure\"}}"; + } + }; + + try { + wrapper.freezeVmFilesystems(mock(Domain.class), "vm-name"); + fail("QEMU guest agent error responses must be treated as freeze failures."); + } catch (IOException e) { + assertTrue(e.getMessage().contains("Failed to freeze VM [vm-name] filesystems")); + } + } + + @Test + public void testThawVmFilesystemsFailsForQemuErrorResponse() { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected String getResultOfQemuCommand(String cmd, Domain domain) { + return "{\"error\":{\"class\":\"GenericError\",\"desc\":\"guest agent failure\"}}"; + } + }; + + assertFalse(wrapper.thawVmFilesystemsIfNeeded(mock(Domain.class), "vm-name")); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmThawsWhenFreezeVerificationFails() throws Exception { + final boolean[] thawCalled = {false}; + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return true; + } + + @Override + protected void freezeVmFilesystems(Domain domain, String vmName) { + } + + @Override + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) throws IOException { + throw new IOException("status verification failed"); + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + thawCalled[0] = true; + return true; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertFalse(answer.getResult()); + assertTrue("Thaw must be attempted after a successful freeze followed by verification failure", thawCalled[0]); + } + + @Test + public void testVerifyVmFilesystemsFrozenFailsForQemuErrorResponse() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected String getResultOfQemuCommand(String cmd, Domain domain) { + return "{\"error\":{\"class\":\"GenericError\",\"desc\":\"guest agent failure\"}}"; + } + }; + + try { + wrapper.verifyVmFilesystemsFrozen(mock(Domain.class), "vm-name"); + fail("QEMU guest agent error responses must be treated as IO failures."); + } catch (IOException e) { + assertTrue(e.getMessage().contains("Failed to verify VM [vm-name] filesystem freeze state")); + } + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmReturnsFailureAnswerWhenFreezeStatusJsonIsMalformed() throws Exception { + final boolean[] thawCalled = {false}; + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return true; + } + + @Override + protected void freezeVmFilesystems(Domain domain, String vmName) { + } + + @Override + protected String getResultOfQemuCommand(String cmd, Domain domain) { + return "not-json"; + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + thawCalled[0] = true; + return true; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertFalse(answer.getResult()); + assertTrue(answer.getDetails().contains("Failed to verify VM [vm-name] filesystem freeze state")); + assertTrue("Thaw must be attempted when freeze verification fails on malformed JSON", thawCalled[0]); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmSuspendsBeforeNvramCopyForQuiescedUefiSnapshots() throws Exception { + List operations = new ArrayList<>(); + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected void freezeVmFilesystems(Domain domain, String vmName) { + operations.add("freeze"); + } + + @Override + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) { + operations.add("verify"); + } + + @Override + protected boolean suspendVmIfNeeded(Domain domain) { + operations.add("suspend"); + return true; + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { + operations.add("backup"); + return "nvram/42.fd"; + } + + @Override + protected void cleanupNvramSnapshotIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource, String nvramSnapshotPath) { + } + + @Override + protected boolean resumeVmIfNeeded(Domain domain, String vmName) { + operations.add("resume"); + return true; + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + operations.add("thaw"); + return true; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(command.isUefiEnabled()).thenReturn(true); + when(target.getQuiescevm()).thenReturn(true); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + doThrow(mock(org.libvirt.LibvirtException.class)).when(domain).snapshotCreateXML(anyString(), anyInt()); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertFalse(answer.getResult()); + assertEquals(List.of("freeze", "verify", "suspend", "backup", "resume", "thaw"), operations); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmReturnsSuccessWithWarningWhenThawFails() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return true; + } + + @Override + protected void freezeVmFilesystems(Domain domain, String vmName) { + } + + @Override + protected void verifyVmFilesystemsFrozen(Domain domain, String vmName) { + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { + return null; + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName) { + return false; + } + + @Override + protected boolean thawVmFilesystemsIfNeeded(Domain domain, String vmName, boolean frozen) { + return false; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertTrue("Snapshot metadata must still be returned when thaw fails after snapshot creation", answer.getResult()); + assertTrue(answer.getDetails().contains("could not be thawed")); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmReturnsSuccessWithWarningWhenResumeFails() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return true; + } + + @Override + protected boolean suspendVmIfNeeded(Domain domain) { + return true; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) { + return null; + } + + @Override + protected boolean resumeVmIfNeeded(Domain domain, String vmName) { + return false; + } + + @Override + protected boolean resumeVmIfNeeded(Domain domain, String vmName, boolean suspended) { + return false; + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertTrue("Snapshot metadata must still be returned when resume fails after snapshot creation", answer.getResult()); + assertTrue(answer.getDetails().contains("could not be resumed")); + } + + @Test + public void testTakeDiskOnlyVmSnapshotOfRunningVmHandlesNullErrorMessage() throws Exception { + LibvirtCreateDiskOnlyVMSnapshotCommandWrapper wrapper = new LibvirtCreateDiskOnlyVMSnapshotCommandWrapper() { + @Override + protected Pair>> createSnapshotXmlAndNewVolumePathMap(List volumeObjectTOS, + List disks, VMSnapshotTO target, LibvirtComputingResource resource) { + return new Pair<>("", Collections.emptyMap()); + } + + @Override + protected boolean shouldSuspendVmForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected boolean shouldFreezeVmFilesystemsForSnapshot(CreateDiskOnlyVmSnapshotCommand cmd) { + return false; + } + + @Override + protected String backupNvramIfNeeded(CreateDiskOnlyVmSnapshotCommand cmd, LibvirtComputingResource resource) throws IOException { + throw new IOException(); + } + }; + LibvirtComputingResource resource = mock(LibvirtComputingResource.class); + LibvirtUtilitiesHelper libvirtUtilitiesHelper = mock(LibvirtUtilitiesHelper.class); + org.libvirt.Connect connect = mock(org.libvirt.Connect.class); + Domain domain = mock(Domain.class); + CreateDiskOnlyVmSnapshotCommand command = mock(CreateDiskOnlyVmSnapshotCommand.class); + VMSnapshotTO target = mock(VMSnapshotTO.class); + + when(command.getVmName()).thenReturn("vm-name"); + when(command.getVolumeTOs()).thenReturn(List.of()); + when(command.getTarget()).thenReturn(target); + when(command.isUefiEnabled()).thenReturn(true); + when(resource.getLibvirtUtilitiesHelper()).thenReturn(libvirtUtilitiesHelper); + when(libvirtUtilitiesHelper.getConnection()).thenReturn(connect); + when(resource.getDisks(connect, "vm-name")).thenReturn(List.of()); + when(resource.getDomain(connect, "vm-name")).thenReturn(domain); + + CreateDiskOnlyVmSnapshotAnswer answer = (CreateDiskOnlyVmSnapshotAnswer) wrapper.takeDiskOnlyVmSnapshotOfRunningVm(command, resource); + + assertFalse(answer.getResult()); + } +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java index f028035e8dcc..8c67640eb1f6 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRestoreBackupCommandWrapperTest.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.hypervisor.kvm.resource.wrapper; +import static org.mockito.AdditionalMatchers.aryEq; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyList; import static org.mockito.ArgumentMatchers.anyLong; @@ -25,9 +26,11 @@ import static org.mockito.Mockito.when; import java.io.IOException; +import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.List; import org.apache.cloudstack.backup.BackupAnswer; import org.apache.cloudstack.backup.RestoreBackupCommand; @@ -42,8 +45,11 @@ import com.cloud.agent.api.Answer; import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource; +import com.cloud.hypervisor.kvm.storage.KVMStoragePool; +import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager; import com.cloud.storage.Storage; import com.cloud.utils.Pair; +import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; import com.cloud.vm.VirtualMachine; @@ -261,8 +267,8 @@ public void testExecuteWithMountFailure() throws Exception { filesMock.when(() -> Files.createTempDirectory(anyString())).thenReturn(tempPath); try (MockedStatic