From 0e08a126dfd38cc6894e5ecd60757d2a90b38db1 Mon Sep 17 00:00:00 2001 From: Wei Zhou Date: Thu, 25 Apr 2024 08:41:59 +0200 Subject: [PATCH 1/8] systemvm: add template_zone_ref record when add a new zone with same hypervisor type (#8395) --- .../cloud/upgrade/SystemVmTemplateRegistration.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 dc94dd708b1..1ee42674480 100644 --- a/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java +++ b/engine/schema/src/main/java/com/cloud/upgrade/SystemVmTemplateRegistration.java @@ -482,19 +482,19 @@ public class SystemVmTemplateRegistration { templateZoneVO = vmTemplateZoneDao.persist(templateZoneVO); } else { templateZoneVO.setLastUpdated(new java.util.Date()); - if (vmTemplateZoneDao.update(templateZoneVO.getId(), templateZoneVO)) { + if (!vmTemplateZoneDao.update(templateZoneVO.getId(), templateZoneVO)) { templateZoneVO = null; } } return templateZoneVO; } - private void createCrossZonesTemplateZoneRefEntries(VMTemplateVO template) { + private void createCrossZonesTemplateZoneRefEntries(Long templateId) { List dcs = dataCenterDao.listAll(); for (DataCenterVO dc : dcs) { - VMTemplateZoneVO templateZoneVO = createOrUpdateTemplateZoneEntry(dc.getId(), template.getId()); + VMTemplateZoneVO templateZoneVO = createOrUpdateTemplateZoneEntry(dc.getId(), templateId); if (templateZoneVO == null) { - throw new CloudRuntimeException(String.format("Failed to create template_zone_ref record for the systemVM template for hypervisor: %s and zone: %s", template.getHypervisorType().name(), dc)); + throw new CloudRuntimeException(String.format("Failed to create template_zone_ref record for the systemVM template (id: %s) and zone: %s", templateId, dc)); } } } @@ -624,8 +624,9 @@ public class SystemVmTemplateRegistration { throw new CloudRuntimeException(String.format("Failed to register template for hypervisor: %s", hypervisor.name())); } templateId = template.getId(); - createCrossZonesTemplateZoneRefEntries(template); } + createCrossZonesTemplateZoneRefEntries(templateId); + details.setId(templateId); String destTempFolderName = String.valueOf(templateId); String destTempFolder = filePath + PARTIAL_TEMPLATE_FOLDER + destTempFolderName; 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 2/8] 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 6d69b2f9664..c3b5f9857eb 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 class LibvirtVMDef { 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 c3917314250..bc94bb47ed8 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 class MigrateKVMAsync implements Callable { this.autoConvergence = autoConvergence; this.vmName = vmName; this.destIp = destIp; + this.migrateDiskLabels = migrateDiskLabels; } @Override @@ -121,6 +131,37 @@ public class MigrateKVMAsync implements Callable { 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 fb526626ef8..aebbaa4119d 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.IOException; 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 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper migrateDiskLabels = null; if (migrateStorage) { if (s_logger.isDebugEnabled()) { @@ -199,6 +201,7 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper dpdkPortsMapping = command.getDpdkInterfaceMapping(); @@ -227,7 +230,7 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper 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 @@ public final class LibvirtMigrateCommandWrapper extends CommandWrapper 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 00000000000..28633b925b2 --- /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 c206f898e97..7071758fcbe 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.IOException; 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 class LibvirtMigrateCommandWrapperTest { " \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 @@ public class LibvirtMigrateCommandWrapperTest { @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 class LibvirtMigrateCommandWrapperTest { @Test public void testReplaceStorageWithSecrets() throws Exception { - Map mapMigrateStorage = new HashMap(); final String xmlDesc = "" + @@ -799,8 +806,7 @@ public class LibvirtMigrateCommandWrapperTest { 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 class LibvirtMigrateCommandWrapperTest { 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"))); + } + } From e409c6d870a49762fb4e730ad118a3a836715cc5 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Thu, 25 Apr 2024 16:28:52 +0530 Subject: [PATCH 3/8] Fixup listing of serivce offering & storagepools with tags (#8937) --- .../com/cloud/api/query/QueryManagerImpl.java | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index d72e4760f57..9aee1636c2a 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2977,7 +2977,7 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q ListResponse response = new ListResponse<>(); List poolResponses = ViewResponseHelper.createStoragePoolResponse(storagePools.first().toArray(new StoragePoolJoinVO[storagePools.first().size()])); - Map poolUuidToIdMap = storagePools.first().stream().collect(Collectors.toMap(StoragePoolJoinVO::getUuid, StoragePoolJoinVO::getId)); + Map poolUuidToIdMap = storagePools.first().stream().collect(Collectors.toMap(StoragePoolJoinVO::getUuid, StoragePoolJoinVO::getId, (a, b) -> a)); for (StoragePoolResponse poolResponse : poolResponses) { DataStore store = dataStoreManager.getPrimaryDataStore(poolResponse.getId()); if (store != null) { @@ -3790,9 +3790,8 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q List storageTags = com.cloud.utils.StringUtils.csvTagsToList(diskOffering.getTags()); if (!storageTags.isEmpty() && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.value()) { for (String tag : storageTags) { - diskOfferingSearch.and(tag, diskOfferingSearch.entity().getTags(), Op.EQ); + diskOfferingSearch.and("storageTag" + tag, diskOfferingSearch.entity().getTags(), Op.FIND_IN_SET); } - diskOfferingSearch.done(); } } @@ -3904,18 +3903,24 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q srvOffrDomainDetailSearch.entity().getName(), serviceOfferingSearch.entity().setString(ApiConstants.DOMAIN_ID)); } + List hostTags = new ArrayList<>(); if (currentVmOffering != null) { - List hostTags = com.cloud.utils.StringUtils.csvTagsToList(currentVmOffering.getHostTag()); - if (!hostTags.isEmpty()) { + hostTags.addAll(com.cloud.utils.StringUtils.csvTagsToList(currentVmOffering.getHostTag())); + } - serviceOfferingSearch.and().op("hostTag", serviceOfferingSearch.entity().getHostTag(), Op.NULL); - serviceOfferingSearch.or().op(); - - for(String tag : hostTags) { - serviceOfferingSearch.and(tag, serviceOfferingSearch.entity().getHostTag(), Op.EQ); + if (!hostTags.isEmpty()) { + serviceOfferingSearch.and().op("hostTag", serviceOfferingSearch.entity().getHostTag(), Op.NULL); + serviceOfferingSearch.or(); + boolean flag = true; + for(String tag : hostTags) { + if (flag) { + flag = false; + serviceOfferingSearch.op("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET); + } else { + serviceOfferingSearch.and("hostTag" + tag, serviceOfferingSearch.entity().getHostTag(), Op.FIND_IN_SET); } - serviceOfferingSearch.cp().cp().done(); } + serviceOfferingSearch.cp().cp(); } SearchCriteria sc = serviceOfferingSearch.create(); @@ -4032,22 +4037,18 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q sc.setJoinParameters("domainDetailSearchNormalUser", "domainIdIN", domainIds.toArray()); } - if (currentVmOffering != null) { - - if (diskOffering != null) { - List storageTags = com.cloud.utils.StringUtils.csvTagsToList(diskOffering.getTags()); - if (!storageTags.isEmpty() && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.value()) { - for(String tag : storageTags) { - sc.setJoinParameters("diskOfferingSearch", tag, tag); - } + if (diskOffering != null) { + List storageTags = com.cloud.utils.StringUtils.csvTagsToList(diskOffering.getTags()); + if (!storageTags.isEmpty() && VolumeApiServiceImpl.MatchStoragePoolTagsWithDiskOffering.value()) { + for (String tag : storageTags) { + sc.setJoinParameters("diskOfferingSearch", "storageTag" + tag, tag); } } + } - List hostTags = com.cloud.utils.StringUtils.csvTagsToList(currentVmOffering.getHostTag()); - if (!hostTags.isEmpty()) { - for(String tag : hostTags) { - sc.setParameters(tag, tag); - } + if (!hostTags.isEmpty()) { + for(String tag : hostTags) { + sc.setParameters("hostTag" + tag, tag); } } From eead2710f81858ff889f526c6d1955d0b91cca5f Mon Sep 17 00:00:00 2001 From: dahn Date: Thu, 25 Apr 2024 16:45:42 +0200 Subject: [PATCH 4/8] explanatory error message on delete attempt of default system offering (#8883) Co-authored-by: Gabriel Pordeus Santos --- .../java/com/cloud/configuration/ConfigurationManagerImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java index 080bb83253c..f0e7522bc79 100644 --- a/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java +++ b/server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java @@ -4204,7 +4204,7 @@ public class ConfigurationManagerImpl extends ManagerBase implements Configurati } if (offering.getDefaultUse()) { - throw new InvalidParameterValueException("Default service offerings cannot be deleted"); + throw new InvalidParameterValueException(String.format("The system service offering [%s] is marked for default use and cannot be deleted", offering.getDisplayText())); } final User user = _userDao.findById(userId); From 9d5d4e55648c279c7d462d26469d07174bcd0701 Mon Sep 17 00:00:00 2001 From: Rene Peinthor Date: Fri, 26 Apr 2024 14:25:07 +0200 Subject: [PATCH 5/8] linstor: cleanup diskless nodes on disconnect (#8790) --- .../kvm/storage/LinstorStorageAdaptor.java | 114 ++++++++++++------ 1 file changed, 74 insertions(+), 40 deletions(-) diff --git a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java index 66278f827f7..b38ab382a42 100644 --- a/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java +++ b/plugins/storage/volume/linstor/src/main/java/com/cloud/hypervisor/kvm/storage/LinstorStorageAdaptor.java @@ -38,6 +38,7 @@ import org.libvirt.LibvirtException; import com.cloud.storage.Storage; import com.cloud.utils.exception.CloudRuntimeException; import com.linbit.linstor.api.ApiClient; +import com.linbit.linstor.api.ApiConsts; import com.linbit.linstor.api.ApiException; import com.linbit.linstor.api.Configuration; import com.linbit.linstor.api.DevelopersApi; @@ -103,6 +104,10 @@ public class LinstorStorageAdaptor implements StorageAdaptor { } } + private void logLinstorAnswers(@Nonnull ApiCallRcList answers) { + answers.forEach(this::logLinstorAnswer); + } + private void checkLinstorAnswersThrow(@Nonnull ApiCallRcList answers) { answers.forEach(this::logLinstorAnswer); if (answers.hasError()) @@ -316,23 +321,90 @@ public class LinstorStorageAdaptor implements StorageAdaptor { return true; } + private boolean tryDisconnectLinstor(String volumePath, KVMStoragePool pool) + { + if (volumePath == null) { + return false; + } + + s_logger.debug("Linstor: Using storage pool: " + pool.getUuid()); + final DevelopersApi api = getLinstorAPI(pool); + + Optional optRsc; + try + { + List resources = api.viewResources( + Collections.singletonList(localNodeName), + null, + null, + null, + null, + null); + + optRsc = getResourceByPath(resources, volumePath); + } catch (ApiException apiEx) { + // couldn't query linstor controller + s_logger.error(apiEx.getBestMessage()); + return false; + } + + + if (optRsc.isPresent()) { + try { + Resource rsc = optRsc.get(); + + // if diskless resource remove it, in the worst case it will be transformed to a tiebreaker + if (rsc.getFlags() != null && + rsc.getFlags().contains(ApiConsts.FLAG_DRBD_DISKLESS) && + !rsc.getFlags().contains(ApiConsts.FLAG_TIE_BREAKER)) { + ApiCallRcList delAnswers = api.resourceDelete(rsc.getName(), localNodeName); + logLinstorAnswers(delAnswers); + } + + // remove allow-two-primaries + ResourceDefinitionModify rdm = new ResourceDefinitionModify(); + rdm.deleteProps(Collections.singletonList("DrbdOptions/Net/allow-two-primaries")); + ApiCallRcList answers = api.resourceDefinitionModify(rsc.getName(), rdm); + if (answers.hasError()) { + s_logger.error( + String.format("Failed to remove 'allow-two-primaries' on %s: %s", + rsc.getName(), LinstorUtil.getBestErrorMessage(answers))); + // do not fail here as removing allow-two-primaries property isn't fatal + } + } catch (ApiException apiEx) { + s_logger.error(apiEx.getBestMessage()); + // do not fail here as removing allow-two-primaries property or deleting diskless isn't fatal + } + + return true; + } + + s_logger.warn("Linstor: Couldn't find resource for this path: " + volumePath); + return false; + } + @Override public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool) { s_logger.debug("Linstor: disconnectPhysicalDisk " + pool.getUuid() + ":" + volumePath); + if (MapStorageUuidToStoragePool.containsValue(pool)) { + return tryDisconnectLinstor(volumePath, pool); + } return false; } @Override public boolean disconnectPhysicalDisk(Map volumeToDisconnect) { + // as of now this is only relevant for iscsi targets + s_logger.info("Linstor: disconnectPhysicalDisk(Map volumeToDisconnect) called?"); return false; } private Optional getResourceByPath(final List resources, String path) { return resources.stream() .filter(rsc -> rsc.getVolumes().stream() - .anyMatch(v -> v.getDevicePath().equals(path))) + .anyMatch(v -> path.equals(v.getDevicePath()))) .findFirst(); } @@ -353,46 +425,8 @@ public class LinstorStorageAdaptor implements StorageAdaptor { s_logger.debug("Linstor: disconnectPhysicalDiskByPath " + localPath); final KVMStoragePool pool = optFirstPool.get(); - s_logger.debug("Linstor: Using storpool: " + pool.getUuid()); - final DevelopersApi api = getLinstorAPI(pool); - - Optional optRsc; - try { - List resources = api.viewResources( - Collections.singletonList(localNodeName), - null, - null, - null, - null, - null); - - optRsc = getResourceByPath(resources, localPath); - } catch (ApiException apiEx) { - // couldn't query linstor controller - s_logger.error(apiEx.getBestMessage()); - return false; - } - - if (optRsc.isPresent()) { - try { - Resource rsc = optRsc.get(); - ResourceDefinitionModify rdm = new ResourceDefinitionModify(); - rdm.deleteProps(Collections.singletonList("DrbdOptions/Net/allow-two-primaries")); - ApiCallRcList answers = api.resourceDefinitionModify(rsc.getName(), rdm); - if (answers.hasError()) { - s_logger.error( - String.format("Failed to remove 'allow-two-primaries' on %s: %s", - rsc.getName(), LinstorUtil.getBestErrorMessage(answers))); - // do not fail here as removing allow-two-primaries property isn't fatal - } - } catch(ApiException apiEx){ - s_logger.error(apiEx.getBestMessage()); - // do not fail here as removing allow-two-primaries property isn't fatal - } - return true; - } + return tryDisconnectLinstor(localPath, pool); } - s_logger.info("Linstor: Couldn't find resource for this path: " + localPath); return false; } From 80a8b80a9d511d630ae8bbd38b6f2d1e46b3f1a4 Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 29 Apr 2024 12:18:09 +0530 Subject: [PATCH 6/8] Update volume's passphrase to null if diskOffering doesn't support encryption (#8904) --- .../service/VolumeOrchestrationService.java | 2 +- .../engine/orchestration/VolumeOrchestrator.java | 11 +++++------ .../cloudstack/storage/volume/VolumeServiceImpl.java | 2 +- .../src/main/java/com/cloud/vm/UserVmManagerImpl.java | 6 +++--- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java index 814091bf3ae..c3525466ce1 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/orchestration/service/VolumeOrchestrationService.java @@ -85,7 +85,7 @@ public interface VolumeOrchestrationService { VolumeInfo moveVolume(VolumeInfo volume, long destPoolDcId, Long destPoolPodId, Long destPoolClusterId, HypervisorType dataDiskHyperType) throws ConcurrentOperationException, StorageUnavailableException; - Volume allocateDuplicateVolume(Volume oldVol, Long templateId); + Volume allocateDuplicateVolume(Volume oldVol, DiskOffering diskOffering, Long templateId); boolean volumeOnSharedStoragePool(Volume volume); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java index 22406292ce1..565c3f2101a 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java @@ -305,11 +305,11 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati } @Override - public Volume allocateDuplicateVolume(Volume oldVol, Long templateId) { - return allocateDuplicateVolumeVO(oldVol, templateId); + public Volume allocateDuplicateVolume(Volume oldVol, DiskOffering diskOffering, Long templateId) { + return allocateDuplicateVolumeVO(oldVol, diskOffering, templateId); } - public VolumeVO allocateDuplicateVolumeVO(Volume oldVol, Long templateId) { + public VolumeVO allocateDuplicateVolumeVO(Volume oldVol, DiskOffering diskOffering, Long templateId) { VolumeVO newVol = new VolumeVO(oldVol.getVolumeType(), oldVol.getName(), oldVol.getDataCenterId(), oldVol.getDomainId(), oldVol.getAccountId(), oldVol.getDiskOfferingId(), oldVol.getProvisioningType(), oldVol.getSize(), oldVol.getMinIops(), oldVol.getMaxIops(), oldVol.get_iScsiName()); if (templateId != null) { @@ -321,8 +321,7 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati newVol.setInstanceId(oldVol.getInstanceId()); newVol.setRecreatable(oldVol.isRecreatable()); newVol.setFormat(oldVol.getFormat()); - - if (oldVol.getPassphraseId() != null) { + if ((diskOffering == null || diskOffering.getEncrypt()) && oldVol.getPassphraseId() != null) { PassphraseVO passphrase = passphraseDao.persist(new PassphraseVO(true)); newVol.setPassphraseId(passphrase.getId()); } @@ -1180,7 +1179,7 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati return Transaction.execute(new TransactionCallback() { @Override public VolumeVO doInTransaction(TransactionStatus status) { - VolumeVO newVolume = allocateDuplicateVolumeVO(existingVolume, templateIdToUseFinal); + VolumeVO newVolume = allocateDuplicateVolumeVO(existingVolume, null, templateIdToUseFinal); try { stateTransitTo(existingVolume, Volume.Event.DestroyRequested); } catch (NoTransitionException e) { 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 75f652da379..59d027e3c42 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 @@ -1236,7 +1236,7 @@ public class VolumeServiceImpl implements VolumeService { volumeInfo.processEvent(Event.DestroyRequested); - Volume newVol = _volumeMgr.allocateDuplicateVolume(volume, null); + Volume newVol = _volumeMgr.allocateDuplicateVolume(volume, null, null); VolumeVO newVolume = (VolumeVO) newVol; newVolume.set_iScsiName(null); volDao.update(newVolume.getId(), newVolume); diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 6c8aec0da48..adab257f450 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7863,19 +7863,19 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Volume newVol = null; if (newTemplateId != null) { if (isISO) { - newVol = volumeMgr.allocateDuplicateVolume(root, null); + newVol = volumeMgr.allocateDuplicateVolume(root, diskOffering, null); userVm.setIsoId(newTemplateId); userVm.setGuestOSId(template.getGuestOSId()); userVm.setTemplateId(newTemplateId); } else { - newVol = volumeMgr.allocateDuplicateVolume(root, newTemplateId); + newVol = volumeMgr.allocateDuplicateVolume(root, diskOffering, newTemplateId); userVm.setGuestOSId(template.getGuestOSId()); userVm.setTemplateId(newTemplateId); } // check and update VM if it can be dynamically scalable with the new template updateVMDynamicallyScalabilityUsingTemplate(userVm, newTemplateId); } else { - newVol = volumeMgr.allocateDuplicateVolume(root, null); + newVol = volumeMgr.allocateDuplicateVolume(root, diskOffering, null); } updateVolume(newVol, template, userVm, diskOffering, details); From 08132acaa2fa989d623db8e3b194df2bede13f7c Mon Sep 17 00:00:00 2001 From: Vishesh Date: Mon, 29 Apr 2024 12:19:05 +0530 Subject: [PATCH 7/8] Fix restore VM with allocated root disk (#8977) * Fix restore VM with allocated root disk * Add e2e test for restore vm * Add more checks for e2e test --- .github/workflows/ci.yml | 1 + .../java/com/cloud/vm/UserVmManagerImpl.java | 4 +- test/integration/smoke/test_restore_vm.py | 108 ++++++++++++++++++ ui/src/views/compute/ReinstallVm.vue | 18 ++- 4 files changed, 124 insertions(+), 7 deletions(-) create mode 100644 test/integration/smoke/test_restore_vm.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6a6ea33b14a..42407a48b9b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -54,6 +54,7 @@ jobs: smoke/test_deploy_vm_with_userdata smoke/test_deploy_vms_in_parallel smoke/test_deploy_vms_with_varied_deploymentplanners + smoke/test_restore_vm smoke/test_diagnostics smoke/test_direct_download smoke/test_disk_offerings diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index adab257f450..e574d9887c3 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -7846,7 +7846,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir DiskOffering diskOffering = rootDiskOfferingId != null ? _diskOfferingDao.findById(rootDiskOfferingId) : null; for (VolumeVO root : rootVols) { if ( !Volume.State.Allocated.equals(root.getState()) || newTemplateId != null ) { - _volumeService.validateDestroyVolume(root, caller, expunge, false); + _volumeService.validateDestroyVolume(root, caller, Volume.State.Allocated.equals(root.getState()) || expunge, false); final UserVmVO userVm = vm; Pair vmAndNewVol = Transaction.execute(new TransactionCallbackWithException, CloudRuntimeException>() { @Override @@ -7909,7 +7909,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir // Detach, destroy and create the usage event for the old root volume. _volsDao.detachVolume(root.getId()); - _volumeService.destroyVolume(root.getId(), caller, expunge, false); + _volumeService.destroyVolume(root.getId(), caller, Volume.State.Allocated.equals(root.getState()) || expunge, false); // For VMware hypervisor since the old root volume is replaced by the new root volume, force expunge old root volume if it has been created in storage if (vm.getHypervisorType() == HypervisorType.VMware) { diff --git a/test/integration/smoke/test_restore_vm.py b/test/integration/smoke/test_restore_vm.py new file mode 100644 index 00000000000..ec0383d17a8 --- /dev/null +++ b/test/integration/smoke/test_restore_vm.py @@ -0,0 +1,108 @@ +# 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. +""" P1 tests for Scaling up Vm +""" +# Import Local Modules +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.base import (VirtualMachine, Volume, ServiceOffering, Template) +from marvin.lib.common import (get_zone, get_domain) +from nose.plugins.attrib import attr + +_multiprocess_shared_ = True + + +class TestRestoreVM(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + testClient = super(TestRestoreVM, cls).getClsTestClient() + cls.apiclient = testClient.getApiClient() + cls.services = testClient.getParsedTestDataConfig() + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.apiclient) + cls.zone = get_zone(cls.apiclient, testClient.getZoneForTests()) + cls.hypervisor = testClient.getHypervisorInfo() + cls.services['mode'] = cls.zone.networktype + + cls.services["virtual_machine"]["zoneid"] = cls.zone.id + + cls.service_offering = ServiceOffering.create(cls.apiclient, cls.services["service_offering"]) + + cls.template_t1 = Template.register(cls.apiclient, cls.services["test_templates"][ + cls.hypervisor.lower() if cls.hypervisor.lower() != 'simulator' else 'xenserver'], + zoneid=cls.zone.id, hypervisor=cls.hypervisor.lower()) + + cls.template_t2 = Template.register(cls.apiclient, cls.services["test_templates"][ + cls.hypervisor.lower() if cls.hypervisor.lower() != 'simulator' else 'xenserver'], + zoneid=cls.zone.id, hypervisor=cls.hypervisor.lower()) + + cls._cleanup = [cls.service_offering, cls.template_t1, cls.template_t2] + + @classmethod + def tearDownClass(cls): + super(TestRestoreVM, cls).tearDownClass() + return + + @attr(tags=["advanced", "basic"], required_hardware="false") + def test_01_restore_vm(self): + """Test restore virtual machine + """ + # create a virtual machine + virtual_machine = VirtualMachine.create(self.apiclient, self.services["virtual_machine"], zoneid=self.zone.id, + templateid=self.template_t1.id, + serviceofferingid=self.service_offering.id) + self._cleanup.append(virtual_machine) + + root_vol = Volume.list(self.apiclient, virtualmachineid=virtual_machine.id)[0] + self.assertEqual(root_vol.state, 'Ready', "Volume should be in Ready state") + self.assertEqual(root_vol.size, self.template_t1.size, "Size of volume and template should match") + + virtual_machine.restore(self.apiclient, self.template_t2.id) + restored_vm = VirtualMachine.list(self.apiclient, id=virtual_machine.id)[0] + self.assertEqual(restored_vm.state, 'Running', "VM should be in a running state") + self.assertEqual(restored_vm.templateid, self.template_t2.id, "VM's template after restore is incorrect") + root_vol = Volume.list(self.apiclient, virtualmachineid=restored_vm.id)[0] + self.assertEqual(root_vol.state, 'Ready', "Volume should be in Ready state") + self.assertEqual(root_vol.size, self.template_t2.size, "Size of volume and template should match") + + @attr(tags=["advanced", "basic"], required_hardware="false") + def test_02_restore_vm_allocated_root(self): + """Test restore virtual machine with root disk in allocated state + """ + # create a virtual machine with allocated root disk by setting startvm=False + virtual_machine = VirtualMachine.create(self.apiclient, self.services["virtual_machine"], zoneid=self.zone.id, + templateid=self.template_t1.id, + serviceofferingid=self.service_offering.id, + startvm=False) + self._cleanup.append(virtual_machine) + root_vol = Volume.list(self.apiclient, virtualmachineid=virtual_machine.id)[0] + self.assertEqual(root_vol.state, 'Allocated', "Volume should be in Allocated state") + self.assertEqual(root_vol.size, self.template_t1.size, "Size of volume and template should match") + + virtual_machine.restore(self.apiclient, self.template_t2.id) + restored_vm = VirtualMachine.list(self.apiclient, id=virtual_machine.id)[0] + self.assertEqual(restored_vm.state, 'Stopped', "Check the state of VM") + self.assertEqual(restored_vm.templateid, self.template_t2.id, "Check the template of VM") + + root_vol = Volume.list(self.apiclient, virtualmachineid=restored_vm.id)[0] + self.assertEqual(root_vol.state, 'Allocated', "Volume should be in Allocated state") + self.assertEqual(root_vol.size, self.template_t2.size, "Size of volume and template should match") + + virtual_machine.start(self.apiclient) + root_vol = Volume.list(self.apiclient, virtualmachineid=restored_vm.id)[0] + self.assertEqual(root_vol.state, 'Ready', "Volume should be in Ready state") diff --git a/ui/src/views/compute/ReinstallVm.vue b/ui/src/views/compute/ReinstallVm.vue index ee07011fe28..909f4769425 100644 --- a/ui/src/views/compute/ReinstallVm.vue +++ b/ui/src/views/compute/ReinstallVm.vue @@ -36,7 +36,7 @@ :items="templates" :selected="tabKey" :loading="loading.templates" - :preFillContent="resource.templateid" + :preFillContent="dataPrefill" :key="templateKey" @handle-search-filter="($event) => fetchAllTemplates($event)" @update-template-iso="updateFieldValue" @@ -61,7 +61,7 @@ :zoneId="resource.zoneId" :value="diskOffering ? diskOffering.id : ''" :loading="loading.diskOfferings" - :preFillContent="resource.diskofferingid" + :preFillContent="dataPrefill" :isIsoSelected="false" :isRootDiskOffering="true" @on-selected-disk-size="onSelectDiskSize" @@ -170,7 +170,11 @@ export default { ], diskOffering: {}, diskOfferingCount: 0, - templateKey: 0 + templateKey: 0, + dataPrefill: { + templateid: this.resource.templateid, + diskofferingid: this.resource.diskofferingid + } } }, beforeCreate () { @@ -192,8 +196,10 @@ export default { }, handleSubmit () { const params = { - virtualmachineid: this.resource.id, - templateid: this.templateid + virtualmachineid: this.resource.id + } + if (this.templateid) { + params.templateid = this.templateid } if (this.overrideDiskOffering) { params.diskofferingid = this.diskOffering.id @@ -285,9 +291,11 @@ export default { }, onSelectDiskSize (rowSelected) { this.diskOffering = rowSelected + this.dataPrefill.diskofferingid = rowSelected.id }, updateFieldValue (input, value) { this[input] = value + this.dataPrefill[input] = value } } } From b23ceefc7eabbc0bab85c85ffd7d4fdcbdc4c7b8 Mon Sep 17 00:00:00 2001 From: Rohit Yadav Date: Mon, 29 Apr 2024 13:42:31 +0530 Subject: [PATCH 8/8] utils: cleanup MacAddress and MacAddressTest (#8988) * utils: cleanup MacAddress and MacAddressTest Cleanup old mac address handling code to use JDK11 lib instead of hacks. Also really strange to see some basic string parsing code was written by hand, replaced with Long.parseValue(str, 16) to convert hex string to long. Signed-off-by: Rohit Yadav * address review comments Signed-off-by: Rohit Yadav --------- Signed-off-by: Rohit Yadav --- .../java/com/cloud/utils/net/MacAddress.java | 233 +++--------------- .../com/cloud/utils/net/MacAddressTest.java | 14 +- 2 files changed, 40 insertions(+), 207 deletions(-) diff --git a/utils/src/main/java/com/cloud/utils/net/MacAddress.java b/utils/src/main/java/com/cloud/utils/net/MacAddress.java index d7ac9e39e7f..76f3f6c24ac 100644 --- a/utils/src/main/java/com/cloud/utils/net/MacAddress.java +++ b/utils/src/main/java/com/cloud/utils/net/MacAddress.java @@ -19,25 +19,19 @@ package com.cloud.utils.net; -import static com.cloud.utils.AutoCloseableUtil.closeAutoCloseable; - -import java.io.BufferedReader; -import java.io.File; -import java.io.IOException; -import java.io.InputStreamReader; import java.net.InetAddress; +import java.net.NetworkInterface; +import java.net.SocketException; import java.net.UnknownHostException; +import java.util.Collections; import java.util.Formatter; - -import org.apache.log4j.Logger; +import java.util.List; /** * This class retrieves the (first) MAC address for the machine is it is loaded on and stores it statically for retrieval. * It can also be used for formatting MAC addresses. - * copied fnd addpeted rom the public domain utility from John Burkard. **/ public class MacAddress { - private static final Logger s_logger = Logger.getLogger(MacAddress.class); private long _addr = 0; protected MacAddress() { @@ -75,213 +69,52 @@ public class MacAddress { return toString(":"); } - private static MacAddress s_address; + private static MacAddress macAddress; + static { - String macAddress = null; - - Process p = null; - BufferedReader in = null; - + String macString = null; try { - String osname = System.getProperty("os.name"); - - if (osname.startsWith("Windows")) { - p = Runtime.getRuntime().exec(new String[] {"ipconfig", "/all"}, null); - } else if (osname.startsWith("Solaris") || osname.startsWith("SunOS")) { - // Solaris code must appear before the generic code - String hostName = MacAddress.getFirstLineOfCommand(new String[] {"uname", "-n"}); - if (hostName != null) { - p = Runtime.getRuntime().exec(new String[] {"/usr/sbin/arp", hostName}, null); - } - } else if (new File("/usr/sbin/lanscan").exists()) { - p = Runtime.getRuntime().exec(new String[] {"/usr/sbin/lanscan"}, null); - } else if (new File("/sbin/ifconfig").exists()) { - p = Runtime.getRuntime().exec(new String[] {"/sbin/ifconfig", "-a"}, null); - } - - if (p != null) { - in = new BufferedReader(new InputStreamReader(p.getInputStream()), 128); - String l = null; - while ((l = in.readLine()) != null) { - macAddress = MacAddress.parse(l); - if (macAddress != null) { - short parsedShortMacAddress = MacAddress.parseShort(macAddress); - if (parsedShortMacAddress != 0xff && parsedShortMacAddress != 0x00) - break; + final List nics = Collections.list(NetworkInterface.getNetworkInterfaces()); + Collections.reverse(nics); + for (final NetworkInterface nic : nics) { + final byte[] mac = nic.getHardwareAddress(); + if (mac != null && + !nic.isVirtual() && + !nic.isLoopback() && + !nic.getName().startsWith("br") && + !nic.getName().startsWith("veth") && + !nic.getName().startsWith("vnet")) { + StringBuilder macAddressBuilder = new StringBuilder(); + for (byte b : mac) { + macAddressBuilder.append(String.format("%02X", b)); } - macAddress = null; + macString = macAddressBuilder.toString(); + break; } } - - } catch (SecurityException ex) { - s_logger.info("[ignored] security exception in static initializer of MacAddress", ex); - } catch (IOException ex) { - s_logger.info("[ignored] io exception in static initializer of MacAddress"); - } finally { - if (p != null) { - closeAutoCloseable(in, "closing init process input stream"); - closeAutoCloseable(p.getErrorStream(), "closing init process error output stream"); - closeAutoCloseable(p.getOutputStream(), "closing init process std output stream"); - p.destroy(); - } + } catch (SocketException ignore) { } - long clockSeqAndNode = 0; + long macAddressLong = 0; - if (macAddress != null) { - if (macAddress.indexOf(':') != -1) { - clockSeqAndNode |= MacAddress.parseLong(macAddress); - } else if (macAddress.startsWith("0x")) { - clockSeqAndNode |= MacAddress.parseLong(macAddress.substring(2)); - } + if (macString != null) { + macAddressLong = Long.parseLong(macString, 16); } else { try { byte[] local = InetAddress.getLocalHost().getAddress(); - clockSeqAndNode |= (local[0] << 24) & 0xFF000000L; - clockSeqAndNode |= (local[1] << 16) & 0xFF0000; - clockSeqAndNode |= (local[2] << 8) & 0xFF00; - clockSeqAndNode |= local[3] & 0xFF; + macAddressLong |= (local[0] << 24) & 0xFF000000L; + macAddressLong |= (local[1] << 16) & 0xFF0000; + macAddressLong |= (local[2] << 8) & 0xFF00; + macAddressLong |= local[3] & 0xFF; } catch (UnknownHostException ex) { - clockSeqAndNode |= (long)(Math.random() * 0x7FFFFFFF); + macAddressLong |= (long)(Math.random() * 0x7FFFFFFF); } } - s_address = new MacAddress(clockSeqAndNode); + MacAddress.macAddress = new MacAddress(macAddressLong); } public static MacAddress getMacAddress() { - return s_address; - } - - private static String getFirstLineOfCommand(String[] commands) throws IOException { - - Process p = null; - BufferedReader reader = null; - - try { - p = Runtime.getRuntime().exec(commands); - reader = new BufferedReader(new InputStreamReader(p.getInputStream()), 128); - - return reader.readLine(); - } finally { - if (p != null) { - closeAutoCloseable(reader, "closing process input stream"); - closeAutoCloseable(p.getErrorStream(), "closing process error output stream"); - closeAutoCloseable(p.getOutputStream(), "closing process std output stream"); - p.destroy(); - } - } - - } - - /** - * The MAC address parser attempts to find the following patterns: - *
      - *
    • .{1,2}:.{1,2}:.{1,2}:.{1,2}:.{1,2}:.{1,2}
      • - *
    • .{1,2}-.{1,2}-.{1,2}-.{1,2}-.{1,2}-.{1,2}
      • - *
    - * - * This is copied from the author below. The author encouraged copying - * it. - * - */ - static String parse(String in) { - - // lanscan - - int hexStart = in.indexOf("0x"); - if (hexStart != -1) { - int hexEnd = in.indexOf(' ', hexStart); - if (hexEnd != -1) { - return in.substring(hexStart, hexEnd); - } - } - - int octets = 0; - int lastIndex, old, end; - - if (in.indexOf('-') > -1) { - in = in.replace('-', ':'); - } - - lastIndex = in.lastIndexOf(':'); - - if (lastIndex > in.length() - 2) - return null; - - end = Math.min(in.length(), lastIndex + 3); - - ++octets; - old = lastIndex; - while (octets != 5 && lastIndex != -1 && lastIndex > 1) { - lastIndex = in.lastIndexOf(':', --lastIndex); - if (old - lastIndex == 3 || old - lastIndex == 2) { - ++octets; - old = lastIndex; - } - } - - if (octets == 5 && lastIndex > 1) { - return in.substring(lastIndex - 2, end).trim(); - } - return null; - } - - /** - * Parses a long from a hex encoded number. This method will skip - * all characters that are not 0-9 and a-f (the String is lower cased first). - * Returns 0 if the String does not contain any interesting characters. - * - * @param s the String to extract a long from, may not be null - * @return a long - * @throws NullPointerException if the String is null - */ - private static long parseLong(String s) throws NullPointerException { - s = s.toLowerCase(); - long out = 0; - byte shifts = 0; - char c; - for (int i = 0; i < s.length() && shifts < 16; i++) { - c = s.charAt(i); - if ((c > 47) && (c < 58)) { - out <<= 4; - ++shifts; - out |= c - 48; - } else if ((c > 96) && (c < 103)) { - ++shifts; - out <<= 4; - out |= c - 87; - } - } - return out; - } - - /** - * Parses a short from a hex encoded number. This method will skip - * all characters that are not 0-9 and a-f (the String is lower cased first). - * Returns 0 if the String does not contain any interesting characters. - * - * @param s the String to extract a short from, may not be null - * @return a short - * @throws NullPointerException if the String is null - */ - private static short parseShort(String s) throws NullPointerException { - s = s.toLowerCase(); - short out = 0; - byte shifts = 0; - char c; - for (int i = 0; i < s.length() && shifts < 4; i++) { - c = s.charAt(i); - if ((c > 47) && (c < 58)) { - out <<= 4; - ++shifts; - out |= c - 48; - } else if ((c > 96) && (c < 103)) { - ++shifts; - out <<= 4; - out |= c - 87; - } - } - return out; + return macAddress; } } diff --git a/utils/src/test/java/com/cloud/utils/net/MacAddressTest.java b/utils/src/test/java/com/cloud/utils/net/MacAddressTest.java index 42b03b70daf..ff33ed03f00 100644 --- a/utils/src/test/java/com/cloud/utils/net/MacAddressTest.java +++ b/utils/src/test/java/com/cloud/utils/net/MacAddressTest.java @@ -41,14 +41,14 @@ public class MacAddressTest { public final void testMacAddressToLong() throws Exception { // TODO this test should fail this address is beyond the acceptable range for macaddresses MacAddress mac = new MacAddress(Long.MAX_VALUE); - assertEquals(Long.MAX_VALUE,mac.toLong()); + assertEquals(Long.MAX_VALUE, mac.toLong()); System.out.println(mac.toString()); } - // TODO public final void testToLong() throws Exception { - // TODO public final void testToByteArray() throws Exception { - // TODO public final void testToStringString() throws Exception { - // TODO public final void testToString() throws Exception { - // TODO public final void testGetMacAddress() throws Exception { - // TODO public final void testParse() throws Exception { + @Test + public final void testSpecificMacAddress() throws Exception { + // Test specific mac address 76:3F:76:EB:02:81 + MacAddress mac = new MacAddress(130014950130305L); + assertEquals("76:3f:76:eb:02:81", mac.toString()); + } }