From 2326164c1781c6241d34170f3b77ffd5ce2133ac Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Thu, 30 Jun 2022 17:36:42 +0530 Subject: [PATCH 1/2] api: Added information about deviceid 0 to attach root volume to VM (#6518) This is in the workflow of attaching and detaching a root volume of a VM. We can detach the root volume from a VM and Cloudstack marks it as datadisk. We can reattach the volume to make it root volume again. During attach Device ID 0 has to be passed so that the volume will be converted back to ROOT again Added a small information in the DeviceID section in Attach volume form. --- .../cloudstack/api/command/user/volume/AttachVolumeCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java index b7e10be074a..687d683309c 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/volume/AttachVolumeCmd.java @@ -49,7 +49,7 @@ public class AttachVolumeCmd extends BaseAsyncCmd implements UserCmd { ///////////////////////////////////////////////////// @Parameter(name = ApiConstants.DEVICE_ID, type = CommandType.LONG, description = "The ID of the device to map the volume to the guest OS. " - + "If no deviceID is informed, the next available deviceID will be chosen. When using a linux operating system and the hypervisor XenServer, the devices IDs will be mapped as follows:" + + "If no deviceID is informed, the next available deviceID will be chosen. Use 0 when volume needs to be attached as ROOT.
When using a linux operating system and the hypervisor XenServer, the devices IDs will be mapped as follows:" + "" + "Please refer to the docs of your hypervisor for the correct mapping of the deviceID and the actual logical disk structure.") private Long deviceId; From 637a1029228dcdf7c1f262bf8dbf9afe44fa3a36 Mon Sep 17 00:00:00 2001 From: Harikrishna Date: Fri, 1 Jul 2022 10:39:02 +0530 Subject: [PATCH 2/2] Fix for VMware VM migration with volume in local storage (#6483) * Fix VMware VM migration with volume in case of local storage * Break the loop once target host is found * Code optimisations in getting the target host guid for local storage * Fixed code smells and added unit test --- .../com/cloud/hypervisor/guru/VMwareGuru.java | 29 ++++- .../cloud/hypervisor/guru/VMwareGuruTest.java | 119 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java index a5e4140357b..5d29be9ebc9 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/guru/VMwareGuru.java @@ -27,6 +27,8 @@ import java.util.UUID; import javax.inject.Inject; +import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.dao.StoragePoolHostDao; import org.apache.cloudstack.acl.ControlledEntity; import org.apache.cloudstack.backup.Backup; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; @@ -184,6 +186,7 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co @Inject UserVmDao userVmDao; @Inject DiskOfferingDao diskOfferingDao; @Inject PhysicalNetworkDao physicalNetworkDao; + @Inject StoragePoolHostDao storagePoolHostDao; protected VMwareGuru() { super(); @@ -1070,6 +1073,13 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co return srcClusterId != null && destClusterId != null && ! srcClusterId.equals(destClusterId); } + private String getHostGuidForLocalStorage(StoragePool pool) { + List storagePoolHostVOs = storagePoolHostDao.listByPoolId(pool.getId()); + StoragePoolHostVO storagePoolHostVO = storagePoolHostVOs.get(0); + HostVO hostVO = _hostDao.findById(storagePoolHostVO.getHostId()); + return hostVO.getGuid(); + } + private String getHostGuidInTargetCluster(boolean isInterClusterMigration, Long destClusterId) { String hostGuidInTargetCluster = null; if (isInterClusterMigration) { @@ -1096,6 +1106,7 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co // OfflineVmwareMigration: specialised migration command List> volumeToFilerTo = new ArrayList>(); Long poolClusterId = null; + StoragePool targetLocalPoolForVM = null; for (Map.Entry entry : volumeToPool.entrySet()) { Volume volume = entry.getKey(); StoragePool pool = entry.getValue(); @@ -1104,13 +1115,18 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co if (pool.getClusterId() != null) { poolClusterId = pool.getClusterId(); } + if (volume.getVolumeType().equals(Volume.Type.ROOT) && pool.isLocal()) { + targetLocalPoolForVM = pool; + } volumeToFilerTo.add(new Pair(volumeTo, filerTo)); } final Long destClusterId = poolClusterId; final Long srcClusterId = vmManager.findClusterAndHostIdForVm(vm.getId()).first(); final boolean isInterClusterMigration = isInterClusterMigration(destClusterId, srcClusterId); + String targetHostGuid = getTargetHostGuid(targetLocalPoolForVM, destClusterId, isInterClusterMigration); + MigrateVmToPoolCommand migrateVmToPoolCommand = new MigrateVmToPoolCommand(vm.getInstanceName(), - volumeToFilerTo, getHostGuidInTargetCluster(isInterClusterMigration, destClusterId), true); + volumeToFilerTo, targetHostGuid, true); commands.add(migrateVmToPoolCommand); // OfflineVmwareMigration: cleanup if needed @@ -1127,6 +1143,17 @@ public class VMwareGuru extends HypervisorGuruBase implements HypervisorGuru, Co return commands; } + private String getTargetHostGuid(StoragePool targetLocalPoolForVM, Long destClusterId, boolean isInterClusterMigration) { + String targetHostGuid = null; + if (targetLocalPoolForVM != null) { + // Get the target host for local storage migration + targetHostGuid = getHostGuidForLocalStorage(targetLocalPoolForVM); + } else { + targetHostGuid = getHostGuidInTargetCluster(isInterClusterMigration, destClusterId); + } + return targetHostGuid; + } + @Override protected VirtualMachineTO toVirtualMachineTO(VirtualMachineProfile vmProfile) { return super.toVirtualMachineTO(vmProfile); diff --git a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java new file mode 100644 index 00000000000..ca618ac1b0f --- /dev/null +++ b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/guru/VMwareGuruTest.java @@ -0,0 +1,119 @@ +// 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.guru; + +import com.cloud.agent.api.Command; +import com.cloud.agent.api.MigrateVmToPoolCommand; +import com.cloud.dc.ClusterDetailsDao; +import com.cloud.host.HostVO; +import com.cloud.host.dao.HostDao; +import com.cloud.storage.StoragePool; +import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.Volume; +import com.cloud.storage.dao.StoragePoolHostDao; +import com.cloud.utils.Pair; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineManager; +import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.Spy; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.springframework.test.context.ContextConfiguration; +import org.springframework.test.context.support.AnnotationConfigContextLoader; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({VMwareGuru.class}) +@ContextConfiguration(loader = AnnotationConfigContextLoader.class) +public class VMwareGuruTest { + + @Spy + @InjectMocks + private VMwareGuru vMwareGuru = new VMwareGuru(); + + @Mock + PrimaryDataStoreDao _storagePoolDao; + + @Mock + StoragePoolHostDao storagePoolHostDao; + + @Mock + HostDao _hostDao; + + @Mock + VirtualMachineManager vmManager; + + @Mock + ClusterDetailsDao _clusterDetailsDao; + + @Before + public void testSetUp() throws Exception { + MockitoAnnotations.initMocks(this); + } + + @Test + public void finalizeMigrateForLocalStorageToHaveTargetHostGuid(){ + VirtualMachine vm = Mockito.mock(VirtualMachine.class); + Map volumeToPool = new HashMap<>(); + Volume rootVolume = Mockito.mock(Volume.class); + Volume dataVolume = Mockito.mock(Volume.class); + StoragePool localStorage = Mockito.mock(StoragePool.class); + volumeToPool.put(rootVolume, localStorage); + volumeToPool.put(dataVolume, localStorage); + + // prepare localstorage host guid + StoragePoolVO storagePoolVO = Mockito.mock(StoragePoolVO.class); + StoragePoolHostVO storagePoolHostVO = Mockito.mock(StoragePoolHostVO.class); + HostVO hostVO = Mockito.mock(HostVO.class); + + Mockito.when(localStorage.getId()).thenReturn(1L); + Mockito.when(vm.getId()).thenReturn(1L); + Mockito.when(_storagePoolDao.findById(1L)).thenReturn(storagePoolVO); + Mockito.when(rootVolume.getVolumeType()).thenReturn(Volume.Type.ROOT); + Mockito.when(dataVolume.getVolumeType()).thenReturn(Volume.Type.DATADISK); + Mockito.when(localStorage.isLocal()).thenReturn(true); + Pair clusterAndHost = new Pair<>(1L, 1L); + + Mockito.when(vmManager.findClusterAndHostIdForVm(1L)).thenReturn(clusterAndHost); + + List storagePoolHostVOS = new ArrayList<>(); + storagePoolHostVOS.add(storagePoolHostVO); + Mockito.when(storagePoolHostDao.listByPoolId(1L)).thenReturn(storagePoolHostVOS); + Mockito.when(storagePoolHostVO.getHostId()).thenReturn(2L); + Mockito.when(_hostDao.findById(2L)).thenReturn(hostVO); + Mockito.when(hostVO.getGuid()).thenReturn("HostSystem:host-a@x.x.x.x"); + + List commandsList = vMwareGuru.finalizeMigrate(vm, volumeToPool); + + MigrateVmToPoolCommand migrateVmToPoolCommand = (MigrateVmToPoolCommand) commandsList.get(0); + Assert.assertEquals("HostSystem:host-a@x.x.x.x", migrateVmToPoolCommand.getHostGuidInTargetCluster()); + } + +}