From cec6ade257ca231f7bb5a7062ee1ef0739b24e32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Thu, 25 Apr 2024 04:35:25 -0300 Subject: [PATCH] change live migration API used on kvm (#8952) --- .../hypervisor/kvm/resource/LibvirtVMDef.java | 4 + .../kvm/resource/MigrateKVMAsync.java | 45 +++++++++- .../wrapper/LibvirtMigrateCommandWrapper.java | 29 ++++++- .../kvm/resource/MigrateKVMAsyncTest.java | 83 +++++++++++++++++++ .../LibvirtMigrateCommandWrapperTest.java | 78 +++++++++++++++-- 5 files changed, 230 insertions(+), 9 deletions(-) create mode 100644 plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsyncTest.java diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java index 6d69b2f96649..c3b5f9857eb5 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtVMDef.java @@ -972,6 +972,10 @@ public String getDiskLabel() { return _diskLabel; } + public void setDiskLabel(String label) { + _diskLabel = label; + } + public DiskType getDiskType() { return _diskType; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java index c39173142501..bc94bb47ed81 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsync.java @@ -18,13 +18,20 @@ */ package com.cloud.hypervisor.kvm.resource; +import java.util.Iterator; +import java.util.Set; import java.util.concurrent.Callable; +import org.apache.log4j.Logger; import org.libvirt.Connect; import org.libvirt.Domain; import org.libvirt.LibvirtException; +import org.libvirt.TypedParameter; +import org.libvirt.TypedStringParameter; +import org.libvirt.TypedUlongParameter; public class MigrateKVMAsync implements Callable { + protected Logger logger = Logger.getLogger(getClass()); private final LibvirtComputingResource libvirtComputingResource; @@ -37,6 +44,8 @@ public class MigrateKVMAsync implements Callable { private boolean migrateNonSharedInc; private boolean autoConvergence; + protected Set migrateDiskLabels; + // Libvirt Migrate Flags reference: // https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainMigrateFlags @@ -87,7 +96,7 @@ public class MigrateKVMAsync implements Callable { private static final int LIBVIRT_VERSION_SUPPORTS_AUTO_CONVERGE = 1002003; public MigrateKVMAsync(final LibvirtComputingResource libvirtComputingResource, final Domain dm, final Connect dconn, final String dxml, - final boolean migrateStorage, final boolean migrateNonSharedInc, final boolean autoConvergence, final String vmName, final String destIp) { + final boolean migrateStorage, final boolean migrateNonSharedInc, final boolean autoConvergence, final String vmName, final String destIp, Set migrateDiskLabels) { this.libvirtComputingResource = libvirtComputingResource; this.dm = dm; @@ -98,6 +107,7 @@ public MigrateKVMAsync(final LibvirtComputingResource libvirtComputingResource, this.autoConvergence = autoConvergence; this.vmName = vmName; this.destIp = destIp; + this.migrateDiskLabels = migrateDiskLabels; } @Override @@ -121,6 +131,37 @@ public Domain call() throws LibvirtException { flags |= VIR_MIGRATE_AUTO_CONVERGE; } - return dm.migrate(dconn, flags, dxml, vmName, "tcp:" + destIp, libvirtComputingResource.getMigrateSpeed()); + TypedParameter [] parameters = createTypedParameterList(); + + logger.debug(String.format("Migrating [%s] with flags [%s], destination [%s] and speed [%s]. The disks with the following labels will be migrated [%s].", vmName, flags, + destIp, libvirtComputingResource.getMigrateSpeed(), migrateDiskLabels)); + + return dm.migrate(dconn, parameters, flags); + + } + + protected TypedParameter[] createTypedParameterList() { + int sizeOfMigrateDiskLabels = 0; + if (migrateDiskLabels != null) { + sizeOfMigrateDiskLabels = migrateDiskLabels.size(); + } + + TypedParameter[] parameters = new TypedParameter[4 + sizeOfMigrateDiskLabels]; + parameters[0] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_NAME, vmName); + parameters[1] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_XML, dxml); + parameters[2] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_URI, "tcp:" + destIp); + parameters[3] = new TypedUlongParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_BANDWIDTH, libvirtComputingResource.getMigrateSpeed()); + + if (sizeOfMigrateDiskLabels == 0) { + return parameters; + } + + Iterator iterator = migrateDiskLabels.iterator(); + for (int i = 0; i < sizeOfMigrateDiskLabels; i++) { + parameters[4 + i] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_MIGRATE_DISKS, iterator.next()); + } + + return parameters; } + } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java index fb526626ef8f..aebbaa4119da 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapper.java @@ -24,6 +24,7 @@ import java.io.InputStream; import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -190,6 +191,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. // migrateStorage's value should always only be associated with the initial state of mapMigrateStorage. final boolean migrateStorage = MapUtils.isNotEmpty(mapMigrateStorage); final boolean migrateStorageManaged = command.isMigrateStorageManaged(); + Set migrateDiskLabels = null; if (migrateStorage) { if (s_logger.isDebugEnabled()) { @@ -199,6 +201,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. if (s_logger.isDebugEnabled()) { s_logger.debug(String.format("Changed VM [%s] XML configuration of used storage. New XML configuration is [%s].", vmName, xmlDesc)); } + migrateDiskLabels = getMigrateStorageDeviceLabels(disks, mapMigrateStorage); } Map dpdkPortsMapping = command.getDpdkInterfaceMapping(); @@ -227,7 +230,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. final Callable worker = new MigrateKVMAsync(libvirtComputingResource, dm, dconn, xmlDesc, migrateStorage, migrateNonSharedInc, - command.isAutoConvergence(), vmName, command.getDestinationIp()); + command.isAutoConvergence(), vmName, command.getDestinationIp(), migrateDiskLabels); final Future migrateThread = executor.submit(worker); executor.shutdown(); long sleeptime = 0; @@ -365,6 +368,30 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. return new MigrateAnswer(command, result == null, result, null); } + /** + * Gets the disk labels (vda, vdb...) of the disks mapped for migration on mapMigrateStorage. + * @param diskDefinitions list of all the disksDefinitions of the VM. + * @param mapMigrateStorage map of the disks that should be migrated. + * @return set with the labels of the disks that should be migrated. + * */ + protected Set getMigrateStorageDeviceLabels(List diskDefinitions, Map mapMigrateStorage) { + HashSet setOfLabels = new HashSet<>(); + s_logger.debug(String.format("Searching for disk labels of disks [%s].", mapMigrateStorage.keySet())); + for (String fileName : mapMigrateStorage.keySet()) { + for (DiskDef diskDef : diskDefinitions) { + String diskPath = diskDef.getDiskPath(); + if (diskPath != null && diskPath.contains(fileName)) { + setOfLabels.add(diskDef.getDiskLabel()); + s_logger.debug(String.format("Found label [%s] for disk [%s].", diskDef.getDiskLabel(), fileName)); + break; + } + } + } + + return setOfLabels; + } + + /** * Checks if the CPU shares are equal in the source host and destination host. *
    diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsyncTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsyncTest.java new file mode 100644 index 000000000000..28633b925b21 --- /dev/null +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/MigrateKVMAsyncTest.java @@ -0,0 +1,83 @@ +// +// 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; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.libvirt.Connect; +import org.libvirt.Domain; +import org.libvirt.TypedParameter; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import java.util.Set; + +@RunWith(MockitoJUnitRunner.class) +public class MigrateKVMAsyncTest { + + @Mock + private LibvirtComputingResource libvirtComputingResource; + @Mock + private Connect connect; + @Mock + private Domain domain; + + + @Test + public void createTypedParameterListTestNoMigrateDiskLabels() { + MigrateKVMAsync migrateKVMAsync = new MigrateKVMAsync(libvirtComputingResource, domain, connect, "testxml", + false, false, false, "tst", "1.1.1.1", null); + + Mockito.doReturn(10).when(libvirtComputingResource).getMigrateSpeed(); + + TypedParameter[] result = migrateKVMAsync.createTypedParameterList(); + + Assert.assertEquals(4, result.length); + + Assert.assertEquals("tst", result[0].getValueAsString()); + Assert.assertEquals("testxml", result[1].getValueAsString()); + Assert.assertEquals("tcp:1.1.1.1", result[2].getValueAsString()); + Assert.assertEquals("10", result[3].getValueAsString()); + + } + + @Test + public void createTypedParameterListTestWithMigrateDiskLabels() { + Set labels = Set.of("vda", "vdb"); + MigrateKVMAsync migrateKVMAsync = new MigrateKVMAsync(libvirtComputingResource, domain, connect, "testxml", + false, false, false, "tst", "1.1.1.1", labels); + + Mockito.doReturn(10).when(libvirtComputingResource).getMigrateSpeed(); + + TypedParameter[] result = migrateKVMAsync.createTypedParameterList(); + + Assert.assertEquals(6, result.length); + + Assert.assertEquals("tst", result[0].getValueAsString()); + Assert.assertEquals("testxml", result[1].getValueAsString()); + Assert.assertEquals("tcp:1.1.1.1", result[2].getValueAsString()); + Assert.assertEquals("10", result[3].getValueAsString()); + + Assert.assertEquals(labels, Set.of(result[4].getValueAsString(), result[5].getValueAsString())); + } + +} diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java index c206f898e97d..7071758fcbe4 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMigrateCommandWrapperTest.java @@ -26,10 +26,12 @@ import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Scanner; +import java.util.Set; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; @@ -585,6 +587,14 @@ public void setup() throws Exception { " \n" + "\n"; + private Map createMapMigrateStorage(String sourceText, String path) { + Map mapMigrateStorage = new HashMap(); + + MigrateDiskInfo diskInfo = new MigrateDiskInfo("123456", DiskType.BLOCK, DriverType.RAW, Source.FILE, sourceText); + mapMigrateStorage.put(path, diskInfo); + return mapMigrateStorage; + } + @Test public void testReplaceIpForVNCInDescFile() { final String targetIp = "192.168.22.21"; @@ -761,10 +771,8 @@ static void assertXpath(final Document doc, final String xPathExpr, @Test public void testReplaceStorage() throws Exception { - Map mapMigrateStorage = new HashMap(); + Map mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6"); - MigrateDiskInfo diskInfo = new MigrateDiskInfo("123456", DiskType.BLOCK, DriverType.RAW, Source.FILE, "sourctest"); - mapMigrateStorage.put("/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6", diskInfo); final String result = libvirtMigrateCmdWrapper.replaceStorage(fullfile, mapMigrateStorage, true); InputStream in = IOUtils.toInputStream(result); @@ -778,7 +786,6 @@ public void testReplaceStorage() throws Exception { @Test public void testReplaceStorageWithSecrets() throws Exception { - Map mapMigrateStorage = new HashMap(); final String xmlDesc = "" + @@ -799,8 +806,7 @@ public void testReplaceStorageWithSecrets() throws Exception { final String volumeFile = "3530f749-82fd-458e-9485-a357e6e541db"; String newDiskPath = "/mnt/2d0435e1-99e0-4f1d-94c0-bee1f6f8b99e/" + volumeFile; - MigrateDiskInfo diskInfo = new MigrateDiskInfo("123456", DiskType.BLOCK, DriverType.RAW, Source.FILE, newDiskPath); - mapMigrateStorage.put("/mnt/07eb495b-5590-3877-9fb7-23c6e9a40d40/bf8621b3-027c-497d-963b-06319650f048", diskInfo); + Map mapMigrateStorage = createMapMigrateStorage(newDiskPath, "/mnt/07eb495b-5590-3877-9fb7-23c6e9a40d40/bf8621b3-027c-497d-963b-06319650f048"); final String result = libvirtMigrateCmdWrapper.replaceStorage(xmlDesc, mapMigrateStorage, false); final String expectedSecretUuid = LibvirtComputingResource.generateSecretUUIDFromString(volumeFile); @@ -951,4 +957,64 @@ public void updateVmSharesIfNeededTestNewCpuSharesLowerThanCurrentSharesShouldUp Assert.assertEquals(updateShares, newVmCpuShares); } + + @Test + public void getMigrateStorageDeviceLabelsTestNoDiskDefinitions() { + Map mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6"); + + Set result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(new ArrayList<>(), mapMigrateStorage); + + assertTrue(result.isEmpty()); + } + + @Test + public void getMigrateStorageDeviceLabelsTestNoMapMigrateStorage() { + List disks = new ArrayList<>(); + DiskDef diskDef0 = new DiskDef(); + + diskDef0.setDiskPath("volPath"); + disks.add(diskDef0); + + Set result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(disks, new HashMap<>()); + + assertTrue(result.isEmpty()); + } + + @Test + public void getMigrateStorageDeviceLabelsTestPathIsNotFound() { + List disks = new ArrayList<>(); + DiskDef diskDef0 = new DiskDef(); + + diskDef0.setDiskPath("volPath"); + disks.add(diskDef0); + + Map mapMigrateStorage = createMapMigrateStorage("sourceTest", "/mnt/812ea6a3-7ad0-30f4-9cab-01e3f2985b98/4650a2f7-fce5-48e2-beaa-bcdf063194e6"); + + Set result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(disks, mapMigrateStorage); + + assertTrue(result.isEmpty()); + } + + @Test + public void getMigrateStorageDeviceLabelsTestFindPathAndLabels() { + List disks = new ArrayList<>(); + DiskDef diskDef0 = new DiskDef(); + DiskDef diskDef1 = new DiskDef(); + + diskDef0.setDiskPath("volPath1"); + diskDef0.setDiskLabel("vda"); + disks.add(diskDef0); + + diskDef1.setDiskPath("volPath2"); + diskDef1.setDiskLabel("vdb"); + disks.add(diskDef1); + + Map mapMigrateStorage = createMapMigrateStorage("sourceTest", "volPath1"); + mapMigrateStorage.put("volPath2", new MigrateDiskInfo("123457", DiskType.BLOCK, DriverType.RAW, Source.FILE, "sourceText")); + + Set result = libvirtMigrateCmdWrapper.getMigrateStorageDeviceLabels(disks, mapMigrateStorage); + + assertTrue(result.containsAll(Arrays.asList("vda", "vdb"))); + } + }