kvm: restore backups into the volume they were taken from

.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

.github/linters/codespell.txt

@@ -187,6 +187,7 @@ environmnet
 equivalant
 erro
 erronous
+errorprone
 everthing
 everytime
 excute

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.

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) {

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";

api/src/main/java/com/cloud/resource/ResourceService.java

@@ -91,7 +91,7 @@ public interface ResourceService {
 
     DataCenter getZone(Long zoneId);
 
-    List<HypervisorType> getSupportedHypervisorTypes(long zoneId, boolean forVirtualRouter, Long podId);
+    List<HypervisorType> getSupportedHypervisorTypes(long zoneId, boolean forSystemVm, Long podId);
 
     boolean releaseHostReservation(Long hostId);
 

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;
 

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";

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);

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;

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 {
 

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;
     }

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<ServiceResponse> 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;
+    }
 }

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

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

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);
+    }
+}

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"));
+    }
+}

core/src/main/java/com/cloud/agent/api/storage/CreateDiskOnlyVmSnapshotAnswer.java

@@ -27,13 +27,24 @@
 public class CreateDiskOnlyVmSnapshotAnswer extends Answer {
 
     protected Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath;
+    private String nvramSnapshotPath;
 
     public CreateDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath) {
+        this(command, success, details, mapVolumeToSnapshotSizeAndNewVolumePath, null);
+    }
+
+    public CreateDiskOnlyVmSnapshotAnswer(Command command, boolean success, String details, Map<String, Pair<Long, String>> mapVolumeToSnapshotSizeAndNewVolumePath,
+            String nvramSnapshotPath) {
         super(command, success, details);
         this.mapVolumeToSnapshotSizeAndNewVolumePath = mapVolumeToSnapshotSizeAndNewVolumePath;
+        this.nvramSnapshotPath = nvramSnapshotPath;
     }
 
     public Map<String, Pair<Long, String>> getMapVolumeToSnapshotSizeAndNewVolumePath() {
         return mapVolumeToSnapshotSizeAndNewVolumePath;
     }
+
+    public String getNvramSnapshotPath() {
+        return nvramSnapshotPath;
+    }
 }

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<VolumeObjectTO> volumeTOs, String guestOSType, VirtualMachine.State vmState) {
+        this(vmName, null, snapshot, volumeTOs, guestOSType, vmState, false);
+    }
+
+    public CreateDiskOnlyVmSnapshotCommand(String vmName, String vmUuid, VMSnapshotTO snapshot, List<VolumeObjectTO> 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;
+    }
 }

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<DataTO> snapshots;
+    private final List<DataTO> snapshots;
+    private final String nvramSnapshotPath;
+    private final PrimaryDataStoreTO primaryDataStore;
 
     public DeleteDiskOnlyVmSnapshotCommand(List<DataTO> snapshots) {
+        this(snapshots, null);
+    }
+
+    public DeleteDiskOnlyVmSnapshotCommand(List<DataTO> snapshots, String nvramSnapshotPath) {
+        this(snapshots, nvramSnapshotPath, null);
+    }
+
+    public DeleteDiskOnlyVmSnapshotCommand(List<DataTO> snapshots, String nvramSnapshotPath, PrimaryDataStoreTO primaryDataStore) {
         this.snapshots = snapshots;
+        this.nvramSnapshotPath = nvramSnapshotPath;
+        this.primaryDataStore = primaryDataStore;
     }
 
     public List<DataTO> getSnapshots() {
         return snapshots;
     }
 
+    public String getNvramSnapshotPath() {
+        return nvramSnapshotPath;
+    }
+
+    public PrimaryDataStoreTO getPrimaryDataStore() {
+        return primaryDataStore;
+    }
+
     @Override
     public boolean executeInSequence() {
         return false;