From 80c3007a1f840ff221797a7a1f3fc3faab05aa9f Mon Sep 17 00:00:00 2001 From: gabriel Date: Sun, 26 Jun 2022 17:03:49 +0200 Subject: [PATCH 1/2] Add virDomainMigrate3 into Libvirt migration. This implementation uses virDomainMigrate3 in order to point which disk device should be migrated. Thus, avoid mapping shared volumes to libvirt when copying disks. This allows to save crucial time (can be even hours depending on all data-disks), bandwidth, and processing. --- .../hypervisor/kvm/resource/LibvirtVMDef.java | 2 +- .../kvm/resource/MigrateKVMAsync.java | 32 ++++++++++++++- .../wrapper/LibvirtMigrateCommandWrapper.java | 22 ++++++++++- .../LibvirtMigrateCommandWrapperTest.java | 39 +++++++++++++++++++ 4 files changed, 91 insertions(+), 4 deletions(-) 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 7c65f7970adc..b63863b44536 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 @@ -573,7 +573,7 @@ public String toString() { } } - enum DiskType { + public enum DiskType { FILE("file"), BLOCK("block"), DIRECTROY("dir"), NETWORK("network"); String _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..c0e7cdfba766 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 @@ -20,9 +20,13 @@ import java.util.concurrent.Callable; +import org.apache.commons.lang3.StringUtils; 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 { @@ -33,6 +37,7 @@ public class MigrateKVMAsync implements Callable { private String dxml = ""; private String vmName = ""; private String destIp = ""; + private String rootDiskDiskDeviceLabel = null; private boolean migrateStorage; private boolean migrateNonSharedInc; private boolean autoConvergence; @@ -85,9 +90,11 @@ public class MigrateKVMAsync implements Callable { // Libvirt 1.2.3 supports auto converge. private static final int LIBVIRT_VERSION_SUPPORTS_AUTO_CONVERGE = 1002003; + private static final String TCP_URI = "tcp:"; 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, + final String rootDiskDiskDeviceLabel) { this.libvirtComputingResource = libvirtComputingResource; this.dm = dm; @@ -98,11 +105,13 @@ public MigrateKVMAsync(final LibvirtComputingResource libvirtComputingResource, this.autoConvergence = autoConvergence; this.vmName = vmName; this.destIp = destIp; + this.rootDiskDiskDeviceLabel = rootDiskDiskDeviceLabel; } @Override public Domain call() throws LibvirtException { long flags = VIR_MIGRATE_LIVE; + String destUri = TCP_URI + destIp; if (dconn.getLibVirVersion() >= LIBVIRT_VERSION_SUPPORTS_MIGRATE_COMPRESSED) { flags |= VIR_MIGRATE_COMPRESSED; @@ -121,6 +130,25 @@ public Domain call() throws LibvirtException { flags |= VIR_MIGRATE_AUTO_CONVERGE; } - return dm.migrate(dconn, flags, dxml, vmName, "tcp:" + destIp, libvirtComputingResource.getMigrateSpeed()); + if (StringUtils.isNotBlank(rootDiskDiskDeviceLabel) && migrateStorage && !migrateNonSharedInc) { + return migrateRootDisk(flags, rootDiskDiskDeviceLabel, destUri); + } + return dm.migrate(dconn, flags, dxml, vmName, destUri, libvirtComputingResource.getMigrateSpeed()); + } + + /** + * Uses libvirt.virDomainMigrate3 in order to map the root volume to be copied when using the flag VIR_MIGRATE_NON_SHARED_DISK. + * VIR_MIGRATE_NON_SHARED_DISK is analagous to "--copy-all" in "virsh". + * Binding into: "virsh migrate i-2-13456-VM qemu:///system --live --compressed --copy-storage-all --migrate-disks vda --xml i-2-13456-VM.xml" + */ + private Domain migrateRootDisk(long flags, String diskDeviceName, String uri) throws LibvirtException { + int nParams = 5; + TypedParameter[] params = new TypedParameter[nParams]; + params[0] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_XML, dxml); + params[1] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_NAME, vmName); + params[2] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_URI, uri); + params[3] = new TypedUlongParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_BANDWIDTH, (long)libvirtComputingResource.getMigrateSpeed()); + params[4] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_MIGRATE_DISKS, diskDeviceName); + return dm.migrate(dconn, params, flags); } } 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 812ca76ca46f..07673a43cb83 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 @@ -188,6 +188,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. final boolean migrateStorage = MapUtils.isNotEmpty(mapMigrateStorage); final boolean migrateStorageManaged = command.isMigrateStorageManaged(); + String rootDiskDiskDeviceLabel = null; if (migrateStorage) { if (s_logger.isDebugEnabled()) { s_logger.debug(String.format("Changing VM [%s] volumes during migration to host: [%s].", vmName, target)); @@ -196,6 +197,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)); } + rootDiskDiskDeviceLabel = retrieveLocalRootDiskDeviceLabel(disks); } Map dpdkPortsMapping = command.getDpdkInterfaceMapping(); @@ -222,7 +224,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(), rootDiskDiskDeviceLabel); final Future migrateThread = executor.submit(worker); executor.shutdown(); long sleeptime = 0; @@ -359,6 +361,24 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. return new MigrateAnswer(command, result == null, result, null); } + /** + * It returns the label of the Root disk in case it is placed in a local storage. + * In such cases, the disk device is of type "File". + * If the Root volume is not in local storage then it returns null. + * Example of output: "sda". + * + * @Note: At the moment of this implementation, CloudStack supports only Root volumes to be placed in Local storage. + * It is not possible to have local data-disk, therefore, only one device of type "Disk" can be a "File". + * Domain's XML contain also "DeviceType.CDROM" of type "DiskDef.DiskType.FILE", which is filtered in this method. + */ + protected String retrieveLocalRootDiskDeviceLabel(List disks) { + DiskDef diskDef = disks.get(0); + if (DiskDef.DiskType.FILE == diskDef.getDiskType() && DiskDef.DeviceType.DISK == diskDef.getDeviceType()) { + return diskDef.getDiskLabel(); + } + return null; + } + /** * Replace DPDK source path and target before migrations */ 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 ae2e4cc41c25..46270929dd5e 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 @@ -791,4 +791,43 @@ public void testReplaceDPDKPorts() throws ParserConfigurationException, IOExcept Assert.assertTrue(replaced.contains("csdpdk-7")); Assert.assertFalse(replaced.contains("csdpdk-1")); } + + @Test + public void discoverLocalRootDiskDeviceLabelTestRootDiskFile() { + String rootLabel = "vda"; + DiskDef rootDisk = mockDisk(DiskDef.DiskType.FILE, DiskDef.DeviceType.DISK); + Mockito.when(rootDisk.getDiskLabel()).thenReturn(rootLabel); + prepareAndTestDiscoverLocalRootDiskDeviceLabel(rootDisk, rootLabel); + } + + @Test + public void discoverLocalRootDiskDeviceLabelTestRootDiskFileNotDisk() { + String rootLabel = "vda"; + String expectedLabel = null; + for (DiskDef.DeviceType deviceType : DiskDef.DeviceType.values()) { + if (DiskDef.DeviceType.DISK != deviceType) { + List disks = new ArrayList<>(); + DiskDef rootDisk = mockDisk(DiskDef.DiskType.FILE, deviceType); + Mockito.when(rootDisk.getDiskLabel()).thenReturn(rootLabel); + prepareAndTestDiscoverLocalRootDiskDeviceLabel(rootDisk, expectedLabel); + } + } + } + + private void prepareAndTestDiscoverLocalRootDiskDeviceLabel(DiskDef rootDisk, String expected) { + List disks = new ArrayList<>(); + disks.add(rootDisk); + disks.add(mockDisk(DiskDef.DiskType.NETWORK, DiskDef.DeviceType.DISK)); + disks.add(mockDisk(DiskDef.DiskType.NETWORK, DiskDef.DeviceType.DISK)); + disks.add(mockDisk(DiskDef.DiskType.BLOCK, DiskDef.DeviceType.DISK)); + String diskLabel = libvirtMigrateCmdWrapper.retrieveLocalRootDiskDeviceLabel(disks); + Assert.assertEquals(expected, diskLabel); + } + + private DiskDef mockDisk(DiskDef.DiskType file, DiskDef.DeviceType diskType) { + DiskDef mockedDisk = Mockito.mock(DiskDef.class); + Mockito.when(mockedDisk.getDiskType()).thenReturn(file); + Mockito.when(mockedDisk.getDeviceType()).thenReturn(diskType); + return mockedDisk; + } } From 55fd42df72423aec05e38df0f8d091ef5293859c Mon Sep 17 00:00:00 2001 From: gabriel Date: Wed, 13 Jul 2022 09:43:32 +0200 Subject: [PATCH 2/2] Manage multiple local disk devices labels for VM live-migration. Signed-off-by: gabriel --- .../kvm/resource/MigrateKVMAsync.java | 14 +++---- .../wrapper/LibvirtMigrateCommandWrapper.java | 32 +++++++++------ .../LibvirtMigrateCommandWrapperTest.java | 40 +++++++++++++------ 3 files changed, 53 insertions(+), 33 deletions(-) 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 c0e7cdfba766..e270458c8094 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 @@ -37,7 +37,7 @@ public class MigrateKVMAsync implements Callable { private String dxml = ""; private String vmName = ""; private String destIp = ""; - private String rootDiskDiskDeviceLabel = null; + private String rootDiskDiskDevicesLabels = null; private boolean migrateStorage; private boolean migrateNonSharedInc; private boolean autoConvergence; @@ -94,7 +94,7 @@ public class MigrateKVMAsync implements Callable { 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 String rootDiskDiskDeviceLabel) { + final String rootDiskDiskDevicesLabels) { this.libvirtComputingResource = libvirtComputingResource; this.dm = dm; @@ -105,7 +105,7 @@ public MigrateKVMAsync(final LibvirtComputingResource libvirtComputingResource, this.autoConvergence = autoConvergence; this.vmName = vmName; this.destIp = destIp; - this.rootDiskDiskDeviceLabel = rootDiskDiskDeviceLabel; + this.rootDiskDiskDevicesLabels = rootDiskDiskDevicesLabels; } @Override @@ -130,8 +130,8 @@ public Domain call() throws LibvirtException { flags |= VIR_MIGRATE_AUTO_CONVERGE; } - if (StringUtils.isNotBlank(rootDiskDiskDeviceLabel) && migrateStorage && !migrateNonSharedInc) { - return migrateRootDisk(flags, rootDiskDiskDeviceLabel, destUri); + if (StringUtils.isNotBlank(rootDiskDiskDevicesLabels) && migrateStorage && !migrateNonSharedInc) { + return migrateRootDisk(flags, rootDiskDiskDevicesLabels, destUri); } return dm.migrate(dconn, flags, dxml, vmName, destUri, libvirtComputingResource.getMigrateSpeed()); } @@ -141,14 +141,14 @@ public Domain call() throws LibvirtException { * VIR_MIGRATE_NON_SHARED_DISK is analagous to "--copy-all" in "virsh". * Binding into: "virsh migrate i-2-13456-VM qemu:///system --live --compressed --copy-storage-all --migrate-disks vda --xml i-2-13456-VM.xml" */ - private Domain migrateRootDisk(long flags, String diskDeviceName, String uri) throws LibvirtException { + private Domain migrateRootDisk(long flags, String diskDevicesName, String uri) throws LibvirtException { int nParams = 5; TypedParameter[] params = new TypedParameter[nParams]; params[0] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_XML, dxml); params[1] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_DEST_NAME, vmName); params[2] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_URI, uri); params[3] = new TypedUlongParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_BANDWIDTH, (long)libvirtComputingResource.getMigrateSpeed()); - params[4] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_MIGRATE_DISKS, diskDeviceName); + params[4] = new TypedStringParameter(Domain.DomainMigrateParameters.VIR_MIGRATE_PARAM_MIGRATE_DISKS, diskDevicesName); return dm.migrate(dconn, params, flags); } } 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 07673a43cb83..7b722bbc80be 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 @@ -23,6 +23,7 @@ import java.io.IOException; import java.io.InputStream; import java.net.URISyntaxException; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Set; @@ -45,6 +46,7 @@ import com.cloud.agent.api.to.DiskTO; import com.cloud.agent.api.to.DpdkTO; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.collections.MapUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.StringUtils; @@ -188,7 +190,7 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. final boolean migrateStorage = MapUtils.isNotEmpty(mapMigrateStorage); final boolean migrateStorageManaged = command.isMigrateStorageManaged(); - String rootDiskDiskDeviceLabel = null; + String rootDiskDiskDevicesLabels = null; if (migrateStorage) { if (s_logger.isDebugEnabled()) { s_logger.debug(String.format("Changing VM [%s] volumes during migration to host: [%s].", vmName, target)); @@ -197,7 +199,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)); } - rootDiskDiskDeviceLabel = retrieveLocalRootDiskDeviceLabel(disks); + rootDiskDiskDevicesLabels = retrieveLocalDiskDevicesLabels(disks); } Map dpdkPortsMapping = command.getDpdkInterfaceMapping(); @@ -224,7 +226,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(), rootDiskDiskDeviceLabel); + command.isAutoConvergence(), vmName, command.getDestinationIp(), rootDiskDiskDevicesLabels); final Future migrateThread = executor.submit(worker); executor.shutdown(); long sleeptime = 0; @@ -365,18 +367,22 @@ Use VIR_DOMAIN_XML_SECURE (value = 1) prior to v1.0.0. * It returns the label of the Root disk in case it is placed in a local storage. * In such cases, the disk device is of type "File". * If the Root volume is not in local storage then it returns null. - * Example of output: "sda". - * - * @Note: At the moment of this implementation, CloudStack supports only Root volumes to be placed in Local storage. - * It is not possible to have local data-disk, therefore, only one device of type "Disk" can be a "File". - * Domain's XML contain also "DeviceType.CDROM" of type "DiskDef.DiskType.FILE", which is filtered in this method. + * Example of output: "sda" for one Root local. + * In case of multiple devices, then comma separated; example: "sda,sdb". */ - protected String retrieveLocalRootDiskDeviceLabel(List disks) { - DiskDef diskDef = disks.get(0); - if (DiskDef.DiskType.FILE == diskDef.getDiskType() && DiskDef.DeviceType.DISK == diskDef.getDeviceType()) { - return diskDef.getDiskLabel(); + protected String retrieveLocalDiskDevicesLabels(List disks) { + List labels = new ArrayList<>(); + for (DiskDef diskDef : disks) { + if (DiskDef.DiskType.FILE == diskDef.getDiskType() && DiskDef.DeviceType.DISK == diskDef.getDeviceType()) { + labels.add(diskDef.getDiskLabel()); + } } - return null; + + String localDiskDevicesLabels = null; + if (CollectionUtils.isNotEmpty(labels)) + localDiskDevicesLabels = String.join(",", labels); + + return localDiskDevicesLabels; } /** 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 46270929dd5e..66a18af54829 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 @@ -794,40 +794,54 @@ public void testReplaceDPDKPorts() throws ParserConfigurationException, IOExcept @Test public void discoverLocalRootDiskDeviceLabelTestRootDiskFile() { - String rootLabel = "vda"; - DiskDef rootDisk = mockDisk(DiskDef.DiskType.FILE, DiskDef.DeviceType.DISK); + String rootLabel = "rootLabel"; + DiskDef rootDisk = mockDisk(DiskDef.DiskType.FILE, DiskDef.DeviceType.DISK, rootLabel); Mockito.when(rootDisk.getDiskLabel()).thenReturn(rootLabel); - prepareAndTestDiscoverLocalRootDiskDeviceLabel(rootDisk, rootLabel); + List disks = new ArrayList<>(); + disks.add(rootDisk); + prepareAndTestDiscoverLocalRootDiskDeviceLabel(disks, rootLabel); } @Test public void discoverLocalRootDiskDeviceLabelTestRootDiskFileNotDisk() { - String rootLabel = "vda"; + String rootLabel = "rootLabel"; String expectedLabel = null; for (DiskDef.DeviceType deviceType : DiskDef.DeviceType.values()) { if (DiskDef.DeviceType.DISK != deviceType) { List disks = new ArrayList<>(); - DiskDef rootDisk = mockDisk(DiskDef.DiskType.FILE, deviceType); + DiskDef rootDisk = mockDisk(DiskDef.DiskType.FILE, deviceType, rootLabel); Mockito.when(rootDisk.getDiskLabel()).thenReturn(rootLabel); - prepareAndTestDiscoverLocalRootDiskDeviceLabel(rootDisk, expectedLabel); + disks.add(rootDisk); + prepareAndTestDiscoverLocalRootDiskDeviceLabel(disks, expectedLabel); } } } - private void prepareAndTestDiscoverLocalRootDiskDeviceLabel(DiskDef rootDisk, String expected) { + @Test + public void TestMultipleDeviceLabel() { List disks = new ArrayList<>(); - disks.add(rootDisk); - disks.add(mockDisk(DiskDef.DiskType.NETWORK, DiskDef.DeviceType.DISK)); - disks.add(mockDisk(DiskDef.DiskType.NETWORK, DiskDef.DeviceType.DISK)); - disks.add(mockDisk(DiskDef.DiskType.BLOCK, DiskDef.DeviceType.DISK)); - String diskLabel = libvirtMigrateCmdWrapper.retrieveLocalRootDiskDeviceLabel(disks); + disks.add(mockDisk(DiskDef.DiskType.FILE, DiskDef.DeviceType.DISK, "label1")); + disks.add(mockDisk(DiskDef.DiskType.FILE, DiskDef.DeviceType.DISK, "label2")); + disks.add(mockDisk(DiskDef.DiskType.NETWORK, DiskDef.DeviceType.DISK, "label3")); + disks.add(mockDisk(DiskDef.DiskType.BLOCK, DiskDef.DeviceType.DISK, "label4")); + String diskLabel = libvirtMigrateCmdWrapper.retrieveLocalDiskDevicesLabels(disks); + String expected = "label1,label2"; + Assert.assertEquals(expected, diskLabel); + } + + private void prepareAndTestDiscoverLocalRootDiskDeviceLabel(List disks, String expected) { + disks.add(mockDisk(DiskDef.DiskType.NETWORK, DiskDef.DeviceType.DISK, "label1")); + disks.add(mockDisk(DiskDef.DiskType.NETWORK, DiskDef.DeviceType.DISK, "label2")); + disks.add(mockDisk(DiskDef.DiskType.BLOCK, DiskDef.DeviceType.DISK,"label3")); + String diskLabel = libvirtMigrateCmdWrapper.retrieveLocalDiskDevicesLabels(disks); Assert.assertEquals(expected, diskLabel); } - private DiskDef mockDisk(DiskDef.DiskType file, DiskDef.DeviceType diskType) { + private DiskDef mockDisk(DiskDef.DiskType file, DiskDef.DeviceType diskType, String label) { DiskDef mockedDisk = Mockito.mock(DiskDef.class); Mockito.when(mockedDisk.getDiskType()).thenReturn(file); Mockito.when(mockedDisk.getDeviceType()).thenReturn(diskType); + Mockito.when(mockedDisk.getDiskLabel()).thenReturn(label); return mockedDisk; } }