diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateHypervisorCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateHypervisorCapabilitiesCmd.java index 50984188bf5..01f7af10841 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateHypervisorCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/config/UpdateHypervisorCapabilitiesCmd.java @@ -43,6 +43,12 @@ public class UpdateHypervisorCapabilitiesCmd extends BaseCmd { @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = HypervisorCapabilitiesResponse.class, description = "ID of the hypervisor capability") private Long id; + @Parameter(name = ApiConstants.HYPERVISOR, type = CommandType.STRING, description = "the hypervisor for which the hypervisor capabilities are to be updated", since = "4.19.1") + private String hypervisor; + + @Parameter(name = ApiConstants.HYPERVISOR_VERSION, type = CommandType.STRING, description = "the hypervisor version for which the hypervisor capabilities are to be updated", since = "4.19.1") + private String hypervisorVersion; + @Parameter(name = ApiConstants.SECURITY_GROUP_EANBLED, type = CommandType.BOOLEAN, description = "set true to enable security group for this hypervisor.") private Boolean securityGroupEnabled; @@ -73,6 +79,14 @@ public class UpdateHypervisorCapabilitiesCmd extends BaseCmd { return id; } + public String getHypervisor() { + return hypervisor; + } + + public String getHypervisorVersion() { + return hypervisorVersion; + } + public Long getMaxGuestsLimit() { return maxGuestsLimit; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java index dd897218a4d..ae6ceff26c7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/vm/ImportUnmanagedInstanceCmd.java @@ -201,8 +201,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { for (Map entry : (Collection>)nicNetworkList.values()) { String nic = entry.get(VmDetailConstants.NIC); String networkUuid = entry.get(VmDetailConstants.NETWORK); - if (logger.isTraceEnabled()) { - logger.trace(String.format("nic, '%s', goes on net, '%s'", nic, networkUuid)); + if (logger.isDebugEnabled()) { + logger.debug(String.format("nic, '%s', goes on net, '%s'", nic, networkUuid)); } if (StringUtils.isAnyEmpty(nic, networkUuid) || _entityMgr.findByUuid(Network.class, networkUuid) == null) { throw new InvalidParameterValueException(String.format("Network ID: %s for NIC ID: %s is invalid", networkUuid, nic)); @@ -219,8 +219,8 @@ public class ImportUnmanagedInstanceCmd extends BaseAsyncCmd { for (Map entry : (Collection>)nicIpAddressList.values()) { String nic = entry.get(VmDetailConstants.NIC); String ipAddress = StringUtils.defaultIfEmpty(entry.get(VmDetailConstants.IP4_ADDRESS), null); - if (logger.isTraceEnabled()) { - logger.trace(String.format("nic, '%s', gets ip, '%s'", nic, ipAddress)); + if (logger.isDebugEnabled()) { + logger.debug(String.format("nic, '%s', gets ip, '%s'", nic, ipAddress)); } if (StringUtils.isEmpty(nic)) { throw new InvalidParameterValueException(String.format("NIC ID: '%s' is invalid for IP address mapping", nic)); diff --git a/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java b/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java index 53aece94964..e3a484b35b3 100644 --- a/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java +++ b/api/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManager.java @@ -30,6 +30,15 @@ public interface UnmanagedVMsManager extends VmImportService, UnmanageVMService, "If set to true, do not remove VM nics (and its MAC addresses) when unmanaging a VM, leaving them allocated but not reserved. " + "If set to false, nics are removed and MAC addresses can be reassigned", true, ConfigKey.Scope.Zone); + ConfigKey RemoteKvmInstanceDisksCopyTimeout = new ConfigKey<>(Integer.class, + "remote.kvm.instance.disks.copy.timeout", + "Advanced", + "30", + "Timeout (in mins) to prepare and copy the disks of remote KVM instance while importing the instance from an external host", + true, + ConfigKey.Scope.Global, + null); + static boolean isSupported(Hypervisor.HypervisorType hypervisorType) { return hypervisorType == VMware || hypervisorType == KVM; } diff --git a/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java b/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java index dd136d8642f..5a32ab59a7a 100644 --- a/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/CheckVolumeAnswer.java @@ -17,7 +17,6 @@ package com.cloud.agent.api; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CheckVolumeAnswer extends Answer { private long size; diff --git a/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java index b4036bebf3a..bd44b35c895 100644 --- a/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CheckVolumeCommand.java @@ -21,7 +21,6 @@ package com.cloud.agent.api; import com.cloud.agent.api.to.StorageFilerTO; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CheckVolumeCommand extends Command { String srcFile; diff --git a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java index f6d7cab4596..e79005be71b 100644 --- a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeAnswer.java @@ -17,7 +17,6 @@ package com.cloud.agent.api; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CopyRemoteVolumeAnswer extends Answer { private String remoteIp; diff --git a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java index 82bc4d7cb45..798336b0e72 100644 --- a/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java +++ b/core/src/main/java/com/cloud/agent/api/CopyRemoteVolumeCommand.java @@ -21,16 +21,13 @@ package com.cloud.agent.api; import com.cloud.agent.api.to.StorageFilerTO; -@LogLevel(LogLevel.Log4jLevel.Trace) public class CopyRemoteVolumeCommand extends Command { - String remoteIp; String username; + @LogLevel(LogLevel.Log4jLevel.Off) String password; String srcFile; - String tmpPath; - StorageFilerTO storageFilerTO; public CopyRemoteVolumeCommand(String remoteIp, String username, String password) { diff --git a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java index 8cd072f1da1..c4e590591d0 100644 --- a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsAnswer.java @@ -22,10 +22,10 @@ import org.apache.cloudstack.vm.UnmanagedInstanceTO; import java.util.HashMap; import java.util.List; -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetRemoteVmsAnswer extends Answer { private String remoteIp; + @LogLevel(LogLevel.Log4jLevel.Trace) private HashMap unmanagedInstances; List vmNames; diff --git a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java index 5c71d12dbd0..5b6b9bdd360 100644 --- a/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java +++ b/core/src/main/java/com/cloud/agent/api/GetRemoteVmsCommand.java @@ -19,11 +19,11 @@ package com.cloud.agent.api; -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetRemoteVmsCommand extends Command { String remoteIp; String username; + @LogLevel(LogLevel.Log4jLevel.Off) String password; public GetRemoteVmsCommand(String remoteIp, String username, String password) { diff --git a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java index 771d472be2a..950930ec614 100644 --- a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesAnswer.java @@ -21,10 +21,10 @@ import java.util.HashMap; import org.apache.cloudstack.vm.UnmanagedInstanceTO; -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetUnmanagedInstancesAnswer extends Answer { private String instanceName; + @LogLevel(LogLevel.Log4jLevel.Trace) private HashMap unmanagedInstances; GetUnmanagedInstancesAnswer() { diff --git a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java index 2cd80aebea1..c0b8987e152 100644 --- a/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java +++ b/core/src/main/java/com/cloud/agent/api/GetUnmanagedInstancesCommand.java @@ -28,10 +28,10 @@ import org.apache.commons.collections.CollectionUtils; * All managed instances will be filtered while trying to find unmanaged instances. */ -@LogLevel(LogLevel.Log4jLevel.Trace) public class GetUnmanagedInstancesCommand extends Command { String instanceName; + @LogLevel(LogLevel.Log4jLevel.Trace) List managedInstancesNames; public GetUnmanagedInstancesCommand() { diff --git a/debian/control b/debian/control index 3508c7b5f75..dab7b254b88 100644 --- a/debian/control +++ b/debian/control @@ -24,7 +24,7 @@ Description: CloudStack server library Package: cloudstack-agent Architecture: all -Depends: ${python:Depends}, ${python3:Depends}, openjdk-17-jre-headless | java17-runtime-headless | java17-runtime | zulu-17, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, lsb-release, ufw, apparmor +Depends: ${python:Depends}, ${python3:Depends}, openjdk-17-jre-headless | java17-runtime-headless | java17-runtime | zulu-17, cloudstack-common (= ${source:Version}), lsb-base (>= 9), openssh-client, qemu-kvm (>= 2.5) | qemu-system-x86 (>= 5.2), libvirt-bin (>= 1.3) | libvirt-daemon-system (>= 3.0), iproute2, ebtables, vlan, ipset, python3-libvirt, ethtool, iptables, cryptsetup, rng-tools, lsb-release, ufw, apparmor, cpu-checker Recommends: init-system-helpers Conflicts: cloud-agent, cloud-agent-libs, cloud-agent-deps, cloud-agent-scripts Description: CloudStack agent diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java index 50d083fba9c..e589f688cdb 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java @@ -3064,17 +3064,7 @@ public class NetworkOrchestrator extends ManagerBase implements NetworkOrchestra @Override @DB public boolean shutdownNetwork(final long networkId, final ReservationContext context, final boolean cleanupElements) { - NetworkVO network = _networksDao.findById(networkId); - if (network.getState() == Network.State.Allocated) { - logger.debug("Network is already shutdown: {}", network); - return true; - } - - if (network.getState() != Network.State.Implemented && network.getState() != Network.State.Shutdown) { - logger.debug("Network is not implemented: {}", network); - return false; - } - + NetworkVO network = null; try { //do global lock for the network network = _networksDao.acquireInLockTable(networkId, NetworkLockTimeout.value()); 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 c6d156d470b..36e28145949 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 @@ -1493,18 +1493,17 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati for (VolumeVO vol : vols) { VolumeInfo volumeInfo = volFactory.getVolume(vol.getId()); - DataTO volTO = volumeInfo.getTO(); - DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId()); DataStore dataStore = dataStoreMgr.getDataStore(vol.getPoolId(), DataStoreRole.Primary); - disk.setDetails(getDetails(volumeInfo, dataStore)); - PrimaryDataStore primaryDataStore = (PrimaryDataStore)dataStore; // This might impact other managed storages, enable requires access for migration in relevant datastore driver (currently enabled for PowerFlex storage pool only) if (primaryDataStore.isManaged() && volService.requiresAccessForMigration(volumeInfo, dataStore)) { volService.grantAccess(volFactory.getVolume(vol.getId()), dest.getHost(), dataStore); } - + // make sure this is done AFTER grantAccess, as grantAccess may change the volume's state + DataTO volTO = volumeInfo.getTO(); + DiskTO disk = storageMgr.getDiskWithThrottling(volTO, vol.getVolumeType(), vol.getDeviceId(), vol.getPath(), vm.getServiceOfferingId(), vol.getDiskOfferingId()); + disk.setDetails(getDetails(volumeInfo, dataStore)); vm.addDisk(disk); } diff --git a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java index d1532cdbef1..a4700f6cdc0 100644 --- a/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java +++ b/engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.engine.orchestration; +import static org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService.NetworkLockTimeout; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -69,6 +70,7 @@ import com.cloud.network.guru.NetworkGuru; import com.cloud.network.vpc.VpcManager; import com.cloud.network.vpc.VpcVO; import com.cloud.offerings.NetworkOfferingVO; +import com.cloud.utils.db.EntityManager; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.net.Ip; import com.cloud.vm.DomainRouterVO; @@ -93,7 +95,7 @@ import junit.framework.TestCase; @RunWith(JUnit4.class) public class NetworkOrchestratorTest extends TestCase { - NetworkOrchestrator testOrchastrator = Mockito.spy(new NetworkOrchestrator()); + NetworkOrchestrator testOrchestrator = Mockito.spy(new NetworkOrchestrator()); private String guruName = "GuestNetworkGuru"; private String dhcpProvider = "VirtualRouter"; @@ -112,21 +114,22 @@ public class NetworkOrchestratorTest extends TestCase { @Before public void setUp() { // make class-scope mocks - testOrchastrator._nicDao = mock(NicDao.class); - testOrchastrator._networksDao = mock(NetworkDao.class); - testOrchastrator._networkModel = mock(NetworkModel.class); - testOrchastrator._nicSecondaryIpDao = mock(NicSecondaryIpDao.class); - testOrchastrator._ntwkSrvcDao = mock(NetworkServiceMapDao.class); - testOrchastrator._nicIpAliasDao = mock(NicIpAliasDao.class); - testOrchastrator._ipAddressDao = mock(IPAddressDao.class); - testOrchastrator._vlanDao = mock(VlanDao.class); - testOrchastrator._networkModel = mock(NetworkModel.class); - testOrchastrator._nicExtraDhcpOptionDao = mock(NicExtraDhcpOptionDao.class); - testOrchastrator.routerDao = mock(DomainRouterDao.class); - testOrchastrator.routerNetworkDao = mock(RouterNetworkDao.class); - testOrchastrator._vpcMgr = mock(VpcManager.class); - testOrchastrator.routerJoinDao = mock(DomainRouterJoinDao.class); - testOrchastrator._ipAddrMgr = mock(IpAddressManager.class); + testOrchestrator._nicDao = mock(NicDao.class); + testOrchestrator._networksDao = mock(NetworkDao.class); + testOrchestrator._networkModel = mock(NetworkModel.class); + testOrchestrator._nicSecondaryIpDao = mock(NicSecondaryIpDao.class); + testOrchestrator._ntwkSrvcDao = mock(NetworkServiceMapDao.class); + testOrchestrator._nicIpAliasDao = mock(NicIpAliasDao.class); + testOrchestrator._ipAddressDao = mock(IPAddressDao.class); + testOrchestrator._vlanDao = mock(VlanDao.class); + testOrchestrator._networkModel = mock(NetworkModel.class); + testOrchestrator._nicExtraDhcpOptionDao = mock(NicExtraDhcpOptionDao.class); + testOrchestrator.routerDao = mock(DomainRouterDao.class); + testOrchestrator.routerNetworkDao = mock(RouterNetworkDao.class); + testOrchestrator._vpcMgr = mock(VpcManager.class); + testOrchestrator.routerJoinDao = mock(DomainRouterJoinDao.class); + testOrchestrator._ipAddrMgr = mock(IpAddressManager.class); + testOrchestrator._entityMgr = mock(EntityManager.class); DhcpServiceProvider provider = mock(DhcpServiceProvider.class); Map capabilities = new HashMap(); @@ -135,13 +138,13 @@ public class NetworkOrchestratorTest extends TestCase { when(provider.getCapabilities()).thenReturn(services); capabilities.put(Network.Capability.DhcpAccrossMultipleSubnets, "true"); - when(testOrchastrator._ntwkSrvcDao.getProviderForServiceInNetwork(ArgumentMatchers.anyLong(), ArgumentMatchers.eq(Service.Dhcp))).thenReturn(dhcpProvider); - when(testOrchastrator._networkModel.getElementImplementingProvider(dhcpProvider)).thenReturn(provider); + when(testOrchestrator._ntwkSrvcDao.getProviderForServiceInNetwork(ArgumentMatchers.anyLong(), ArgumentMatchers.eq(Service.Dhcp))).thenReturn(dhcpProvider); + when(testOrchestrator._networkModel.getElementImplementingProvider(dhcpProvider)).thenReturn(provider); when(guru.getName()).thenReturn(guruName); List networkGurus = new ArrayList(); networkGurus.add(guru); - testOrchastrator.networkGurus = networkGurus; + testOrchestrator.networkGurus = networkGurus; when(networkOffering.getGuestType()).thenReturn(GuestType.L2); when(networkOffering.getId()).thenReturn(networkOfferingId); @@ -156,21 +159,21 @@ public class NetworkOrchestratorTest extends TestCase { // make sure that release dhcp will be called when(vm.getType()).thenReturn(Type.User); - when(testOrchastrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(true); + when(testOrchestrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(true); when(network.getTrafficType()).thenReturn(TrafficType.Guest); when(network.getGuestType()).thenReturn(GuestType.Shared); - when(testOrchastrator._nicDao.listByNetworkIdTypeAndGatewayAndBroadcastUri(nic.getNetworkId(), VirtualMachine.Type.User, nic.getIPv4Gateway(), nic.getBroadcastUri())) + when(testOrchestrator._nicDao.listByNetworkIdTypeAndGatewayAndBroadcastUri(nic.getNetworkId(), VirtualMachine.Type.User, nic.getIPv4Gateway(), nic.getBroadcastUri())) .thenReturn(new ArrayList()); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, times(1)).setState(Nic.State.Deallocating); - verify(testOrchastrator._networkModel, times(2)).getElementImplementingProvider(dhcpProvider); - verify(testOrchastrator._ntwkSrvcDao, times(2)).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); - verify(testOrchastrator._networksDao, times(2)).findById(nic.getNetworkId()); + verify(testOrchestrator._networkModel, times(2)).getElementImplementingProvider(dhcpProvider); + verify(testOrchestrator._ntwkSrvcDao, times(2)).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); + verify(testOrchestrator._networksDao, times(2)).findById(nic.getNetworkId()); } @Test public void testDontRemoveDhcpServiceFromDomainRouter() { @@ -183,14 +186,14 @@ public class NetworkOrchestratorTest extends TestCase { when(vm.getType()).thenReturn(Type.DomainRouter); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, times(1)).setState(Nic.State.Deallocating); - verify(testOrchastrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); - verify(testOrchastrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); - verify(testOrchastrator._networksDao, times(1)).findById(nic.getNetworkId()); + verify(testOrchestrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); + verify(testOrchestrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); + verify(testOrchestrator._networksDao, times(1)).findById(nic.getNetworkId()); } @Test public void testDontRemoveDhcpServiceWhenNotProvided() { @@ -201,45 +204,45 @@ public class NetworkOrchestratorTest extends TestCase { // make sure that release dhcp will *not* be called when(vm.getType()).thenReturn(Type.User); - when(testOrchastrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(false); + when(testOrchestrator._networkModel.areServicesSupportedInNetwork(network.getId(), Service.Dhcp)).thenReturn(false); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, times(1)).setState(Nic.State.Deallocating); - verify(testOrchastrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); - verify(testOrchastrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); - verify(testOrchastrator._networksDao, times(1)).findById(nic.getNetworkId()); + verify(testOrchestrator._networkModel, never()).getElementImplementingProvider(dhcpProvider); + verify(testOrchestrator._ntwkSrvcDao, never()).getProviderForServiceInNetwork(network.getId(), Service.Dhcp); + verify(testOrchestrator._networksDao, times(1)).findById(nic.getNetworkId()); } @Test public void testCheckL2OfferingServicesEmptyServices() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(new ArrayList<>()); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(new ArrayList<>()); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test public void testCheckL2OfferingServicesUserDataOnly() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData)); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData)); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test(expected = InvalidParameterValueException.class) public void testCheckL2OfferingServicesMultipleServicesIncludingUserData() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData, Service.Dhcp)); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.UserData, Service.Dhcp)); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(true); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test(expected = InvalidParameterValueException.class) public void testCheckL2OfferingServicesMultipleServicesNotIncludingUserData() { - when(testOrchastrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.Dns, Service.Dhcp)); - when(testOrchastrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); - testOrchastrator.checkL2OfferingServices(networkOffering); + when(testOrchestrator._networkModel.listNetworkOfferingServices(networkOfferingId)).thenReturn(Arrays.asList(Service.Dns, Service.Dhcp)); + when(testOrchestrator._networkModel.areServicesSupportedByNetworkOffering(networkOfferingId, Service.UserData)).thenReturn(false); + testOrchestrator.checkL2OfferingServices(networkOffering); } @Test @@ -251,7 +254,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, null, "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert("192.168.100.150", "192.168.100.1", "255.255.255.0", nicProfile, 1, 1); } @@ -265,7 +268,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert("192.168.100.150", "192.168.100.1", "255.255.255.0", nicProfile, 1, 0); } @@ -292,7 +295,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, null, requestedIpv4Address); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 0, 0); } @@ -319,7 +322,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, ipv4Gateway, "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 1, 0); } @@ -345,7 +348,7 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", ipv4Netmask, "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 1, 0); } @@ -357,9 +360,9 @@ public class NetworkOrchestratorTest extends TestCase { configureTestConfigureNicProfileBasedOnRequestedIpTests(nicProfile, 0l, false, IPAddressVO.State.Free, "192.168.100.1", "255.255.255.0", "00-88-14-4D-4C-FB", requestedNicProfile, "00-88-14-4D-4C-FB", "192.168.100.150"); - when(testOrchastrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); + when(testOrchestrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); - testOrchastrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); + testOrchestrator.configureNicProfileBasedOnRequestedIp(requestedNicProfile, nicProfile, network); verifyAndAssert(null, null, null, nicProfile, 0, 0); } @@ -375,21 +378,21 @@ public class NetworkOrchestratorTest extends TestCase { when(ipVoSpy.getState()).thenReturn(state); if (ipVoIsNull) { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); } else { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); } VlanVO vlanSpy = Mockito.spy(new VlanVO(Vlan.VlanType.DirectAttached, "vlanTag", vlanGateway, vlanNetmask, 0l, "192.168.100.100 - 192.168.100.200", 0l, new Long(0l), "ip6Gateway", "ip6Cidr", "ip6Range")); Mockito.doReturn(0l).when(vlanSpy).getId(); - when(testOrchastrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(vlanSpy); - when(testOrchastrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); - when(testOrchastrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); - when(testOrchastrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); + when(testOrchestrator._vlanDao.findByNetworkIdAndIpv4(Mockito.anyLong(), Mockito.anyString())).thenReturn(vlanSpy); + when(testOrchestrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); + when(testOrchestrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); try { - when(testOrchastrator._networkModel.getNextAvailableMacAddressInNetwork(Mockito.anyLong())).thenReturn(macAddress); + when(testOrchestrator._networkModel.getNextAvailableMacAddressInNetwork(Mockito.anyLong())).thenReturn(macAddress); } catch (InsufficientAddressCapacityException e) { e.printStackTrace(); } @@ -397,9 +400,9 @@ public class NetworkOrchestratorTest extends TestCase { private void verifyAndAssert(String requestedIpv4Address, String ipv4Gateway, String ipv4Netmask, NicProfile nicProfile, int acquireLockAndCheckIfIpv4IsFreeTimes, int nextMacAddressTimes) { - verify(testOrchastrator, times(acquireLockAndCheckIfIpv4IsFreeTimes)).acquireLockAndCheckIfIpv4IsFree(Mockito.any(Network.class), Mockito.anyString()); + verify(testOrchestrator, times(acquireLockAndCheckIfIpv4IsFreeTimes)).acquireLockAndCheckIfIpv4IsFree(Mockito.any(Network.class), Mockito.anyString()); try { - verify(testOrchastrator._networkModel, times(nextMacAddressTimes)).getNextAvailableMacAddressInNetwork(Mockito.anyLong()); + verify(testOrchestrator._networkModel, times(nextMacAddressTimes)).getNextAvailableMacAddressInNetwork(Mockito.anyLong()); } catch (InsufficientAddressCapacityException e) { e.printStackTrace(); } @@ -441,27 +444,27 @@ public class NetworkOrchestratorTest extends TestCase { ipVoSpy.setState(state); ipVoSpy.setState(state); if (isIPAddressVONull) { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(null); } else { - when(testOrchastrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy); } - when(testOrchastrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); - when(testOrchastrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); - when(testOrchastrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); + when(testOrchestrator._ipAddressDao.acquireInLockTable(Mockito.anyLong())).thenReturn(ipVoSpy); + when(testOrchestrator._ipAddressDao.releaseFromLockTable(Mockito.anyLong())).thenReturn(true); + when(testOrchestrator._ipAddressDao.update(Mockito.anyLong(), Mockito.any(IPAddressVO.class))).thenReturn(true); - testOrchastrator.acquireLockAndCheckIfIpv4IsFree(network, "192.168.100.150"); + testOrchestrator.acquireLockAndCheckIfIpv4IsFree(network, "192.168.100.150"); - verify(testOrchastrator._ipAddressDao, Mockito.times(findByIpTimes)).findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString()); - verify(testOrchastrator._ipAddressDao, Mockito.times(acquireLockTimes)).acquireInLockTable(Mockito.anyLong()); - verify(testOrchastrator._ipAddressDao, Mockito.times(releaseFromLockTimes)).releaseFromLockTable(Mockito.anyLong()); - verify(testOrchastrator._ipAddressDao, Mockito.times(updateTimes)).update(Mockito.anyLong(), Mockito.any(IPAddressVO.class)); - verify(testOrchastrator, Mockito.times(validateTimes)).validateLockedRequestedIp(Mockito.any(IPAddressVO.class), Mockito.any(IPAddressVO.class)); + verify(testOrchestrator._ipAddressDao, Mockito.times(findByIpTimes)).findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString()); + verify(testOrchestrator._ipAddressDao, Mockito.times(acquireLockTimes)).acquireInLockTable(Mockito.anyLong()); + verify(testOrchestrator._ipAddressDao, Mockito.times(releaseFromLockTimes)).releaseFromLockTable(Mockito.anyLong()); + verify(testOrchestrator._ipAddressDao, Mockito.times(updateTimes)).update(Mockito.anyLong(), Mockito.any(IPAddressVO.class)); + verify(testOrchestrator, Mockito.times(validateTimes)).validateLockedRequestedIp(Mockito.any(IPAddressVO.class), Mockito.any(IPAddressVO.class)); } @Test(expected = InvalidParameterValueException.class) public void validateLockedRequestedIpTestNullLockedIp() { IPAddressVO ipVoSpy = Mockito.spy(new IPAddressVO(new Ip("192.168.100.100"), 0l, 0l, 0l, true)); - testOrchastrator.validateLockedRequestedIp(ipVoSpy, null); + testOrchestrator.validateLockedRequestedIp(ipVoSpy, null); } @Test @@ -476,7 +479,7 @@ public class NetworkOrchestratorTest extends TestCase { IPAddressVO lockedIp = ipVoSpy; lockedIp.setState(states[i]); try { - testOrchastrator.validateLockedRequestedIp(ipVoSpy, lockedIp); + testOrchestrator.validateLockedRequestedIp(ipVoSpy, lockedIp); } catch (InvalidParameterValueException e) { expectedException = true; } @@ -489,7 +492,7 @@ public class NetworkOrchestratorTest extends TestCase { IPAddressVO ipVoSpy = Mockito.spy(new IPAddressVO(new Ip("192.168.100.100"), 0l, 0l, 0l, true)); IPAddressVO lockedIp = ipVoSpy; lockedIp.setState(State.Free); - testOrchastrator.validateLockedRequestedIp(ipVoSpy, lockedIp); + testOrchestrator.validateLockedRequestedIp(ipVoSpy, lockedIp); } @Test @@ -500,16 +503,16 @@ public class NetworkOrchestratorTest extends TestCase { when(vm.getType()).thenReturn(Type.User); when(network.getGuruName()).thenReturn(guruName); - when(testOrchastrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); + when(testOrchestrator._networksDao.findById(nic.getNetworkId())).thenReturn(network); Long nicId = 1L; when(nic.getId()).thenReturn(nicId); when(vm.getParameter(VirtualMachineProfile.Param.PreserveNics)).thenReturn(true); - testOrchastrator.removeNic(vm, nic); + testOrchestrator.removeNic(vm, nic); verify(nic, never()).setState(Nic.State.Deallocating); - verify(testOrchastrator._nicDao, never()).remove(nicId); + verify(testOrchestrator._nicDao, never()).remove(nicId); } public void encodeVlanIdIntoBroadcastUriTestVxlan() { @@ -568,7 +571,7 @@ public class NetworkOrchestratorTest extends TestCase { @Test(expected = InvalidParameterValueException.class) public void encodeVlanIdIntoBroadcastUriTestNullNetwork() { - URI resultUri = testOrchastrator.encodeVlanIdIntoBroadcastUri("vxlan://123", null); + URI resultUri = testOrchestrator.encodeVlanIdIntoBroadcastUri("vxlan://123", null); } private void encodeVlanIdIntoBroadcastUriPrepareAndTest(String vlanId, String isolationMethod, String expectedIsolation, String expectedUri) { @@ -577,7 +580,7 @@ public class NetworkOrchestratorTest extends TestCase { isolationMethods.add(isolationMethod); physicalNetwork.setIsolationMethods(isolationMethods); - URI resultUri = testOrchastrator.encodeVlanIdIntoBroadcastUri(vlanId, physicalNetwork); + URI resultUri = testOrchestrator.encodeVlanIdIntoBroadcastUri(vlanId, physicalNetwork); Assert.assertEquals(expectedIsolation, resultUri.getScheme()); Assert.assertEquals(expectedUri, resultUri.toString()); @@ -595,17 +598,17 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(network.getDns2()).thenReturn(ip4Dns[1]); Mockito.when(network.getIp6Dns1()).thenReturn(ip6Dns[0]); Mockito.when(network.getIp6Dns2()).thenReturn(ip6Dns[1]); - Mockito.when(testOrchastrator._networkModel.getNetworkRate(networkId, vmId)).thenReturn(networkRate); + Mockito.when(testOrchestrator._networkModel.getNetworkRate(networkId, vmId)).thenReturn(networkRate); NicVO nicVO = Mockito.mock(NicVO.class); Mockito.when(nicVO.isDefaultNic()).thenReturn(isDefaultNic); - Mockito.when(testOrchastrator._nicDao.findById(nicId)).thenReturn(nicVO); - Mockito.when(testOrchastrator._nicDao.update(nicId, nicVO)).thenReturn(true); - Mockito.when(testOrchastrator._networkModel.isSecurityGroupSupportedInNetwork(network)).thenReturn(false); - Mockito.when(testOrchastrator._networkModel.getNetworkTag(hypervisorType, network)).thenReturn(null); - Mockito.when(testOrchastrator._ntwkSrvcDao.getDistinctProviders(networkId)).thenReturn(new ArrayList<>()); - testOrchastrator.networkElements = new ArrayList<>(); - Mockito.when(testOrchastrator._nicExtraDhcpOptionDao.listByNicId(nicId)).thenReturn(new ArrayList<>()); - Mockito.when(testOrchastrator._ntwkSrvcDao.areServicesSupportedInNetwork(networkId, Service.Dhcp)).thenReturn(false); + Mockito.when(testOrchestrator._nicDao.findById(nicId)).thenReturn(nicVO); + Mockito.when(testOrchestrator._nicDao.update(nicId, nicVO)).thenReturn(true); + Mockito.when(testOrchestrator._networkModel.isSecurityGroupSupportedInNetwork(network)).thenReturn(false); + Mockito.when(testOrchestrator._networkModel.getNetworkTag(hypervisorType, network)).thenReturn(null); + Mockito.when(testOrchestrator._ntwkSrvcDao.getDistinctProviders(networkId)).thenReturn(new ArrayList<>()); + testOrchestrator.networkElements = new ArrayList<>(); + Mockito.when(testOrchestrator._nicExtraDhcpOptionDao.listByNicId(nicId)).thenReturn(new ArrayList<>()); + Mockito.when(testOrchestrator._ntwkSrvcDao.areServicesSupportedInNetwork(networkId, Service.Dhcp)).thenReturn(false); VirtualMachineProfile virtualMachineProfile = Mockito.mock(VirtualMachineProfile.class); Mockito.when(virtualMachineProfile.getType()).thenReturn(vmType); Mockito.when(virtualMachineProfile.getId()).thenReturn(vmId); @@ -634,7 +637,7 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(vpcVO.getIp4Dns1()).thenReturn(null); Mockito.when(vpcVO.getIp6Dns1()).thenReturn(null); } - Mockito.when(testOrchastrator._vpcMgr.getActiveVpc(vpcId)).thenReturn(vpcVO); + Mockito.when(testOrchestrator._vpcMgr.getActiveVpc(vpcId)).thenReturn(vpcVO); } else { Mockito.when(routerVO.getVpcId()).thenReturn(null); Long routerNetworkId = 2L; @@ -648,13 +651,13 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(routerNetworkVO.getDns1()).thenReturn(null); Mockito.when(routerNetworkVO.getIp6Dns1()).thenReturn(null); } - Mockito.when(testOrchastrator.routerNetworkDao.getRouterNetworks(vmId)).thenReturn(List.of(routerNetworkId)); - Mockito.when(testOrchastrator._networksDao.findById(routerNetworkId)).thenReturn(routerNetworkVO); + Mockito.when(testOrchestrator.routerNetworkDao.getRouterNetworks(vmId)).thenReturn(List.of(routerNetworkId)); + Mockito.when(testOrchestrator._networksDao.findById(routerNetworkId)).thenReturn(routerNetworkVO); } - Mockito.when(testOrchastrator.routerDao.findById(vmId)).thenReturn(routerVO); + Mockito.when(testOrchestrator.routerDao.findById(vmId)).thenReturn(routerVO); NicProfile profile = null; try { - profile = testOrchastrator.prepareNic(virtualMachineProfile, deployDestination, reservationContext, nicId, network); + profile = testOrchestrator.prepareNic(virtualMachineProfile, deployDestination, reservationContext, nicId, network); } catch (InsufficientCapacityException | ResourceUnavailableException e) { Assert.fail(String.format("Failure with exception %s", e.getMessage())); } @@ -723,7 +726,7 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); Mockito.when(network.getGateway()).thenReturn(networkGateway); Mockito.when(network.getCidr()).thenReturn(networkCidr); - Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); + Pair pair = testOrchestrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); Assert.assertNotNull(pair); Assert.assertEquals(networkGateway, pair.first()); Assert.assertEquals(networkNetmask, pair.second()); @@ -743,9 +746,9 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(vlan.getVlanNetmask()).thenReturn(defaultNetworkNetmask); Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Basic); Mockito.when(ipAddressVO.getVlanId()).thenReturn(1L); - Mockito.when(testOrchastrator._vlanDao.findById(1L)).thenReturn(vlan); - Mockito.when(testOrchastrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO); - Pair pair = testOrchastrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); + Mockito.when(testOrchestrator._vlanDao.findById(1L)).thenReturn(vlan); + Mockito.when(testOrchestrator._ipAddressDao.findByIp(ipAddress)).thenReturn(ipAddressVO); + Pair pair = testOrchestrator.getNetworkGatewayAndNetmaskForNicImport(network, dataCenter, ipAddress); Assert.assertNotNull(pair); Assert.assertEquals(defaultNetworkGateway, pair.first()); Assert.assertEquals(defaultNetworkNetmask, pair.second()); @@ -757,7 +760,7 @@ public class NetworkOrchestratorTest extends TestCase { DataCenter dataCenter = Mockito.mock(DataCenter.class); Network.IpAddresses ipAddresses = Mockito.mock(Network.IpAddresses.class); Mockito.when(network.getGuestType()).thenReturn(GuestType.L2); - Assert.assertNull(testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses)); + Assert.assertNull(testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses)); } @Test @@ -769,8 +772,8 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(dataCenter.getNetworkType()).thenReturn(DataCenter.NetworkType.Advanced); String ipAddress = "10.1.10.10"; Mockito.when(ipAddresses.getIp4Address()).thenReturn(ipAddress); - Mockito.when(testOrchastrator._ipAddrMgr.acquireGuestIpAddress(network, ipAddress)).thenReturn(ipAddress); - String guestIp = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddrMgr.acquireGuestIpAddress(network, ipAddress)).thenReturn(ipAddress); + String guestIp = testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); Assert.assertEquals(ipAddress, guestIp); } @@ -791,8 +794,8 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(ipAddressVO.getState()).thenReturn(State.Free); Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); - Mockito.when(testOrchastrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO); - String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddressDao.findBySourceNetworkIdAndDatacenterIdAndState(networkId, dataCenterId, State.Free)).thenReturn(ipAddressVO); + String ipAddress = testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); Assert.assertEquals(freeIp, ipAddress); } @@ -814,8 +817,8 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp); - Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); - String ipAddress = testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); + String ipAddress = testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); Assert.assertEquals(requestedIp, ipAddress); } @@ -837,7 +840,54 @@ public class NetworkOrchestratorTest extends TestCase { Mockito.when(network.getId()).thenReturn(networkId); Mockito.when(dataCenter.getId()).thenReturn(dataCenterId); Mockito.when(ipAddresses.getIp4Address()).thenReturn(requestedIp); - Mockito.when(testOrchastrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); - testOrchastrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + Mockito.when(testOrchestrator._ipAddressDao.findByIp(requestedIp)).thenReturn(ipAddressVO); + testOrchestrator.getSelectedIpForNicImport(network, dataCenter, ipAddresses); + } + + @Test + public void testShutdownNetworkAcquireLockFailed() { + ReservationContext reservationContext = Mockito.mock(ReservationContext.class); + NetworkVO network = mock(NetworkVO.class); + long networkId = 1; + when(testOrchestrator._networksDao.acquireInLockTable(Mockito.anyLong(), Mockito.anyInt())).thenReturn(null); + + boolean shutdownNetworkStatus = testOrchestrator.shutdownNetwork(networkId, reservationContext, false); + Assert.assertFalse(shutdownNetworkStatus); + + verify(testOrchestrator._networksDao, times(1)).acquireInLockTable(networkId, NetworkLockTimeout.value()); + } + + @Test + public void testShutdownNetworkInAllocatedState() { + ReservationContext reservationContext = Mockito.mock(ReservationContext.class); + NetworkVO network = mock(NetworkVO.class); + long networkId = 1; + when(testOrchestrator._networksDao.acquireInLockTable(Mockito.anyLong(), Mockito.anyInt())).thenReturn(network); + when(network.getId()).thenReturn(networkId); + when(network.getState()).thenReturn(Network.State.Allocated); + + boolean shutdownNetworkStatus = testOrchestrator.shutdownNetwork(networkId, reservationContext, false); + Assert.assertTrue(shutdownNetworkStatus); + + verify(network, times(1)).getState(); + verify(testOrchestrator._networksDao, times(1)).acquireInLockTable(networkId, NetworkLockTimeout.value()); + verify(testOrchestrator._networksDao, times(1)).releaseFromLockTable(networkId); + } + + @Test + public void testShutdownNetworkInImplementingState() { + ReservationContext reservationContext = Mockito.mock(ReservationContext.class); + NetworkVO network = mock(NetworkVO.class); + long networkId = 1; + when(testOrchestrator._networksDao.acquireInLockTable(Mockito.anyLong(), Mockito.anyInt())).thenReturn(network); + when(network.getId()).thenReturn(networkId); + when(network.getState()).thenReturn(Network.State.Implementing); + + boolean shutdownNetworkStatus = testOrchestrator.shutdownNetwork(networkId, reservationContext, false); + Assert.assertFalse(shutdownNetworkStatus); + + verify(network, times(3)).getState(); + verify(testOrchestrator._networksDao, times(1)).acquireInLockTable(networkId, NetworkLockTimeout.value()); + verify(testOrchestrator._networksDao, times(1)).releaseFromLockTable(networkId); } } diff --git a/engine/schema/src/main/java/com/cloud/hypervisor/HypervisorCapabilitiesVO.java b/engine/schema/src/main/java/com/cloud/hypervisor/HypervisorCapabilitiesVO.java index 4455c7491dd..a3b03280fdf 100644 --- a/engine/schema/src/main/java/com/cloud/hypervisor/HypervisorCapabilitiesVO.java +++ b/engine/schema/src/main/java/com/cloud/hypervisor/HypervisorCapabilitiesVO.java @@ -80,6 +80,18 @@ public class HypervisorCapabilitiesVO implements HypervisorCapabilities { this.uuid = UUID.randomUUID().toString(); } + public HypervisorCapabilitiesVO(HypervisorCapabilitiesVO source) { + this.hypervisorType = source.getHypervisorType(); + this.hypervisorVersion = source.getHypervisorVersion(); + this.maxGuestsLimit = source.getMaxGuestsLimit(); + this.maxDataVolumesLimit = source.getMaxDataVolumesLimit(); + this.maxHostsPerCluster = source.getMaxHostsPerCluster(); + this.securityGroupEnabled = source.isSecurityGroupEnabled(); + this.storageMotionSupported = source.isStorageMotionSupported(); + this.vmSnapshotEnabled = source.isVmSnapshotEnabled(); + this.uuid = UUID.randomUUID().toString(); + } + /** * @param hypervisorType the hypervisorType to set */ diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java index 879faaf5c90..0d7aa703a8c 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDao.java @@ -75,8 +75,10 @@ public interface VmStatsDao extends GenericDao { /** * Removes (expunges) all VM stats with {@code timestamp} less than * a given Date. - * @param limit the maximum date to keep stored. Records that exceed this limit will be removed. + * @param limitDate the maximum date to keep stored. Records that exceed this limit will be removed. + * @param limitPerQuery the maximum amount of rows to be removed in a single query. We loop if there are still rows to be removed after a given query. + * If 0 or negative, no limit is used. */ - void removeAllByTimestampLessThan(Date limit); + void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery); } diff --git a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java index 1bef8f0626c..aa58e489364 100644 --- a/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java +++ b/engine/schema/src/main/java/com/cloud/vm/dao/VmStatsDaoImpl.java @@ -21,6 +21,8 @@ import java.util.List; import javax.annotation.PostConstruct; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.springframework.stereotype.Component; import com.cloud.utils.db.Filter; @@ -33,6 +35,8 @@ import com.cloud.vm.VmStatsVO; @Component public class VmStatsDaoImpl extends GenericDaoBase implements VmStatsDao { + protected Logger logger = LogManager.getLogger(getClass()); + protected SearchBuilder vmIdSearch; protected SearchBuilder vmIdTimestampGreaterThanEqualSearch; protected SearchBuilder vmIdTimestampLessThanEqualSearch; @@ -113,10 +117,22 @@ public class VmStatsDaoImpl extends GenericDaoBase implements V } @Override - public void removeAllByTimestampLessThan(Date limit) { + public void removeAllByTimestampLessThan(Date limitDate, long limitPerQuery) { SearchCriteria sc = timestampSearch.create(); - sc.setParameters("timestamp", limit); - expunge(sc); + sc.setParameters("timestamp", limitDate); + + logger.debug(String.format("Starting to remove all vm_stats rows older than [%s].", limitDate)); + + long totalRemoved = 0; + long removed; + + do { + removed = expunge(sc, limitPerQuery); + totalRemoved += removed; + logger.trace(String.format("Removed [%s] vm_stats rows on the last update and a sum of [%s] vm_stats rows older than [%s] until now.", removed, totalRemoved, limitDate)); + } while (limitPerQuery > 0 && removed >= limitPerQuery); + + logger.info(String.format("Removed a total of [%s] vm_stats rows older than [%s].", totalRemoved, limitDate)); } } diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java index 21c5dc76d96..84b88c215ca 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java @@ -130,7 +130,7 @@ public class ImageStoreDaoImpl extends GenericDaoBase implem } if (scope.getScopeId() != null) { SearchCriteria scc = createSearchCriteria(); - scc.addOr("scope", SearchCriteria.Op.EQ, ScopeType.ZONE); + scc.addOr("scope", SearchCriteria.Op.EQ, ScopeType.REGION); scc.addOr("dcId", SearchCriteria.Op.EQ, scope.getScopeId()); sc.addAnd("scope", SearchCriteria.Op.SC, scc); } diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41900to41910.sql b/engine/schema/src/main/resources/META-INF/db/schema-41900to41910.sql index bdb23d9844c..0cb10f4a0ef 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41900to41910.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41900to41910.sql @@ -65,3 +65,8 @@ CREATE TABLE IF NOT EXISTS `cloud_usage`.`usage_vpc` ( CALL `cloud_usage`.`IDEMPOTENT_ADD_COLUMN`('cloud_usage.cloud_usage', 'state', 'VARCHAR(100) DEFAULT NULL'); CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.user_data', 'removed', 'datetime COMMENT "date removed or null, if still present"'); + +-- Update options for config - host.allocators.order +UPDATE `cloud`.`configuration` SET + `options` = 'FirstFitRouting,RandomAllocator,TestingAllocator,FirstFitAllocator,RecreateHostAllocator' +WHERE `name` = 'host.allocators.order'; diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java index 03aa5b50988..70e79e3252b 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategy.java @@ -24,6 +24,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Random; import java.util.Set; import java.util.UUID; @@ -44,6 +45,7 @@ import org.apache.cloudstack.engine.subsystem.api.storage.EndPoint; import org.apache.cloudstack.engine.subsystem.api.storage.EndPointSelector; import org.apache.cloudstack.engine.subsystem.api.storage.HostScope; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine; +import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStore; import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event; import org.apache.cloudstack.engine.subsystem.api.storage.PrimaryDataStoreDriver; import org.apache.cloudstack.engine.subsystem.api.storage.Scope; @@ -69,7 +71,6 @@ import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; -import org.apache.commons.collections.MapUtils; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; @@ -82,7 +83,6 @@ import com.cloud.agent.api.MigrateCommand.MigrateDiskInfo; import com.cloud.agent.api.ModifyTargetsAnswer; import com.cloud.agent.api.ModifyTargetsCommand; import com.cloud.agent.api.PrepareForMigrationCommand; -import com.cloud.agent.api.storage.CheckStorageAvailabilityCommand; import com.cloud.agent.api.storage.CopyVolumeAnswer; import com.cloud.agent.api.storage.CopyVolumeCommand; import com.cloud.agent.api.storage.MigrateVolumeAnswer; @@ -141,12 +141,16 @@ import java.util.HashSet; import java.util.stream.Collectors; import org.apache.commons.collections.CollectionUtils; +import static org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.KVM_VM_IMPORT_DEFAULT_TEMPLATE_NAME; +import static org.apache.cloudstack.vm.UnmanagedVMsManagerImpl.VM_IMPORT_DEFAULT_TEMPLATE_NAME; + public class StorageSystemDataMotionStrategy implements DataMotionStrategy { protected Logger logger = LogManager.getLogger(getClass()); private static final Random RANDOM = new Random(System.nanoTime()); private static final int LOCK_TIME_IN_SECONDS = 300; private static final String OPERATION_NOT_SUPPORTED = "This operation is not supported."; + @Inject protected AgentManager agentManager; @Inject @@ -684,8 +688,6 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { private void handleVolumeMigrationFromNonManagedStorageToManagedStorage(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo, AsyncCompletionCallback callback) { - String errMsg = null; - try { HypervisorType hypervisorType = srcVolumeInfo.getHypervisorType(); @@ -696,37 +698,21 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { if (HypervisorType.XenServer.equals(hypervisorType)) { handleVolumeMigrationForXenServer(srcVolumeInfo, destVolumeInfo); - } - else { + destVolumeInfo = _volumeDataFactory.getVolume(destVolumeInfo.getId(), destVolumeInfo.getDataStore()); + DataTO dataTO = destVolumeInfo.getTO(); + CopyCmdAnswer copyCmdAnswer = new CopyCmdAnswer(dataTO); + CopyCommandResult result = new CopyCommandResult(null, copyCmdAnswer); + callback.complete(result); + } else { handleVolumeMigrationForKVM(srcVolumeInfo, destVolumeInfo, callback); } } catch (Exception ex) { - errMsg = "Migration operation failed in 'StorageSystemDataMotionStrategy.handleVolumeMigrationFromNonManagedStorageToManagedStorage': " + + String errMsg = "Migration operation failed in 'StorageSystemDataMotionStrategy.handleVolumeMigrationFromNonManagedStorageToManagedStorage': " + ex.getMessage(); throw new CloudRuntimeException(errMsg, ex); } - finally { - CopyCmdAnswer copyCmdAnswer; - - if (errMsg != null) { - copyCmdAnswer = new CopyCmdAnswer(errMsg); - } - else { - destVolumeInfo = _volumeDataFactory.getVolume(destVolumeInfo.getId(), destVolumeInfo.getDataStore()); - - DataTO dataTO = destVolumeInfo.getTO(); - - copyCmdAnswer = new CopyCmdAnswer(dataTO); - } - - CopyCommandResult result = new CopyCommandResult(null, copyCmdAnswer); - - result.setResult(errMsg); - - callback.complete(result); - } } private void handleVolumeMigrationForXenServer(VolumeInfo srcVolumeInfo, VolumeInfo destVolumeInfo) { @@ -845,12 +831,25 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { checkAvailableForMigration(vm); String errMsg = null; + HostVO hostVO = null; try { destVolumeInfo.getDataStore().getDriver().createAsync(destVolumeInfo.getDataStore(), destVolumeInfo, null); VolumeVO volumeVO = _volumeDao.findById(destVolumeInfo.getId()); updatePathFromScsiName(volumeVO); destVolumeInfo = _volumeDataFactory.getVolume(destVolumeInfo.getId(), destVolumeInfo.getDataStore()); - HostVO hostVO = getHostOnWhichToExecuteMigrationCommand(srcVolumeInfo, destVolumeInfo); + hostVO = getHostOnWhichToExecuteMigrationCommand(srcVolumeInfo, destVolumeInfo); + + // if managed we need to grant access + PrimaryDataStore pds = (PrimaryDataStore)this.dataStoreMgr.getPrimaryDataStore(destVolumeInfo.getDataStore().getUuid()); + if (pds == null) { + throw new CloudRuntimeException("Unable to find primary data store driver for this volume"); + } + + // grant access (for managed volumes) + _volumeService.grantAccess(destVolumeInfo, hostVO, destVolumeInfo.getDataStore()); + + // re-retrieve volume to get any updated information from grant + destVolumeInfo = _volumeDataFactory.getVolume(destVolumeInfo.getId(), destVolumeInfo.getDataStore()); // migrate the volume via the hypervisor String path = migrateVolumeForKVM(srcVolumeInfo, destVolumeInfo, hostVO, "Unable to migrate the volume from non-managed storage to managed storage"); @@ -871,6 +870,18 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { throw new CloudRuntimeException(errMsg, ex); } } finally { + // revoke access (for managed volumes) + if (hostVO != null) { + try { + _volumeService.revokeAccess(destVolumeInfo, hostVO, destVolumeInfo.getDataStore()); + } catch (Exception e) { + logger.warn(String.format("Failed to revoke access for volume 'name=%s,uuid=%s' after a migration attempt", destVolumeInfo.getVolume(), destVolumeInfo.getUuid()), e); + } + } + + // re-retrieve volume to get any updated information from grant + destVolumeInfo = _volumeDataFactory.getVolume(destVolumeInfo.getId(), destVolumeInfo.getDataStore()); + CopyCmdAnswer copyCmdAnswer; if (errMsg != null) { copyCmdAnswer = new CopyCmdAnswer(errMsg); @@ -921,6 +932,125 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { return hostVO; } + private VolumeInfo createTemporaryVolumeCopyOfSnapshotAdaptive(SnapshotInfo snapshotInfo) { + VolumeInfo tempVolumeInfo = null; + VolumeVO tempVolumeVO = null; + try { + tempVolumeVO = new VolumeVO(Volume.Type.DATADISK, snapshotInfo.getName() + "_" + System.currentTimeMillis() + ".TMP", + snapshotInfo.getDataCenterId(), snapshotInfo.getDomainId(), snapshotInfo.getAccountId(), 0, ProvisioningType.THIN, snapshotInfo.getSize(), 0L, 0L, ""); + tempVolumeVO.setPoolId(snapshotInfo.getDataStore().getId()); + _volumeDao.persist(tempVolumeVO); + tempVolumeInfo = this._volFactory.getVolume(tempVolumeVO.getId()); + + if (snapshotInfo.getDataStore().getDriver().canCopy(snapshotInfo, tempVolumeInfo)) { + snapshotInfo.getDataStore().getDriver().copyAsync(snapshotInfo, tempVolumeInfo, null, null); + // refresh volume info as data could have changed + tempVolumeInfo = this._volFactory.getVolume(tempVolumeVO.getId()); + } else { + throw new CloudRuntimeException("Storage driver indicated it could create a volume from the snapshot but rejected the subsequent request to do so"); + } + return tempVolumeInfo; + } catch (Throwable e) { + try { + if (tempVolumeInfo != null) { + tempVolumeInfo.getDataStore().getDriver().deleteAsync(tempVolumeInfo.getDataStore(), tempVolumeInfo, null); + } + + // cleanup temporary volume + if (tempVolumeVO != null) { + _volumeDao.remove(tempVolumeVO.getId()); + } + } catch (Throwable e2) { + logger.warn("Failed to delete temporary volume created for copy", e2); + } + + throw e; + } + } + + /** + * Simplier logic for copy from snapshot for adaptive driver only. + * @param snapshotInfo + * @param destData + * @param callback + */ + private void handleCopyAsyncToSecondaryStorageAdaptive(SnapshotInfo snapshotInfo, DataObject destData, AsyncCompletionCallback callback) { + CopyCmdAnswer copyCmdAnswer = null; + DataObject srcFinal = null; + HostVO hostVO = null; + DataStore srcDataStore = null; + boolean tempRequired = false; + + try { + snapshotInfo.processEvent(Event.CopyingRequested); + hostVO = getHost(snapshotInfo); + DataObject destOnStore = destData; + srcDataStore = snapshotInfo.getDataStore(); + int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); + CopyCommand copyCommand = null; + if (!Boolean.parseBoolean(srcDataStore.getDriver().getCapabilities().get("CAN_DIRECT_ATTACH_SNAPSHOT"))) { + srcFinal = createTemporaryVolumeCopyOfSnapshotAdaptive(snapshotInfo); + tempRequired = true; + } else { + srcFinal = snapshotInfo; + } + + _volumeService.grantAccess(srcFinal, hostVO, srcDataStore); + + DataTO srcTo = srcFinal.getTO(); + + // have to set PATH as extraOptions due to logic in KVM hypervisor processor + HashMap extraDetails = new HashMap<>(); + extraDetails.put(DiskTO.PATH, srcTo.getPath()); + + copyCommand = new CopyCommand(srcFinal.getTO(), destOnStore.getTO(), primaryStorageDownloadWait, + VirtualMachineManager.ExecuteInSequence.value()); + copyCommand.setOptions(extraDetails); + copyCmdAnswer = (CopyCmdAnswer)agentManager.send(hostVO.getId(), copyCommand); + } catch (Exception ex) { + String msg = "Failed to create template from snapshot (Snapshot ID = " + snapshotInfo.getId() + ") : "; + logger.warn(msg, ex); + throw new CloudRuntimeException(msg + ex.getMessage(), ex); + } + finally { + // remove access tot he volume that was used + if (srcFinal != null && hostVO != null && srcDataStore != null) { + _volumeService.revokeAccess(srcFinal, hostVO, srcDataStore); + } + + // delete the temporary volume if it was needed + if (srcFinal != null && tempRequired) { + try { + srcFinal.getDataStore().getDriver().deleteAsync(srcFinal.getDataStore(), srcFinal, null); + } catch (Throwable e) { + logger.warn("Failed to delete temporary volume created for copy", e); + } + } + + // check we have a reasonable result + String errMsg = null; + if (copyCmdAnswer == null || (!copyCmdAnswer.getResult() && copyCmdAnswer.getDetails() == null)) { + errMsg = "Unable to create template from snapshot"; + copyCmdAnswer = new CopyCmdAnswer(errMsg); + } else if (!copyCmdAnswer.getResult() && StringUtils.isEmpty(copyCmdAnswer.getDetails())) { + errMsg = "Unable to create template from snapshot"; + } else if (!copyCmdAnswer.getResult()) { + errMsg = copyCmdAnswer.getDetails(); + } + + //submit processEvent + if (StringUtils.isEmpty(errMsg)) { + snapshotInfo.processEvent(Event.OperationSuccessed); + } else { + snapshotInfo.processEvent(Event.OperationFailed); + } + + CopyCommandResult result = new CopyCommandResult(null, copyCmdAnswer); + result.setResult(copyCmdAnswer.getDetails()); + callback.complete(result); + } + } + /** * This function is responsible for copying a snapshot from managed storage to secondary storage. This is used in the following two cases: * 1) When creating a template from a snapshot @@ -931,6 +1061,13 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { * @param callback callback for async */ private void handleCopyAsyncToSecondaryStorage(SnapshotInfo snapshotInfo, DataObject destData, AsyncCompletionCallback callback) { + + // if this flag is set (true or false), we will fall out to use simplier logic for the Adaptive handler + if (snapshotInfo.getDataStore().getDriver().getCapabilities().get("CAN_DIRECT_ATTACH_SNAPSHOT") != null) { + handleCopyAsyncToSecondaryStorageAdaptive(snapshotInfo, destData, callback); + return; + } + String errMsg = null; CopyCmdAnswer copyCmdAnswer = null; boolean usingBackendSnapshot = false; @@ -1697,14 +1834,13 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { private CopyCmdAnswer copyImageToVolume(DataObject srcDataObject, VolumeInfo destVolumeInfo, HostVO hostVO) { int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); - CopyCommand copyCommand = new CopyCommand(srcDataObject.getTO(), destVolumeInfo.getTO(), primaryStorageDownloadWait, - VirtualMachineManager.ExecuteInSequence.value()); - CopyCmdAnswer copyCmdAnswer; try { _volumeService.grantAccess(destVolumeInfo, hostVO, destVolumeInfo.getDataStore()); + CopyCommand copyCommand = new CopyCommand(srcDataObject.getTO(), destVolumeInfo.getTO(), primaryStorageDownloadWait, + VirtualMachineManager.ExecuteInSequence.value()); Map destDetails = getVolumeDetails(destVolumeInfo); copyCommand.setOptions2(destDetails); @@ -1729,42 +1865,6 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { return copyCmdAnswer; } - /** - * Use normal volume semantics (create a volume known to cloudstack, ask the storage driver to create it as a copy of the snapshot) - - * @param volumeVO - * @param snapshotInfo - */ - public void prepTempVolumeForCopyFromSnapshot(SnapshotInfo snapshotInfo) { - VolumeVO volumeVO = null; - try { - volumeVO = new VolumeVO(Volume.Type.DATADISK, snapshotInfo.getName() + "_" + System.currentTimeMillis() + ".TMP", - snapshotInfo.getDataCenterId(), snapshotInfo.getDomainId(), snapshotInfo.getAccountId(), 0, ProvisioningType.THIN, snapshotInfo.getSize(), 0L, 0L, ""); - volumeVO.setPoolId(snapshotInfo.getDataStore().getId()); - _volumeDao.persist(volumeVO); - VolumeInfo tempVolumeInfo = this._volFactory.getVolume(volumeVO.getId()); - - if (snapshotInfo.getDataStore().getDriver().canCopy(snapshotInfo, tempVolumeInfo)) { - snapshotInfo.getDataStore().getDriver().copyAsync(snapshotInfo, tempVolumeInfo, null, null); - // refresh volume info as data could have changed - tempVolumeInfo = this._volFactory.getVolume(volumeVO.getId()); - // save the "temp" volume info into the snapshot details (we need this to clean up at the end) - _snapshotDetailsDao.addDetail(snapshotInfo.getId(), "TemporaryVolumeCopyUUID", tempVolumeInfo.getUuid(), true); - _snapshotDetailsDao.addDetail(snapshotInfo.getId(), "TemporaryVolumeCopyPath", tempVolumeInfo.getPath(), true); - // NOTE: for this to work, the Driver must return a custom SnapshotObjectTO object from getTO() - // whenever the TemporaryVolumeCopyPath is set. - } else { - throw new CloudRuntimeException("Storage driver indicated it could create a volume from the snapshot but rejected the subsequent request to do so"); - } - } catch (Throwable e) { - // cleanup temporary volume - if (volumeVO != null) { - _volumeDao.remove(volumeVO.getId()); - } - throw e; - } - } - /** * If the underlying storage system is making use of read-only snapshots, this gives the storage system the opportunity to * create a volume from the snapshot so that we can copy the VHD file that should be inside of the snapshot to secondary storage. @@ -1776,13 +1876,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { * resign the SR and the VDI that should be inside of the snapshot before copying the VHD file to secondary storage. */ private void createVolumeFromSnapshot(SnapshotInfo snapshotInfo) { - if ("true".equalsIgnoreCase(snapshotInfo.getDataStore().getDriver().getCapabilities().get("CAN_CREATE_TEMP_VOLUME_FROM_SNAPSHOT"))) { - prepTempVolumeForCopyFromSnapshot(snapshotInfo); - return; - - } - SnapshotDetailsVO snapshotDetails = handleSnapshotDetails(snapshotInfo.getId(), "create"); + try { snapshotInfo.getDataStore().getDriver().createAsync(snapshotInfo.getDataStore(), snapshotInfo, null); } @@ -1797,31 +1892,20 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { * invocation of createVolumeFromSnapshot(SnapshotInfo). */ private void deleteVolumeFromSnapshot(SnapshotInfo snapshotInfo) { - VolumeVO volumeVO = null; - // cleanup any temporary volume previously created for copy from a snapshot - if ("true".equalsIgnoreCase(snapshotInfo.getDataStore().getDriver().getCapabilities().get("CAN_CREATE_TEMP_VOLUME_FROM_SNAPSHOT"))) { - SnapshotDetailsVO tempUuid = null; - tempUuid = _snapshotDetailsDao.findDetail(snapshotInfo.getId(), "TemporaryVolumeCopyUUID"); - if (tempUuid == null || tempUuid.getValue() == null) { - return; - } - - volumeVO = _volumeDao.findByUuid(tempUuid.getValue()); - if (volumeVO != null) { - _volumeDao.remove(volumeVO.getId()); - } - _snapshotDetailsDao.remove(tempUuid.getId()); - _snapshotDetailsDao.removeDetail(snapshotInfo.getId(), "TemporaryVolumeCopyUUID"); - return; - } - - SnapshotDetailsVO snapshotDetails = handleSnapshotDetails(snapshotInfo.getId(), "delete"); - try { - snapshotInfo.getDataStore().getDriver().createAsync(snapshotInfo.getDataStore(), snapshotInfo, null); - } - finally { - _snapshotDetailsDao.remove(snapshotDetails.getId()); + logger.debug("Cleaning up temporary volume created for copy from a snapshot"); + + SnapshotDetailsVO snapshotDetails = handleSnapshotDetails(snapshotInfo.getId(), "delete"); + + try { + snapshotInfo.getDataStore().getDriver().createAsync(snapshotInfo.getDataStore(), snapshotInfo, null); + } + finally { + _snapshotDetailsDao.remove(snapshotDetails.getId()); + } + + } catch (Throwable e) { + logger.warn("Failed to clean up temporary volume created for copy from a snapshot, transction will not be failed but an adminstrator should clean this up: " + snapshotInfo.getUuid() + " - " + snapshotInfo.getPath(), e); } } @@ -1906,7 +1990,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { throw new CloudRuntimeException("Invalid hypervisor type (only KVM supported for this operation at the time being)"); } - verifyLiveMigrationForKVM(volumeDataStoreMap, destHost); + verifyLiveMigrationForKVM(volumeDataStoreMap); VMInstanceVO vmInstance = _vmDao.findById(vmTO.getId()); vmTO.setState(vmInstance.getState()); @@ -1933,7 +2017,10 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { continue; } - if (srcVolumeInfo.getTemplateId() != null) { + VMTemplateVO vmTemplate = _vmTemplateDao.findById(vmInstance.getTemplateId()); + if (srcVolumeInfo.getTemplateId() != null && + Objects.nonNull(vmTemplate) && + !Arrays.asList(KVM_VM_IMPORT_DEFAULT_TEMPLATE_NAME, VM_IMPORT_DEFAULT_TEMPLATE_NAME).contains(vmTemplate.getName())) { logger.debug(String.format("Copying template [%s] of volume [%s] from source storage pool [%s] to target storage pool [%s].", srcVolumeInfo.getTemplateId(), srcVolumeInfo.getId(), sourceStoragePool.getId(), destStoragePool.getId())); copyTemplateToTargetFilesystemStorageIfNeeded(srcVolumeInfo, sourceStoragePool, destDataStore, destStoragePool, destHost); } else { @@ -1977,8 +2064,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { MigrateCommand.MigrateDiskInfo migrateDiskInfo; - boolean isNonManagedNfsToNfsOrSharedMountPointToNfs = supportStoragePoolType(sourceStoragePool.getPoolType()) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination; - if (isNonManagedNfsToNfsOrSharedMountPointToNfs) { + boolean isNonManagedToNfs = supportStoragePoolType(sourceStoragePool.getPoolType(), StoragePoolType.Filesystem) && destStoragePool.getPoolType() == StoragePoolType.NetworkFilesystem && !managedStorageDestination; + if (isNonManagedToNfs) { migrateDiskInfo = new MigrateCommand.MigrateDiskInfo(srcVolumeInfo.getPath(), MigrateCommand.MigrateDiskInfo.DiskType.FILE, MigrateCommand.MigrateDiskInfo.DriverType.QCOW2, @@ -2152,7 +2239,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { if (srcVolumeInfo.getHypervisorType() == HypervisorType.KVM && srcVolumeInfo.getTemplateId() != null && srcVolumeInfo.getPoolId() != null) { VMTemplateVO template = _vmTemplateDao.findById(srcVolumeInfo.getTemplateId()); - if (template.getFormat() != null && template.getFormat() != Storage.ImageFormat.ISO) { + if (Objects.nonNull(template) && template.getFormat() != null && template.getFormat() != Storage.ImageFormat.ISO) { VMTemplateStoragePoolVO ref = templatePoolDao.findByPoolTemplate(srcVolumeInfo.getPoolId(), srcVolumeInfo.getTemplateId(), null); return ref != null ? ref.getInstallPath() : null; } @@ -2357,9 +2444,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { * At a high level: The source storage cannot be managed and * the destination storages can be all managed or all not managed, not mixed. */ - protected void verifyLiveMigrationForKVM(Map volumeDataStoreMap, Host destHost) { + protected void verifyLiveMigrationForKVM(Map volumeDataStoreMap) { Boolean storageTypeConsistency = null; - Map sourcePools = new HashMap<>(); for (Map.Entry entry : volumeDataStoreMap.entrySet()) { VolumeInfo volumeInfo = entry.getKey(); @@ -2386,47 +2472,6 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { } else if (storageTypeConsistency != destStoragePoolVO.isManaged()) { throw new CloudRuntimeException("Destination storage pools must be either all managed or all not managed"); } - - addSourcePoolToPoolsMap(sourcePools, srcStoragePoolVO, destStoragePoolVO); - } - verifyDestinationStorage(sourcePools, destHost); - } - - /** - * Adds source storage pool to the migration map if the destination pool is not managed and it is NFS. - */ - protected void addSourcePoolToPoolsMap(Map sourcePools, StoragePoolVO srcStoragePoolVO, StoragePoolVO destStoragePoolVO) { - if (destStoragePoolVO.isManaged() || !StoragePoolType.NetworkFilesystem.equals(destStoragePoolVO.getPoolType())) { - logger.trace(String.format("Skipping adding source pool [%s] to map due to destination pool [%s] is managed or not NFS.", srcStoragePoolVO, destStoragePoolVO)); - return; - } - - String sourceStoragePoolUuid = srcStoragePoolVO.getUuid(); - if (!sourcePools.containsKey(sourceStoragePoolUuid)) { - sourcePools.put(sourceStoragePoolUuid, srcStoragePoolVO.getPoolType()); - } - } - - /** - * Perform storage validation on destination host for KVM live storage migrations. - * Validate that volume source storage pools are mounted on the destination host prior the migration - * @throws CloudRuntimeException if any source storage pool is not mounted on the destination host - */ - private void verifyDestinationStorage(Map sourcePools, Host destHost) { - if (MapUtils.isNotEmpty(sourcePools)) { - logger.debug("Verifying source pools are already available on destination host " + destHost.getUuid()); - CheckStorageAvailabilityCommand cmd = new CheckStorageAvailabilityCommand(sourcePools); - try { - Answer answer = agentManager.send(destHost.getId(), cmd); - if (answer == null || !answer.getResult()) { - throw new CloudRuntimeException("Storage verification failed on host " - + destHost.getUuid() +": " + answer.getDetails()); - } - } catch (AgentUnavailableException | OperationTimedoutException e) { - e.printStackTrace(); - throw new CloudRuntimeException("Cannot perform storage verification on host " + destHost.getUuid() + - "due to: " + e.getMessage()); - } } } @@ -2497,15 +2542,15 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { int primaryStorageDownloadWait = StorageManager.PRIMARY_STORAGE_DOWNLOAD_WAIT.value(); - CopyCommand copyCommand = new CopyCommand(volumeInfo.getTO(), templateInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); - try { handleQualityOfServiceForVolumeMigration(volumeInfo, PrimaryDataStoreDriver.QualityOfServiceState.MIGRATION); - if (srcVolumeDetached || StoragePoolType.PowerFlex == storagePoolVO.getPoolType()) { + if (srcVolumeDetached || StoragePoolType.PowerFlex == storagePoolVO.getPoolType() || StoragePoolType.FiberChannel == storagePoolVO.getPoolType()) { _volumeService.grantAccess(volumeInfo, hostVO, srcDataStore); } + CopyCommand copyCommand = new CopyCommand(volumeInfo.getTO(), templateInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); + Map srcDetails = getVolumeDetails(volumeInfo); copyCommand.setOptions(srcDetails); @@ -2534,7 +2579,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { throw new CloudRuntimeException(msg + ex.getMessage(), ex); } finally { - if (srcVolumeDetached || StoragePoolType.PowerFlex == storagePoolVO.getPoolType()) { + if (srcVolumeDetached || StoragePoolType.PowerFlex == storagePoolVO.getPoolType() || StoragePoolType.FiberChannel == storagePoolVO.getPoolType()) { try { _volumeService.revokeAccess(volumeInfo, hostVO, srcDataStore); } @@ -2629,13 +2674,7 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { long snapshotId = snapshotInfo.getId(); - // if the snapshot required a temporary volume be created check if the UUID is set so we can - // retrieve the temporary volume's path to use during remote copy - List storedDetails = _snapshotDetailsDao.findDetails(snapshotInfo.getId(), "TemporaryVolumeCopyPath"); - if (storedDetails != null && storedDetails.size() > 0) { - String value = storedDetails.get(0).getValue(); - snapshotDetails.put(DiskTO.PATH, value); - } else if (storagePoolVO.getPoolType() == StoragePoolType.PowerFlex || storagePoolVO.getPoolType() == StoragePoolType.FiberChannel) { + if (storagePoolVO.getPoolType() == StoragePoolType.PowerFlex || storagePoolVO.getPoolType() == StoragePoolType.FiberChannel) { snapshotDetails.put(DiskTO.IQN, snapshotInfo.getPath()); } else { snapshotDetails.put(DiskTO.IQN, getSnapshotProperty(snapshotId, DiskTO.IQN)); @@ -2851,6 +2890,8 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { Map srcDetails = getVolumeDetails(srcVolumeInfo); Map destDetails = getVolumeDetails(destVolumeInfo); + _volumeService.grantAccess(srcVolumeInfo, hostVO, srcVolumeInfo.getDataStore()); + MigrateVolumeCommand migrateVolumeCommand = new MigrateVolumeCommand(srcVolumeInfo.getTO(), destVolumeInfo.getTO(), srcDetails, destDetails, StorageManager.KvmStorageOfflineMigrationWait.value()); @@ -2893,18 +2934,18 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { StoragePoolVO storagePoolVO = _storagePoolDao.findById(srcVolumeInfo.getPoolId()); Map srcDetails = getVolumeDetails(srcVolumeInfo); - CopyVolumeCommand copyVolumeCommand = new CopyVolumeCommand(srcVolumeInfo.getId(), destVolumeInfo.getPath(), storagePoolVO, - destVolumeInfo.getDataStore().getUri(), true, StorageManager.KvmStorageOfflineMigrationWait.value(), true); - - copyVolumeCommand.setSrcData(srcVolumeInfo.getTO()); - copyVolumeCommand.setSrcDetails(srcDetails); - handleQualityOfServiceForVolumeMigration(srcVolumeInfo, PrimaryDataStoreDriver.QualityOfServiceState.MIGRATION); if (srcVolumeDetached) { _volumeService.grantAccess(srcVolumeInfo, hostVO, srcVolumeInfo.getDataStore()); } + CopyVolumeCommand copyVolumeCommand = new CopyVolumeCommand(srcVolumeInfo.getId(), destVolumeInfo.getPath(), storagePoolVO, + destVolumeInfo.getDataStore().getUri(), true, StorageManager.KvmStorageOfflineMigrationWait.value(), true); + + copyVolumeCommand.setSrcData(srcVolumeInfo.getTO()); + copyVolumeCommand.setSrcDetails(srcDetails); + CopyVolumeAnswer copyVolumeAnswer = (CopyVolumeAnswer)agentManager.send(hostVO.getId(), copyVolumeCommand); if (copyVolumeAnswer == null || !copyVolumeAnswer.getResult()) { @@ -2976,19 +3017,20 @@ public class StorageSystemDataMotionStrategy implements DataMotionStrategy { srcData = cacheData; } - CopyCommand copyCommand = new CopyCommand(srcData.getTO(), volumeInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); - try { + CopyCommand copyCommand = null; if (Snapshot.LocationType.PRIMARY.equals(locationType)) { _volumeService.grantAccess(snapshotInfo, hostVO, snapshotInfo.getDataStore()); Map srcDetails = getSnapshotDetails(snapshotInfo); + copyCommand = new CopyCommand(srcData.getTO(), volumeInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); copyCommand.setOptions(srcDetails); + } else { + _volumeService.grantAccess(volumeInfo, hostVO, volumeInfo.getDataStore()); + copyCommand = new CopyCommand(srcData.getTO(), volumeInfo.getTO(), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); } - _volumeService.grantAccess(volumeInfo, hostVO, volumeInfo.getDataStore()); - Map destDetails = getVolumeDetails(volumeInfo); copyCommand.setOptions2(destDetails); diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java index 87a2288cfdc..b7468195f5d 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/KvmNonManagedStorageSystemDataMotionTest.java @@ -476,19 +476,19 @@ public class KvmNonManagedStorageSystemDataMotionTest { @Test public void testVerifyLiveMigrationMapForKVM() { - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test(expected = CloudRuntimeException.class) public void testVerifyLiveMigrationMapForKVMNotExistingSource() { when(primaryDataStoreDao.findById(POOL_1_ID)).thenReturn(null); - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test(expected = CloudRuntimeException.class) public void testVerifyLiveMigrationMapForKVMNotExistingDest() { when(primaryDataStoreDao.findById(POOL_2_ID)).thenReturn(null); - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test(expected = CloudRuntimeException.class) @@ -497,7 +497,7 @@ public class KvmNonManagedStorageSystemDataMotionTest { when(pool1.getId()).thenReturn(POOL_1_ID); when(pool2.getId()).thenReturn(POOL_2_ID); lenient().when(pool2.isManaged()).thenReturn(false); - kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap, host2); + kvmNonManagedStorageDataMotionStrategy.verifyLiveMigrationForKVM(migrationMap); } @Test diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java index cea9de3f1b4..45357fa64b2 100644 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/StorageSystemDataMotionStrategyTest.java @@ -23,7 +23,6 @@ import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; import static org.mockito.MockitoAnnotations.initMocks; import java.util.HashMap; @@ -48,7 +47,6 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; -import org.mockito.verification.VerificationMode; import com.cloud.agent.api.MigrateCommand; import com.cloud.host.HostVO; @@ -62,7 +60,6 @@ import com.cloud.storage.VolumeVO; import java.util.AbstractMap; import java.util.Arrays; import java.util.HashSet; -import java.util.LinkedList; import java.util.List; import java.util.Set; @@ -372,72 +369,4 @@ public class StorageSystemDataMotionStrategyTest { assertFalse(strategy.isStoragePoolTypeInList(StoragePoolType.SharedMountPoint, listTypes)); } - - @Test - public void validateAddSourcePoolToPoolsMapDestinationPoolIsManaged() { - Mockito.doReturn(true).when(destinationStoragePoolVoMock).isManaged(); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - - Mockito.verify(destinationStoragePoolVoMock).isManaged(); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } - - @Test - public void validateAddSourcePoolToPoolsMapDestinationPoolIsNotNFS() { - List storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values())); - storagePoolTypes.remove(StoragePoolType.NetworkFilesystem); - - Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); - storagePoolTypes.forEach(poolType -> { - Mockito.doReturn(poolType).when(destinationStoragePoolVoMock).getPoolType(); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - }); - - VerificationMode times = Mockito.times(storagePoolTypes.size()); - Mockito.verify(destinationStoragePoolVoMock, times).isManaged(); - Mockito.verify(destinationStoragePoolVoMock, times).getPoolType(); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } - - @Test - public void validateAddSourcePoolToPoolsMapMapContainsKey() { - Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); - Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType(); - Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid(); - Mockito.doReturn(true).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - - Mockito.verify(destinationStoragePoolVoMock, never()).getScope(); - Mockito.verify(destinationStoragePoolVoMock).isManaged(); - Mockito.verify(destinationStoragePoolVoMock).getPoolType(); - Mockito.verify(sourceStoragePoolVoMock).getUuid(); - Mockito.verify(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } - - @Test - public void validateAddSourcePoolToPoolsMapMapDoesNotContainsKey() { - List storagePoolTypes = new LinkedList<>(Arrays.asList(StoragePoolType.values())); - - Mockito.doReturn(false).when(destinationStoragePoolVoMock).isManaged(); - Mockito.doReturn(StoragePoolType.NetworkFilesystem).when(destinationStoragePoolVoMock).getPoolType(); - Mockito.doReturn("").when(sourceStoragePoolVoMock).getUuid(); - Mockito.doReturn(false).when(mapStringStoragePoolTypeMock).containsKey(Mockito.anyString()); - Mockito.doReturn(null).when(mapStringStoragePoolTypeMock).put(Mockito.anyString(), Mockito.any()); - - storagePoolTypes.forEach(poolType -> { - Mockito.doReturn(poolType).when(sourceStoragePoolVoMock).getPoolType(); - strategy.addSourcePoolToPoolsMap(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - }); - - VerificationMode times = Mockito.times(storagePoolTypes.size()); - Mockito.verify(destinationStoragePoolVoMock, never()).getScope(); - Mockito.verify(destinationStoragePoolVoMock, times).isManaged(); - Mockito.verify(destinationStoragePoolVoMock, times).getPoolType(); - Mockito.verify(sourceStoragePoolVoMock, times).getUuid(); - Mockito.verify(mapStringStoragePoolTypeMock, times).containsKey(Mockito.anyString()); - Mockito.verify(sourceStoragePoolVoMock, times).getPoolType(); - Mockito.verify(mapStringStoragePoolTypeMock, times).put(Mockito.anyString(), Mockito.any()); - Mockito.verifyNoMoreInteractions(mapStringStoragePoolTypeMock, sourceStoragePoolVoMock, destinationStoragePoolVoMock); - } } diff --git a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java index 11a13e7ccb4..d2f08260aa3 100644 --- a/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java +++ b/engine/storage/image/src/main/java/org/apache/cloudstack/storage/image/manager/ImageStoreProviderManagerImpl.java @@ -202,7 +202,7 @@ public class ImageStoreProviderManagerImpl implements ImageStoreProviderManager, // No store with space found logger.error(String.format("Can't find an image storage in zone with less than %d usage", - Math.round(_statsCollector.getImageStoreCapacityThreshold()*100))); + Math.round(_statsCollector.getImageStoreCapacityThreshold() * 100))); return null; } diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java index de8838b0999..84750c2068c 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDao.java @@ -247,6 +247,14 @@ public interface GenericDao { int expungeList(List ids); + /** + * Delete the entity beans specified by the search criteria with a given limit + * @param sc Search criteria + * @param limit Maximum number of rows that will be affected + * @return Number of rows deleted + */ + int expunge(SearchCriteria sc, long limit); + /** * expunge the removed rows. */ diff --git a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java index 4202f6996c1..b7e4f44cf8c 100644 --- a/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java +++ b/framework/db/src/main/java/com/cloud/utils/db/GenericDaoBase.java @@ -1236,6 +1236,12 @@ public abstract class GenericDaoBase extends Compone } // FIXME: Does not work for joins. + @Override + public int expunge(final SearchCriteria sc, long limit) { + Filter filter = new Filter(limit); + return expunge(sc, filter); + } + @Override public int expunge(final SearchCriteria sc, final Filter filter) { if (sc == null) { diff --git a/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java b/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java index d61e26fc3a8..2a6d0b63e5c 100644 --- a/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java +++ b/framework/spring/module/src/main/java/org/apache/cloudstack/spring/module/model/impl/DefaultModuleDefinitionSet.java @@ -102,7 +102,9 @@ public class DefaultModuleDefinitionSet implements ModuleDefinitionSet { logger.debug(String.format("Trying to obtain module [%s] context.", moduleDefinitionName)); ApplicationContext context = getApplicationContext(moduleDefinitionName); try { - if (context.containsBean("moduleStartup")) { + if (context == null) { + logger.warn(String.format("Application context not found for module definition [%s]", moduleDefinitionName)); + } else if (context.containsBean("moduleStartup")) { Runnable runnable = context.getBean("moduleStartup", Runnable.class); logger.info(String.format("Starting module [%s].", moduleDefinitionName)); runnable.run(); diff --git a/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java b/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java index 94b763d013f..db40b6e68dd 100644 --- a/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java +++ b/plugins/acl/dynamic-role-based/src/main/java/org/apache/cloudstack/acl/DynamicRoleBasedAPIAccessChecker.java @@ -120,7 +120,9 @@ public class DynamicRoleBasedAPIAccessChecker extends AdapterBase implements API } if (accountRole.getRoleType() == RoleType.Admin && accountRole.getId() == RoleType.Admin.getId()) { - logger.info(String.format("Account [%s] is Root Admin or Domain Admin, all APIs are allowed.", account)); + if (logger.isTraceEnabled()) { + logger.trace(String.format("Account [%s] is Root Admin or Domain Admin, all APIs are allowed.", account)); + } return true; } diff --git a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java index 1e766468ba8..2e7ae23d6f1 100644 --- a/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java +++ b/plugins/acl/project-role-based/src/main/java/org/apache/cloudstack/acl/ProjectRoleBasedApiAccessChecker.java @@ -72,7 +72,9 @@ public class ProjectRoleBasedApiAccessChecker extends AdapterBase implements AP Project project = CallContext.current().getProject(); if (project == null) { - logger.warn(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning APIs [%s] for user [%s] as allowed.", apiNames, user)); + if (logger.isTraceEnabled()) { + logger.trace(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning APIs [%s] for user [%s] as allowed.", apiNames, user)); + } return apiNames; } @@ -110,8 +112,10 @@ public class ProjectRoleBasedApiAccessChecker extends AdapterBase implements AP Project project = CallContext.current().getProject(); if (project == null) { - logger.warn(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning API [%s] for user [%s] as allowed.", apiCommandName, + if (logger.isTraceEnabled()) { + logger.trace(String.format("Project is null, ProjectRoleBasedApiAccessChecker only applies to projects, returning API [%s] for user [%s] as allowed.", apiCommandName, user)); + } return true; } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java index b5ec716e805..5cffa77c297 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java @@ -3797,29 +3797,29 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } public List getAllVmNames(final Connect conn) { - final ArrayList la = new ArrayList(); + final ArrayList domainNames = new ArrayList(); try { final String names[] = conn.listDefinedDomains(); for (int i = 0; i < names.length; i++) { - la.add(names[i]); + domainNames.add(names[i]); } } catch (final LibvirtException e) { - LOGGER.warn("Failed to list Defined domains", e); + logger.warn("Failed to list defined domains", e); } int[] ids = null; try { ids = conn.listDomains(); } catch (final LibvirtException e) { - LOGGER.warn("Failed to list domains", e); - return la; + logger.warn("Failed to list domains", e); + return domainNames; } Domain dm = null; for (int i = 0; i < ids.length; i++) { try { dm = conn.domainLookupByID(ids[i]); - la.add(dm.getName()); + domainNames.add(dm.getName()); } catch (final LibvirtException e) { LOGGER.warn("Unable to get vms", e); } finally { @@ -3833,7 +3833,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv } } - return la; + return domainNames; } private HashMap getHostVmStateReport() { @@ -5379,20 +5379,31 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv /* Scp volume from remote host to local directory */ - public String copyVolume(String srcIp, String username, String password, String localDir, String remoteFile, String tmpPath) { + public String copyVolume(String srcIp, String username, String password, String localDir, String remoteFile, String tmpPath, int timeoutInSecs) { + String outputFile = UUID.randomUUID().toString(); try { - String outputFile = UUID.randomUUID().toString(); StringBuilder command = new StringBuilder("qemu-img convert -O qcow2 "); command.append(remoteFile); - command.append(" "+tmpPath); + command.append(" " + tmpPath); command.append(outputFile); - logger.debug("Converting remoteFile: "+remoteFile); - SshHelper.sshExecute(srcIp, 22, username, null, password, command.toString()); - logger.debug("Copying remoteFile to: "+localDir); - SshHelper.scpFrom(srcIp, 22, username, null, password, localDir, tmpPath+outputFile); - logger.debug("Successfully copyied remoteFile to: "+localDir+"/"+outputFile); + logger.debug(String.format("Converting remote disk file: %s, output file: %s%s (timeout: %d secs)", remoteFile, tmpPath, outputFile, timeoutInSecs)); + SshHelper.sshExecute(srcIp, 22, username, null, password, command.toString(), timeoutInSecs * 1000); + logger.debug("Copying converted remote disk file " + outputFile + " to: " + localDir); + SshHelper.scpFrom(srcIp, 22, username, null, password, localDir, tmpPath + outputFile); + logger.debug("Successfully copied converted remote disk file to: " + localDir + "/" + outputFile); return outputFile; } catch (Exception e) { + try { + String deleteRemoteConvertedFileCmd = String.format("rm -f %s%s", tmpPath, outputFile); + SshHelper.sshExecute(srcIp, 22, username, null, password, deleteRemoteConvertedFileCmd); + } catch (Exception ignored) { + } + + try { + FileUtils.deleteQuietly(new File(localDir + "/" + outputFile)); + } catch (Exception ignored) { + } + throw new RuntimeException(e); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java index 025a5ed192c..e6ec05fec23 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCopyRemoteVolumeCommandWrapper.java @@ -43,7 +43,6 @@ public final class LibvirtCopyRemoteVolumeCommandWrapper extends CommandWrapper< @Override public Answer execute(final CopyRemoteVolumeCommand command, final LibvirtComputingResource libvirtComputingResource) { - String result = null; String srcIp = command.getRemoteIp(); String username = command.getUsername(); String password = command.getPassword(); @@ -53,23 +52,25 @@ public final class LibvirtCopyRemoteVolumeCommandWrapper extends CommandWrapper< KVMStoragePoolManager poolMgr = libvirtComputingResource.getStoragePoolMgr(); KVMStoragePool pool = poolMgr.getStoragePool(storageFilerTO.getType(), storageFilerTO.getUuid()); String dstPath = pool.getLocalPath(); + int timeoutInSecs = command.getWait(); try { if (storageFilerTO.getType() == Storage.StoragePoolType.Filesystem || storageFilerTO.getType() == Storage.StoragePoolType.NetworkFilesystem) { - String filename = libvirtComputingResource.copyVolume(srcIp, username, password, dstPath, srcFile, tmpPath); - logger.debug("Volume Copy Successful"); + String filename = libvirtComputingResource.copyVolume(srcIp, username, password, dstPath, srcFile, tmpPath, timeoutInSecs); + logger.debug("Volume " + srcFile + " copy successful, copied to file: " + filename); final KVMPhysicalDisk vol = pool.getPhysicalDisk(filename); final String path = vol.getPath(); long size = getVirtualSizeFromFile(path); - return new CopyRemoteVolumeAnswer(command, "", filename, size); + return new CopyRemoteVolumeAnswer(command, "", filename, size); } else { - return new Answer(command, false, "Unsupported Storage Pool"); + String msg = "Unsupported storage pool type: " + storageFilerTO.getType().toString() + ", only local and NFS pools are supported"; + return new Answer(command, false, msg); } - } catch (final Exception e) { - logger.error("Error while copying file from remote host: "+ e.getMessage()); - return new Answer(command, false, result); + logger.error("Error while copying volume file from remote host: " + e.getMessage(), e); + String msg = "Failed to copy volume due to: " + e.getMessage(); + return new Answer(command, false, msg); } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java index a9da4a50452..114b27d3a5b 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtGetRemoteVmsCommandWrapper.java @@ -47,37 +47,38 @@ public final class LibvirtGetRemoteVmsCommandWrapper extends CommandWrapper unmanagedInstances = new HashMap<>(); try { Connect conn = LibvirtConnection.getConnection(hypervisorURI); final List allVmNames = libvirtComputingResource.getAllVmNames(conn); + logger.info(String.format("Found %d VMs on the remote host %s", allVmNames.size(), remoteIp)); for (String name : allVmNames) { final Domain domain = libvirtComputingResource.getDomain(conn, name); - final DomainInfo.DomainState ps = domain.getInfo().state; final VirtualMachine.PowerState state = libvirtComputingResource.convertToPowerState(ps); - logger.debug("VM " + domain.getName() + " - powerstate: " + ps + ", state: " + state.toString()); + logger.debug(String.format("Remote VM %s - powerstate: %s, state: %s", domain.getName(), ps.toString(), state.toString())); if (state == VirtualMachine.PowerState.PowerOff) { try { UnmanagedInstanceTO instance = getUnmanagedInstance(libvirtComputingResource, domain, conn); unmanagedInstances.put(instance.getName(), instance); } catch (Exception e) { - logger.error("Couldn't fetch VM " + domain.getName() + " details, due to: " + e.getMessage(), e); + logger.error("Couldn't fetch remote VM " + domain.getName() + " details, due to: " + e.getMessage(), e); } } domain.free(); } - logger.debug("Found " + unmanagedInstances.size() + " stopped VMs on host " + command.getRemoteIp()); + logger.debug("Found " + unmanagedInstances.size() + " stopped VMs on remote host " + remoteIp); return new GetRemoteVmsAnswer(command, "", unmanagedInstances); } catch (final LibvirtException e) { - logger.error("Failed to list stopped VMs on remote host " + command.getRemoteIp() + ", due to: " + e.getMessage(), e); + logger.error("Failed to list stopped VMs on remote host " + remoteIp + ", due to: " + e.getMessage(), e); if (e.getMessage().toLowerCase().contains("connection refused")) { - return new Answer(command, false, "Unable to connect to remote host " + command.getRemoteIp() + ", please check the libvirtd tcp connectivity and retry"); + return new Answer(command, false, "Unable to connect to remote host " + remoteIp + ", please check the libvirtd tcp connectivity and retry"); } - return new Answer(command, false, "Unable to list stopped VMs on remote host " + command.getRemoteIp() + ", due to: " + e.getMessage()); + return new Answer(command, false, "Unable to list stopped VMs on remote host " + remoteIp + ", due to: " + e.getMessage()); } } @@ -103,8 +104,8 @@ public final class LibvirtGetRemoteVmsCommandWrapper extends CommandWrapper 0) { + hostname = hostname.substring(0, hostname.indexOf(".")); // strip off domain + } + hostnameFq = inetAddress.getCanonicalHostName(); // fully qualified hostname + LOGGER.info("Loaded FiberChannelAdapter for StorageLayer on host [" + hostname + "]"); + } catch (UnknownHostException e) { + LOGGER.error("Error getting hostname", e); + } } @Override @@ -76,6 +99,11 @@ public class FiberChannelAdapter extends MultipathSCSIAdapterBase { address = value; } else if (key.equals("connid")) { connectionId = value; + } else if (key.startsWith("connid.")) { + String inHostname = key.substring(7); + if (inHostname != null && (inHostname.equals(this.hostname) || inHostname.equals(this.hostnameFq))) { + connectionId = value; + } } } } diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index b9671c872d1..f6242444d91 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -134,6 +134,10 @@ import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.utils.script.Script; import com.cloud.utils.storage.S3.S3Utils; import com.cloud.vm.VmDetailConstants; +import org.apache.cloudstack.utils.cryptsetup.KeyFile; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; +import org.apache.cloudstack.utils.qemu.QemuObject.EncryptFormat; +import java.util.ArrayList; public class KVMStorageProcessor implements StorageProcessor { protected Logger logger = LogManager.getLogger(getClass()); @@ -267,7 +271,7 @@ public class KVMStorageProcessor implements StorageProcessor { Map details = primaryStore.getDetails(); - String path = details != null ? details.get("managedStoreTarget") : null; + String path = derivePath(primaryStore, destData, details); if (!storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), path, details)) { logger.warn("Failed to connect physical disk at path: " + path + ", in storage pool id: " + primaryStore.getUuid()); @@ -327,6 +331,16 @@ public class KVMStorageProcessor implements StorageProcessor { } } + private String derivePath(PrimaryDataStoreTO primaryStore, DataTO destData, Map details) { + String path = null; + if (primaryStore.getPoolType() == StoragePoolType.FiberChannel) { + path = destData.getPath(); + } else { + path = details != null ? details.get("managedStoreTarget") : null; + } + return path; + } + // this is much like PrimaryStorageDownloadCommand, but keeping it separate. copies template direct to root disk private KVMPhysicalDisk templateToPrimaryDownload(final String templateUrl, final KVMStoragePool primaryPool, final String volUuid, final Long size, final int timeout) { final int index = templateUrl.lastIndexOf("/"); @@ -406,7 +420,7 @@ public class KVMStorageProcessor implements StorageProcessor { vol = templateToPrimaryDownload(templatePath, primaryPool, volume.getUuid(), volume.getSize(), cmd.getWaitInMillSeconds()); } if (storagePoolMgr.supportsPhysicalDiskCopy(primaryPool.getType())) { Map details = primaryStore.getDetails(); - String path = details != null ? details.get("managedStoreTarget") : null; + String path = derivePath(primaryStore, destData, details); if (!storagePoolMgr.connectPhysicalDisk(primaryStore.getPoolType(), primaryStore.getUuid(), templatePath, details)) { logger.warn("Failed to connect base template volume at path: " + templatePath + ", in storage pool id: " + primaryStore.getUuid()); @@ -1047,7 +1061,7 @@ public class KVMStorageProcessor implements StorageProcessor { srcVolume.clearPassphrase(); if (isCreatedFromVmSnapshot) { logger.debug("Ignoring removal of vm snapshot on primary as this snapshot is created from vm snapshot"); - } else if (primaryPool.getType() != StoragePoolType.RBD) { + } else if (primaryPool != null && primaryPool.getType() != StoragePoolType.RBD) { deleteSnapshotOnPrimary(cmd, snapshot, primaryPool); } @@ -1748,7 +1762,7 @@ public class KVMStorageProcessor implements StorageProcessor { snapshotPath = getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName); String diskLabel = takeVolumeSnapshot(resource.getDisks(conn, vmName), snapshotName, diskPath, vm); - String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume, cmd.getWait()); + String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, disk, snapshotPath, volume, cmd.getWait()); mergeSnapshotIntoBaseFile(vm, diskLabel, diskPath, snapshotName, volume, conn); @@ -1817,7 +1831,7 @@ public class KVMStorageProcessor implements StorageProcessor { } } else { snapshotPath = getSnapshotPathInPrimaryStorage(primaryPool.getLocalPath(), snapshotName); - String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, diskPath, snapshotPath, volume, cmd.getWait()); + String convertResult = convertBaseFileToSnapshotFileInPrimaryStorageDir(primaryPool, disk, snapshotPath, volume, cmd.getWait()); validateConvertResult(convertResult, snapshotPath); } } @@ -1940,26 +1954,43 @@ public class KVMStorageProcessor implements StorageProcessor { * @param snapshotPath Path to convert the base file; * @return null if the conversion occurs successfully or an error message that must be handled. */ - protected String convertBaseFileToSnapshotFileInPrimaryStorageDir(KVMStoragePool primaryPool, String baseFile, String snapshotPath, VolumeObjectTO volume, int wait) { - try { - logger.debug(String.format("Trying to convert volume [%s] (%s) to snapshot [%s].", volume, baseFile, snapshotPath)); + protected String convertBaseFileToSnapshotFileInPrimaryStorageDir(KVMStoragePool primaryPool, + KVMPhysicalDisk baseFile, String snapshotPath, VolumeObjectTO volume, int wait) { + try (KeyFile srcKey = new KeyFile(volume.getPassphrase())) { + logger.debug( + String.format("Trying to convert volume [%s] (%s) to snapshot [%s].", volume, baseFile, snapshotPath)); primaryPool.createFolder(TemplateConstants.DEFAULT_SNAPSHOT_ROOT_DIR); - - QemuImgFile srcFile = new QemuImgFile(baseFile); - srcFile.setFormat(PhysicalDiskFormat.QCOW2); - - QemuImgFile destFile = new QemuImgFile(snapshotPath); - destFile.setFormat(PhysicalDiskFormat.QCOW2); - - QemuImg q = new QemuImg(wait); - q.convert(srcFile, destFile); - - logger.debug(String.format("Converted volume [%s] (from path \"%s\") to snapshot [%s].", volume, baseFile, snapshotPath)); - return null; - } catch (QemuImgException | LibvirtException ex) { - return String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volume, baseFile, snapshotPath, ex.getMessage()); + convertTheBaseFileToSnapshot(baseFile, snapshotPath, wait, srcKey); + } catch (QemuImgException | LibvirtException | IOException ex) { + return String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volume, baseFile, + snapshotPath, ex.getMessage()); } + + logger.debug(String.format("Converted volume [%s] (from path \"%s\") to snapshot [%s].", volume, baseFile, + snapshotPath)); + return null; + } + + private void convertTheBaseFileToSnapshot(KVMPhysicalDisk baseFile, String snapshotPath, int wait, KeyFile srcKey) + throws LibvirtException, QemuImgException { + List qemuObjects = new ArrayList<>(); + Map options = new HashMap<>(); + QemuImageOptions qemuImageOpts = new QemuImageOptions(baseFile.getPath()); + if (srcKey.isSet()) { + String srcKeyName = "sec0"; + qemuObjects.add(QemuObject.prepareSecretForQemuImg(baseFile.getFormat(), EncryptFormat.LUKS, + srcKey.toString(), srcKeyName, options)); + qemuImageOpts = new QemuImageOptions(baseFile.getFormat(), baseFile.getPath(), srcKeyName); + } + QemuImgFile srcFile = new QemuImgFile(baseFile.getPath()); + srcFile.setFormat(PhysicalDiskFormat.QCOW2); + + QemuImgFile destFile = new QemuImgFile(snapshotPath); + destFile.setFormat(PhysicalDiskFormat.QCOW2); + + QemuImg q = new QemuImg(wait); + q.convert(srcFile, destFile, options, qemuObjects, qemuImageOpts, null, true); } /** @@ -2467,8 +2498,7 @@ public class KVMStorageProcessor implements StorageProcessor { if (!storagePoolMgr.connectPhysicalDisk(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid(), destVolumePath, destPrimaryStore.getDetails())) { logger.warn("Failed to connect dest volume at path: " + destVolumePath + ", in storage pool id: " + destPrimaryStore.getUuid()); } - String managedStoreTarget = destPrimaryStore.getDetails() != null ? destPrimaryStore.getDetails().get("managedStoreTarget") : null; - destVolumeName = managedStoreTarget != null ? managedStoreTarget : destVolumePath; + destVolumeName = derivePath(destPrimaryStore, destData, destPrimaryStore.getDetails()); } else { final String volumeName = UUID.randomUUID().toString(); destVolumeName = volumeName + "." + destFormat.getFileExtension(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java index b80accd6018..97a4c4dc044 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/LibvirtStorageAdaptor.java @@ -273,6 +273,16 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { } } + private void checkNetfsStoragePoolMounted(String uuid) { + String targetPath = _mountPoint + File.separator + uuid; + int mountpointResult = Script.runSimpleBashScriptForExitValue("mountpoint -q " + targetPath); + if (mountpointResult != 0) { + String errMsg = String.format("libvirt failed to mount storage pool %s at %s", uuid, targetPath); + logger.error(errMsg); + throw new CloudRuntimeException(errMsg); + } + } + private StoragePool createNetfsStoragePool(PoolType fsType, Connect conn, String uuid, String host, String path) throws LibvirtException { String targetPath = _mountPoint + File.separator + uuid; LibvirtStoragePoolDef spd = new LibvirtStoragePoolDef(fsType, uuid, uuid, host, path, targetPath); @@ -699,6 +709,10 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { sp.create(0); } + if (type == StoragePoolType.NetworkFilesystem) { + checkNetfsStoragePoolMounted(name); + } + return getStoragePool(name); } catch (LibvirtException e) { String error = e.toString(); @@ -762,11 +776,11 @@ public class LibvirtStorageAdaptor implements StorageAdaptor { // handle ebusy error when pool is quickly destroyed if (e.toString().contains("exit status 16")) { String targetPath = _mountPoint + File.separator + uuid; - logger.error("deleteStoragePool removed pool from libvirt, but libvirt had trouble unmounting the pool. Trying umount location " + targetPath + - "again in a few seconds"); + logger.error("deleteStoragePool removed pool from libvirt, but libvirt had trouble unmounting the pool. Trying umount location " + targetPath + + " again in a few seconds"); String result = Script.runSimpleBashScript("sleep 5 && umount " + targetPath); if (result == null) { - logger.error("Succeeded in unmounting " + targetPath); + logger.info("Succeeded in unmounting " + targetPath); return true; } logger.error("Failed to unmount " + targetPath); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java index 1625ecc171a..03acfcc89ad 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/MultipathSCSIAdapterBase.java @@ -21,18 +21,15 @@ import java.io.BufferedReader; import java.io.File; import java.io.IOException; import java.io.InputStreamReader; -import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Timer; import java.util.TimerTask; -import java.util.UUID; import java.util.concurrent.TimeUnit; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat; -import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; import com.cloud.storage.Storage; @@ -44,7 +41,6 @@ import com.cloud.utils.script.Script; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.libvirt.LibvirtException; import org.joda.time.Duration; public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { @@ -56,6 +52,14 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { */ static byte[] CLEANUP_LOCK = new byte[0]; + /** + * List of supported OUI's (needed for path-based cleanup logic on disconnects after live migrations) + */ + static String[] SUPPORTED_OUI_LIST = { + "0002ac", // HPE Primera 3PAR + "24a937" // Pure Flasharray + }; + /** * Property keys and defaults */ @@ -83,6 +87,7 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { * Initialize static program-wide configurations and background jobs */ static { + long cleanupFrequency = CLEANUP_FREQUENCY_SECS.getFinalValue() * 1000; boolean cleanupEnabled = CLEANUP_ENABLED.getFinalValue(); @@ -97,16 +102,13 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { throw new Error("Unable to find the disconnectVolume.sh script"); } - resizeScript = Script.findScript(STORAGE_SCRIPTS_DIR.getFinalValue(), resizeScript); - if (resizeScript == null) { - throw new Error("Unable to find the resizeVolume.sh script"); - } - copyScript = Script.findScript(STORAGE_SCRIPTS_DIR.getFinalValue(), copyScript); if (copyScript == null) { throw new Error("Unable to find the copyVolume.sh script"); } + resizeScript = Script.findScript(STORAGE_SCRIPTS_DIR.getFinalValue(), resizeScript); + if (cleanupEnabled) { cleanupScript = Script.findScript(STORAGE_SCRIPTS_DIR.getFinalValue(), cleanupScript); if (cleanupScript == null) { @@ -138,9 +140,6 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { public abstract boolean isStoragePoolTypeSupported(Storage.StoragePoolType type); - /** - * We expect WWN values in the volumePath so need to convert it to an actual physical path - */ public abstract AddressInfo parseAndValidatePath(String path); @Override @@ -152,6 +151,7 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { return null; } + // we expect WWN values in the volumePath so need to convert it to an actual physical path AddressInfo address = parseAndValidatePath(volumePath); return getPhysicalDisk(address, pool); } @@ -187,15 +187,23 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { if (StringUtils.isEmpty(volumePath)) { LOGGER.error("Unable to connect physical disk due to insufficient data - volume path is undefined"); - throw new CloudRuntimeException("Unable to connect physical disk due to insufficient data - volume path is underfined"); + return false; } if (pool == null) { LOGGER.error("Unable to connect physical disk due to insufficient data - pool is not set"); - throw new CloudRuntimeException("Unable to connect physical disk due to insufficient data - pool is not set"); + return false; } + // we expect WWN values in the volumePath so need to convert it to an actual physical path AddressInfo address = this.parseAndValidatePath(volumePath); + + // validate we have a connection id - we can't proceed without that + if (address.getConnectionId() == null) { + LOGGER.error("Unable to connect volume with address [" + address.getPath() + "] of the storage pool: " + pool.getUuid() + " - connection id is not set in provided path"); + return false; + } + int waitTimeInSec = diskWaitTimeSecs; if (details != null && details.containsKey(StorageManager.STORAGE_POOL_DISK_WAIT.toString())) { String waitTime = details.get(StorageManager.STORAGE_POOL_DISK_WAIT.toString()); @@ -208,31 +216,62 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { @Override public boolean disconnectPhysicalDisk(String volumePath, KVMStoragePool pool) { - LOGGER.debug(String.format("disconnectPhysicalDiskByPath(volumePath,pool) called with args (%s, %s) START", volumePath, pool.getUuid())); + if (LOGGER.isDebugEnabled()) LOGGER.debug(String.format("disconnectPhysicalDisk(volumePath,pool) called with args (%s, %s) START", volumePath, pool.getUuid())); AddressInfo address = this.parseAndValidatePath(volumePath); + if (address.getAddress() == null) { + if (LOGGER.isDebugEnabled()) LOGGER.debug(String.format("disconnectPhysicalDisk(volumePath,pool) returning FALSE, volume path has no address field", volumePath, pool.getUuid())); + return false; + } ScriptResult result = runScript(disconnectScript, 60000L, address.getAddress().toLowerCase()); - if (LOGGER.isDebugEnabled()) LOGGER.debug("multipath flush output: " + result.getResult()); - LOGGER.debug(String.format("disconnectPhysicalDiskByPath(volumePath,pool) called with args (%s, %s) COMPLETE [rc=%s]", volumePath, pool.getUuid(), result.getResult())); return true; + + if (result.getExitCode() != 0) { + LOGGER.warn(String.format("Disconnect failed for path [%s] with return code [%s]", address.getAddress().toLowerCase(), result.getExitCode())); + } + + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("multipath flush output: " + result.getResult()); + LOGGER.debug(String.format("disconnectPhysicalDisk(volumePath,pool) called with args (%s, %s) COMPLETE [rc=%s]", volumePath, pool.getUuid(), result.getResult())); + } + + return (result.getExitCode() == 0); } @Override public boolean disconnectPhysicalDisk(Map volumeToDisconnect) { - LOGGER.debug(String.format("disconnectPhysicalDiskByPath(volumeToDisconnect) called with arg bag [not implemented]:") + " " + volumeToDisconnect); + LOGGER.debug(String.format("disconnectPhysicalDisk(volumeToDisconnect) called with arg bag [not implemented]:") + " " + volumeToDisconnect); return false; } @Override public boolean disconnectPhysicalDiskByPath(String localPath) { - LOGGER.debug(String.format("disconnectPhysicalDiskByPath(localPath) called with args (%s) STARTED", localPath)); - ScriptResult result = runScript(disconnectScript, 60000L, localPath.replace("/dev/mapper/3", "")); - if (LOGGER.isDebugEnabled()) LOGGER.debug("multipath flush output: " + result.getResult()); - LOGGER.debug(String.format("disconnectPhysicalDiskByPath(localPath) called with args (%s) COMPLETE [rc=%s]", localPath, result.getExitCode())); return true; + if (localPath == null) { + return false; + } + if (LOGGER.isDebugEnabled()) LOGGER.debug(String.format("disconnectPhysicalDiskByPath(localPath) called with args (%s) START", localPath)); + if (localPath.startsWith("/dev/mapper/")) { + String multipathName = localPath.replace("/dev/mapper/3", ""); + // this ensures we only disconnect multipath devices supported by this driver + for (String oui: SUPPORTED_OUI_LIST) { + if (multipathName.length() > 1 && multipathName.substring(2).startsWith(oui)) { + ScriptResult result = runScript(disconnectScript, 60000L, multipathName); + if (result.getExitCode() != 0) { + LOGGER.warn(String.format("Disconnect failed for path [%s] with return code [%s]", multipathName, result.getExitCode())); + } + if (LOGGER.isDebugEnabled()) { + LOGGER.debug("multipath flush output: " + result.getResult()); + LOGGER.debug(String.format("disconnectPhysicalDiskByPath(localPath) called with args (%s) COMPLETE [rc=%s]", localPath, result.getExitCode())); + } + return (result.getExitCode() == 0); + } + } + } + if (LOGGER.isDebugEnabled()) LOGGER.debug(String.format("disconnectPhysicalDiskByPath(localPath) returning FALSE, volume path is not a multipath volume: %s", localPath)); + return false; } @Override public boolean deletePhysicalDisk(String uuid, KVMStoragePool pool, Storage.ImageFormat format) { - LOGGER.info(String.format("deletePhysicalDisk(uuid,pool,format) called with args (%s, %s, %s) [not implemented]", uuid, pool.getUuid(), format.toString())); - return true; + return false; } @Override @@ -276,15 +315,9 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { return true; } - /** - * Validate inputs and return the source file for a template copy - * @param templateFilePath - * @param destTemplatePath - * @param destPool - * @param format - * @return - */ - File createTemplateFromDirectDownloadFileValidate(String templateFilePath, String destTemplatePath, KVMStoragePool destPool, Storage.ImageFormat format) { + + @Override + public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, String destTemplatePath, KVMStoragePool destPool, Storage.ImageFormat format, int timeout) { if (StringUtils.isAnyEmpty(templateFilePath, destTemplatePath) || destPool == null) { LOGGER.error("Unable to create template from direct download template file due to insufficient data"); throw new CloudRuntimeException("Unable to create template from direct download template file due to insufficient data"); @@ -297,57 +330,18 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { throw new CloudRuntimeException("Direct download template file " + templateFilePath + " does not exist on this host"); } - if (destTemplatePath == null || destTemplatePath.isEmpty()) { - LOGGER.error("Failed to create template, target template disk path not provided"); - throw new CloudRuntimeException("Target template disk path not provided"); - } - - if (this.isStoragePoolTypeSupported(destPool.getType())) { - throw new CloudRuntimeException("Unsupported storage pool type: " + destPool.getType().toString()); - } - - if (Storage.ImageFormat.RAW.equals(format) && Storage.ImageFormat.QCOW2.equals(format)) { - LOGGER.error("Failed to create template, unsupported template format: " + format.toString()); - throw new CloudRuntimeException("Unsupported template format: " + format.toString()); - } - return sourceFile; - } - - String extractSourceTemplateIfNeeded(File sourceFile, String templateFilePath) { - String srcTemplateFilePath = templateFilePath; - if (isTemplateExtractable(templateFilePath)) { - srcTemplateFilePath = sourceFile.getParent() + "/" + UUID.randomUUID().toString(); - LOGGER.debug("Extract the downloaded template " + templateFilePath + " to " + srcTemplateFilePath); - String extractCommand = getExtractCommandForDownloadedFile(templateFilePath, srcTemplateFilePath); - Script.runSimpleBashScript(extractCommand); - Script.runSimpleBashScript("rm -f " + templateFilePath); - } - return srcTemplateFilePath; - } - - QemuImg.PhysicalDiskFormat deriveImgFileFormat(Storage.ImageFormat format) { - if (format == Storage.ImageFormat.RAW) { - return QemuImg.PhysicalDiskFormat.RAW; - } else if (format == Storage.ImageFormat.QCOW2) { - return QemuImg.PhysicalDiskFormat.QCOW2; - } else { - return QemuImg.PhysicalDiskFormat.RAW; - } - } - - @Override - public KVMPhysicalDisk createTemplateFromDirectDownloadFile(String templateFilePath, String destTemplatePath, KVMStoragePool destPool, Storage.ImageFormat format, int timeout) { - File sourceFile = createTemplateFromDirectDownloadFileValidate(templateFilePath, destTemplatePath, destPool, format); - LOGGER.debug("Create template from direct download template - file path: " + templateFilePath + ", dest path: " + destTemplatePath + ", format: " + format.toString()); - KVMPhysicalDisk sourceDisk = destPool.getPhysicalDisk(sourceFile.getAbsolutePath()); + KVMPhysicalDisk sourceDisk = destPool.getPhysicalDisk(templateFilePath); return copyPhysicalDisk(sourceDisk, destTemplatePath, destPool, timeout, null, null, Storage.ProvisioningType.THIN); } @Override public KVMPhysicalDisk copyPhysicalDisk(KVMPhysicalDisk disk, String name, KVMStoragePool destPool, int timeout, byte[] srcPassphrase, byte[] dstPassphrase, Storage.ProvisioningType provisioningType) { + if (StringUtils.isEmpty(name) || disk == null || destPool == null) { + LOGGER.error("Unable to copy physical disk due to insufficient data"); + throw new CloudRuntimeException("Unable to copy physical disk due to insufficient data"); + } - validateForDiskCopy(disk, name, destPool); LOGGER.info("Copying FROM source physical disk " + disk.getPath() + ", size: " + disk.getSize() + ", virtualsize: " + disk.getVirtualSize()+ ", format: " + disk.getFormat()); KVMPhysicalDisk destDisk = destPool.getPhysicalDisk(name); @@ -367,60 +361,34 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { LOGGER.info("Copying TO destination physical disk " + destDisk.getPath() + ", size: " + destDisk.getSize() + ", virtualsize: " + destDisk.getVirtualSize()+ ", format: " + destDisk.getFormat()); QemuImgFile srcFile = new QemuImgFile(disk.getPath(), disk.getFormat()); QemuImgFile destFile = new QemuImgFile(destDisk.getPath(), destDisk.getFormat()); - LOGGER.debug("Starting COPY from source downloaded template " + srcFile.getFileName() + " to Primera volume: " + destDisk.getPath()); + + LOGGER.debug("Starting COPY from source path " + srcFile.getFileName() + " to target volume path: " + destDisk.getPath()); + ScriptResult result = runScript(copyScript, timeout, destDisk.getFormat().toString().toLowerCase(), srcFile.getFileName(), destFile.getFileName()); int rc = result.getExitCode(); if (rc != 0) { throw new CloudRuntimeException("Failed to convert from " + srcFile.getFileName() + " to " + destFile.getFileName() + " the error was: " + rc + " - " + result.getResult()); } - LOGGER.debug("Successfully converted source downloaded template " + srcFile.getFileName() + " to Primera volume: " + destDisk.getPath() + " " + result.getResult()); + LOGGER.debug("Successfully converted source volume at " + srcFile.getFileName() + " to destination volume: " + destDisk.getPath() + " " + result.getResult()); return destDisk; } - void validateForDiskCopy(KVMPhysicalDisk disk, String name, KVMStoragePool destPool) { - if (StringUtils.isEmpty(name) || disk == null || destPool == null) { - LOGGER.error("Unable to copy physical disk due to insufficient data"); - throw new CloudRuntimeException("Unable to copy physical disk due to insufficient data"); - } - } - - /** - * Copy a disk path to another disk path using QemuImg command - * @param disk - * @param destDisk - * @param name - * @param timeout - */ - void qemuCopy(KVMPhysicalDisk disk, KVMPhysicalDisk destDisk, String name, int timeout) { - QemuImg qemu; - try { - qemu = new QemuImg(timeout); - } catch (LibvirtException | QemuImgException e) { - throw new CloudRuntimeException (e); - } - QemuImgFile srcFile = null; - QemuImgFile destFile = null; - - try { - srcFile = new QemuImgFile(disk.getPath(), disk.getFormat()); - destFile = new QemuImgFile(destDisk.getPath(), destDisk.getFormat()); - - LOGGER.debug("Starting copy from source disk image " + srcFile.getFileName() + " to volume: " + destDisk.getPath()); - qemu.convert(srcFile, destFile, true); - LOGGER.debug("Successfully converted source disk image " + srcFile.getFileName() + " to volume: " + destDisk.getPath()); - } catch (QemuImgException | LibvirtException e) { - try { - Map srcInfo = qemu.info(srcFile); - LOGGER.debug("Source disk info: " + Arrays.asList(srcInfo)); - } catch (Exception ignored) { - LOGGER.warn("Unable to get info from source disk: " + disk.getName()); - } - - String errMsg = String.format("Unable to convert/copy from %s to %s, due to: %s", disk.getName(), name, ((StringUtils.isEmpty(e.getMessage())) ? "an unknown error" : e.getMessage())); - LOGGER.error(errMsg); - throw new CloudRuntimeException(errMsg, e); + private static final ScriptResult runScript(String script, long timeout, String...args) { + ScriptResult result = new ScriptResult(); + Script cmd = new Script(script, Duration.millis(timeout), LOGGER); + cmd.add(args); + OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); + String output = cmd.execute(parser); + // its possible the process never launches which causes an NPE on getExitValue below + if (output != null && output.contains("Unable to execute the command")) { + result.setResult(output); + result.setExitCode(-1); + return result; } + result.setResult(output); + result.setExitCode(cmd.getExitValue()); + return result; } @Override @@ -461,25 +429,9 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { } } - private static final ScriptResult runScript(String script, long timeout, String...args) { - ScriptResult result = new ScriptResult(); - Script cmd = new Script(script, Duration.millis(timeout), LOGGER); - cmd.add(args); - OutputInterpreter.OneLineParser parser = new OutputInterpreter.OneLineParser(); - String output = cmd.execute(parser); - // its possible the process never launches which causes an NPE on getExitValue below - if (output != null && output.contains("Unable to execute the command")) { - result.setResult(output); - result.setExitCode(-1); - return result; - } - result.setResult(output); - result.setExitCode(cmd.getExitValue()); - return result; - } - boolean waitForDiskToBecomeAvailable(AddressInfo address, KVMStoragePool pool, long waitTimeInSec) { LOGGER.debug("Waiting for the volume with id: " + address.getPath() + " of the storage pool: " + pool.getUuid() + " to become available for " + waitTimeInSec + " secs"); + long scriptTimeoutSecs = 30; // how long to wait for each script execution to run long maxTries = 10; // how many max retries to attempt the script long waitTimeInMillis = waitTimeInSec * 1000; // how long overall to wait @@ -557,40 +509,6 @@ public abstract class MultipathSCSIAdapterBase implements StorageAdaptor { return false; } - void runConnectScript(String lun, AddressInfo address) { - try { - ProcessBuilder builder = new ProcessBuilder(connectScript, lun, address.getAddress()); - Process p = builder.start(); - int rc = p.waitFor(); - StringBuffer output = new StringBuffer(); - if (rc == 0) { - BufferedReader input = new BufferedReader(new InputStreamReader(p.getInputStream())); - String line = null; - while ((line = input.readLine()) != null) { - output.append(line); - output.append(" "); - } - } else { - LOGGER.warn("Failure discovering LUN via " + connectScript); - BufferedReader error = new BufferedReader(new InputStreamReader(p.getErrorStream())); - String line = null; - while ((line = error.readLine()) != null) { - LOGGER.warn("error --> " + line); - } - } - } catch (IOException | InterruptedException e) { - throw new CloudRuntimeException("Problem performing scan on SCSI hosts", e); - } - } - - void sleep(long sleepTimeMs) { - try { - Thread.sleep(sleepTimeMs); - } catch (Exception ex) { - // don't do anything - } - } - long getPhysicalDiskSize(String diskPath) { if (StringUtils.isEmpty(diskPath)) { return 0; diff --git a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java index 4f3ed6bda5b..0159deda347 100644 --- a/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java +++ b/plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessorTest.java @@ -26,19 +26,10 @@ import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtUtilitiesHelper; import com.cloud.storage.template.TemplateConstants; import com.cloud.utils.Pair; import com.cloud.utils.exception.CloudRuntimeException; -import javax.naming.ConfigurationException; - import com.cloud.utils.script.Script; -import java.io.File; -import java.io.IOException; -import java.nio.file.Files; -import java.nio.file.Path; -import java.util.ArrayList; -import java.util.HashSet; -import java.util.List; -import java.util.Set; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.to.VolumeObjectTO; +import org.apache.cloudstack.utils.qemu.QemuImageOptions; import org.apache.cloudstack.utils.qemu.QemuImg; import org.apache.cloudstack.utils.qemu.QemuImgException; import org.apache.cloudstack.utils.qemu.QemuImgFile; @@ -59,6 +50,17 @@ import org.mockito.MockitoAnnotations; import org.mockito.Spy; import org.mockito.junit.MockitoJUnitRunner; +import javax.naming.ConfigurationException; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + @RunWith(MockitoJUnitRunner.class) public class KVMStorageProcessorTest { @@ -259,40 +261,48 @@ public class KVMStorageProcessorTest { } @Test - public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithQemuImgExceptionReturnErrorMessage() throws Exception { - String baseFile = "baseFile"; - String snapshotPath = "snapshotPath"; + public void convertBaseFileToSnapshotFileInPrimaryStorageDirTestFailToConvertWithQemuImgExceptionReturnErrorMessage() throws QemuImgException { + KVMPhysicalDisk baseFile = Mockito.mock(KVMPhysicalDisk.class); String errorMessage = "error"; - String expectedResult = String.format("Failed to convert %s snapshot of volume [%s] to [%s] due to [%s].", volumeObjectToMock, baseFile, snapshotPath, errorMessage); + KVMStoragePool primaryPoolMock = Mockito.mock(KVMStoragePool.class); + KVMPhysicalDisk baseFileMock = Mockito.mock(KVMPhysicalDisk.class); + VolumeObjectTO volumeMock = Mockito.mock(VolumeObjectTO.class); + QemuImgFile srcFileMock = Mockito.mock(QemuImgFile.class); + QemuImgFile destFileMock = Mockito.mock(QemuImgFile.class); + QemuImg qemuImgMock = Mockito.mock(QemuImg.class); - Mockito.doReturn(true).when(kvmStoragePoolMock).createFolder(Mockito.anyString()); - try (MockedConstruction ignored = Mockito.mockConstruction(QemuImg.class, (mock,context) -> { - Mockito.doThrow(new QemuImgException(errorMessage)).when(mock).convert(Mockito.any(QemuImgFile.class), Mockito.any(QemuImgFile.class)); - })) { - String result = storageProcessorSpy.convertBaseFileToSnapshotFileInPrimaryStorageDir(kvmStoragePoolMock, baseFile, snapshotPath, volumeObjectToMock, 1); - Assert.assertEquals(expectedResult, result); + Mockito.when(baseFileMock.getPath()).thenReturn("/path/to/baseFile"); + Mockito.when(primaryPoolMock.createFolder(Mockito.anyString())).thenReturn(true); + try (MockedConstruction