From 0602c7d387cdbb663be97a2ab536481d05cb8bca Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Tue, 2 Feb 2021 11:03:09 +0200 Subject: [PATCH 01/33] Added disk provisioning type support for VMWare --- .../com/cloud/agent/api/StoragePoolInfo.java | 4 + .../com/cloud/storage/StorageService.java | 2 +- .../storage/UpdateStorageCapabilitiesCmd.java | 80 ++++++++++ .../AbstractStoragePoolAllocator.java | 24 ++- .../provider/DefaultHostListener.java | 7 + .../vmware/resource/VmwareResource.java | 23 +++ .../resource/VmwareStorageProcessor.java | 27 ++-- .../cloud/server/ManagementServerImpl.java | 6 +- .../com/cloud/storage/StorageManagerImpl.java | 15 ++ test/integration/smoke/test_disk_offerings.py | 3 +- .../smoke/test_disk_provisioning_types.py | 142 ++++++++++++++++++ tools/apidoc/gen_toc.py | 2 +- tools/marvin/marvin/lib/base.py | 6 + .../vmware/mo/VirtualMachineMO.java | 33 +++- .../mo/VirtualStorageObjectManagerMO.java | 16 +- 15 files changed, 363 insertions(+), 27 deletions(-) create mode 100644 api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java create mode 100644 test/integration/smoke/test_disk_provisioning_types.py diff --git a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java index d923694a854d..10cc77fd6ed7 100644 --- a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java +++ b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java @@ -107,4 +107,8 @@ public void setUuid(String uuid) { public Map getDetails() { return details; } + + public void setDetails(Map details) { + this.details = details; + } } diff --git a/api/src/main/java/com/cloud/storage/StorageService.java b/api/src/main/java/com/cloud/storage/StorageService.java index 4b18739b55dc..c15ea10c2689 100644 --- a/api/src/main/java/com/cloud/storage/StorageService.java +++ b/api/src/main/java/com/cloud/storage/StorageService.java @@ -105,6 +105,6 @@ public interface StorageService { ImageStore updateImageStoreStatus(Long id, Boolean readonly); - StoragePool syncStoragePool(SyncStoragePoolCmd cmd); + void syncHardwareCapability(long poolId); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java new file mode 100644 index 000000000000..ab1b5256ecbd --- /dev/null +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -0,0 +1,80 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.admin.storage; + +import com.cloud.exception.ConcurrentOperationException; +import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.NetworkRuleConflictException; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.exception.ResourceUnavailableException; +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.api.ApiConstants; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.Parameter; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.StoragePoolResponse; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.apache.cloudstack.context.CallContext; + +@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs hardware acceleration capabilities of storage pools", + responseObject = SuccessResponse.class, + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) +public class UpdateStorageCapabilitiesCmd extends BaseCmd { + public static final String APINAME = "updateStorageCapabilities"; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = true, description = "Storage pool id") + private Long poolId; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + public Long getPoolId() { + return poolId; + } + + public void setPoolId(Long poolId) { + this.poolId = poolId; + } + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + + @Override + public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { + _storageService.syncHardwareCapability(poolId); + SuccessResponse response = new SuccessResponse(getCommandName()); + response.setSuccess(true); + this.setResponseObject(response); + } + + @Override + public String getCommandName() { + return APINAME; + } + + @Override + public long getEntityOwnerId() { + return CallContext.current().getCallingAccountId(); + } +} diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 7de40e924de8..d29d1666df48 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -26,12 +26,17 @@ import javax.inject.Inject; import javax.naming.ConfigurationException; +import com.cloud.exception.StorageUnavailableException; +import com.cloud.storage.StoragePoolStatus; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; +import org.apache.log4j.Logger; + import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.StoragePoolAllocator; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; -import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; -import org.apache.log4j.Logger; import com.cloud.capacity.Capacity; import com.cloud.capacity.dao.CapacityDao; @@ -39,12 +44,10 @@ import com.cloud.dc.dao.ClusterDao; import com.cloud.deploy.DeploymentPlan; import com.cloud.deploy.DeploymentPlanner.ExcludeList; -import com.cloud.exception.StorageUnavailableException; import com.cloud.hypervisor.Hypervisor.HypervisorType; import com.cloud.storage.Storage; import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; -import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.StorageUtil; import com.cloud.storage.Volume; import com.cloud.storage.dao.VolumeDao; @@ -60,6 +63,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1); protected String allocationAlgorithm = "random"; protected long extraBytesPerVolume = 0; + protected boolean diskProvisioningTypeStrictness = false; @Inject protected DataStoreManager dataStoreMgr; @Inject protected PrimaryDataStoreDao storagePoolDao; @Inject protected VolumeDao volumeDao; @@ -68,6 +72,7 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement @Inject private ClusterDao clusterDao; @Inject private StorageManager storageMgr; @Inject private StorageUtil storageUtil; + @Inject private StoragePoolDetailsDao storagePoolDetailsDao; @Override public boolean configure(String name, Map params) throws ConfigurationException { @@ -81,6 +86,10 @@ public boolean configure(String name, Map params) throws Configu if (allocationAlgorithm != null) { this.allocationAlgorithm = allocationAlgorithm; } + String strictness = configs.get("disk.provisioning.type.strictness"); + if (strictness != null) { + diskProvisioningTypeStrictness = strictness.equals("true"); + } return true; } return false; @@ -211,6 +220,13 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh, return false; } + if (diskProvisioningTypeStrictness){ + StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported"); + if (!dskCh.getProvisioningType().equals("thin") && hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true")){ + return false; + } + } + if(!checkHypervisorCompatibility(dskCh.getHypervisorType(), dskCh.getType(), pool.getPoolType())){ return false; } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index eb2262f0298d..534f99267324 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -126,6 +126,13 @@ private void updateStoragePoolHostVOAndDetails(StoragePool pool, long hostId, Mo storagePoolDetailsDao.persist(storagePoolDetailVO); } } + if (mspAnswer.getPoolInfo().getDetails() != null && mspAnswer.getPoolInfo().getDetails().containsKey("hardwareAccelerationSupported")){ + StoragePoolDetailVO hardwareAccelerationSupported = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported"); + if (hardwareAccelerationSupported == null) { + StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), "hardwareAccelerationSupported", mspAnswer.getPoolInfo().getDetails().get("hardwareAccelerationSupported"), false); + storagePoolDetailsDao.persist(storagePoolDetailVO); + } + } primaryStoreDao.update(pool.getId(), poolVO); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index f19f7cc5f770..2979e26594ca 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -296,6 +296,8 @@ import com.vmware.vim25.GuestInfo; import com.vmware.vim25.GuestNicInfo; import com.vmware.vim25.HostCapability; +import com.vmware.vim25.HostConfigInfo; +import com.vmware.vim25.HostFileSystemMountInfo; import com.vmware.vim25.HostHostBusAdapter; import com.vmware.vim25.HostInternetScsiHba; import com.vmware.vim25.HostPortGroupSpec; @@ -5029,6 +5031,16 @@ protected Answer execute(ModifyStoragePoolCommand cmd) { answer.setLocalDatastoreName(morDatastore.getValue()); } + HostMO host = (HostMO) hyperHost; + boolean hardwareAccelerationSupportForDataStore = getHardwareAcceleationSupportForDataStore(host.getMor(), dsMo.getName()); + StoragePoolInfo poolInfo = answer.getPoolInfo(); + Map poolDetails = poolInfo.getDetails(); + if (poolDetails == null) { + poolDetails = new HashMap<>(); + } + poolDetails.put("hardwareAccelerationSupported", String.valueOf(hardwareAccelerationSupportForDataStore)); + poolInfo.setDetails(poolDetails); + return answer; } catch (Throwable e) { if (e instanceof RemoteException) { @@ -5045,6 +5057,17 @@ protected Answer execute(ModifyStoragePoolCommand cmd) { } } + private boolean getHardwareAcceleationSupportForDataStore(ManagedObjectReference host, String dataStoreName) throws Exception { + HostConfigInfo config = getServiceContext().getVimClient().getDynamicProperty(host, "config"); + List mountInfoList = config.getFileSystemVolume().getMountInfo(); + for (HostFileSystemMountInfo hostFileSystemMountInfo: mountInfoList) { + if ( hostFileSystemMountInfo.getVolume().getName().equals(dataStoreName) ) { + return hostFileSystemMountInfo.getVStorageSupport().equals("vStorageSupported"); + } + } + return false; + } + private void handleTargets(boolean add, ModifyTargetsCommand.TargetTypeToRemove targetTypeToRemove, boolean isRemoveAsync, List> targets, List hosts) { if (targets != null && targets.size() > 0) { diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index da6713746b84..90adcdfa02b8 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -92,6 +92,7 @@ import com.cloud.storage.DataStoreRole; import com.cloud.storage.JavaStorageLayer; import com.cloud.storage.Storage.ImageFormat; +import com.cloud.storage.Storage.ProvisioningType; import com.cloud.storage.StorageLayer; import com.cloud.storage.Volume; import com.cloud.storage.template.OVAProcessor; @@ -775,10 +776,10 @@ private boolean createVMLinkedClone(VirtualMachineMO vmTemplate, DatacenterMO dc } private boolean createVMFullClone(VirtualMachineMO vmTemplate, DatacenterMO dcMo, DatastoreMO dsMo, String vmdkName, ManagedObjectReference morDatastore, - ManagedObjectReference morPool) throws Exception { + ManagedObjectReference morPool, ProvisioningType diskProvisioningType) throws Exception { s_logger.info("creating full clone from template"); - if (!vmTemplate.createFullClone(vmdkName, dcMo.getVmFolder(), morPool, morDatastore)) { + if (!vmTemplate.createFullClone(vmdkName, dcMo.getVmFolder(), morPool, morDatastore, diskProvisioningType)) { String msg = "Unable to create full clone from the template"; s_logger.error(msg); @@ -866,7 +867,7 @@ public Answer cloneVolumeFromBaseTemplate(CopyCommand cmd) { if (dsMo.getDatastoreType().equalsIgnoreCase("VVOL")) { vmdkFileBaseName = cloneVMforVvols(context, hyperHost, template, vmTemplate, volume, dcMo, dsMo); } else { - vmdkFileBaseName = createVMFolderWithVMName(context, hyperHost, template, vmTemplate, volume, dcMo, dsMo, searchExcludedFolders); + vmdkFileBaseName = createVMAndFolderWithVMName(context, hyperHost, template, vmTemplate, volume, dcMo, dsMo, searchExcludedFolders); } } // restoreVM - move the new ROOT disk into corresponding VM folder @@ -917,7 +918,7 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper if (!_fullCloneFlag) { createVMLinkedClone(vmTemplate, dcMo, vmName, morDatastore, morPool); } else { - createVMFullClone(vmTemplate, dcMo, dsMo, vmName, morDatastore, morPool); + createVMFullClone(vmTemplate, dcMo, dsMo, vmName, morDatastore, morPool, null); } VirtualMachineMO vmMo = new ClusterMO(context, morCluster).findVmOnHyperHost(vmName); @@ -931,21 +932,21 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper return vmdkFileBaseName; } - private String createVMFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template, - VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo, - String searchExcludedFolders) throws Exception { + private String createVMAndFolderWithVMName(VmwareContext context, VmwareHypervisorHost hyperHost, TemplateObjectTO template, + VirtualMachineMO vmTemplate, VolumeObjectTO volume, DatacenterMO dcMo, DatastoreMO dsMo, + String searchExcludedFolders) throws Exception { String vmdkName = volume.getName(); try { ManagedObjectReference morDatastore = dsMo.getMor(); ManagedObjectReference morPool = hyperHost.getHyperHostOwnerResourcePool(); ManagedObjectReference morCluster = hyperHost.getHyperHostCluster(); - if (template.getSize() != null){ + if (template.getSize() != null) { _fullCloneFlag = volume.getSize() > template.getSize() ? true : _fullCloneFlag; } if (!_fullCloneFlag) { createVMLinkedClone(vmTemplate, dcMo, vmdkName, morDatastore, morPool); } else { - createVMFullClone(vmTemplate, dcMo, dsMo, vmdkName, morDatastore, morPool); + createVMFullClone(vmTemplate, dcMo, dsMo, vmdkName, morDatastore, morPool, volume.getProvisioningType()); } VirtualMachineMO vmMo = new ClusterMO(context, morCluster).findVmOnHyperHost(vmdkName); @@ -956,7 +957,7 @@ private String createVMFolderWithVMName(VmwareContext context, VmwareHypervisorH String[] vmwareLayoutFilePair = VmwareStorageLayoutHelper.getVmdkFilePairDatastorePath(dsMo, vmdkName, vmdkFileBaseName, VmwareStorageLayoutType.VMWARE, !_fullCloneFlag); String[] legacyCloudStackLayoutFilePair = VmwareStorageLayoutHelper.getVmdkFilePairDatastorePath(dsMo, vmdkName, vmdkFileBaseName, VmwareStorageLayoutType.CLOUDSTACK_LEGACY, !_fullCloneFlag); - for (int i=0; i sshKeyLength = new ConfigKey("Advanced", Integer.class, "ssh.key.length", "2048", "Specifies custom SSH key length (bit)", true, ConfigKey.Scope.Global); static final ConfigKey humanReadableSizes = new ConfigKey("Advanced", Boolean.class, "display.human.readable.sizes", "true", "Enables outputting human readable byte sizes to logs and usage records.", false, ConfigKey.Scope.Global); public static final ConfigKey customCsIdentifier = new ConfigKey("Advanced", String.class, "custom.cs.identifier", UUID.randomUUID().toString().split("-")[0].substring(4), "Custom identifier for the cloudstack installation", true, ConfigKey.Scope.Global); + static final ConfigKey diskProvisioningStrictness = new ConfigKey("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings.", true, ConfigKey.Scope.Zone); @Inject public AccountManager _accountMgr; @@ -3040,7 +3042,7 @@ public List> getCommands() { cmdList.add(FindStoragePoolsForMigrationCmd.class); cmdList.add(PreparePrimaryStorageForMaintenanceCmd.class); cmdList.add(UpdateStoragePoolCmd.class); - cmdList.add(SyncStoragePoolCmd.class); + cmdList.add(UpdateStorageCapabilitiesCmd.class); cmdList.add(UpdateImageStoreCmd.class); cmdList.add(DestroySystemVmCmd.class); cmdList.add(ListSystemVMsCmd.class); @@ -3457,7 +3459,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {vmPasswordLength, sshKeyLength, humanReadableSizes, customCsIdentifier}; + return new ConfigKey[] {vmPasswordLength, sshKeyLength, humanReadableSizes, diskProvisioningStrictness, customCsIdentifier}; } protected class EventPurgeTask extends ManagedContextRunnable { diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 7bf692442528..5bb27e40b1c3 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -316,8 +316,12 @@ public class StorageManagerImpl extends ManagerBase implements StorageManager, C @Inject SnapshotService _snapshotService; @Inject + public StorageService storageService; + @Inject StoragePoolTagsDao _storagePoolTagsDao; @Inject + PrimaryDataStoreDao primaryStoreDao; + @Inject DiskOfferingDetailsDao _diskOfferingDetailsDao; @Inject ServiceOfferingDetailsDao _serviceOfferingDetailsDao; @@ -2773,6 +2777,17 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) { return imageStoreVO; } + @Override + public void syncHardwareCapability(long poolId) { + StoragePoolVO pool = _storagePoolDao.findById(poolId); + // find the host + List hosts = _storagePoolHostDao.listByPoolId(poolId); + if (hosts.size() > 0) { + StoragePoolHostVO host = hosts.get(0); + _agentMgr.easySend(host.getHostId(), new ModifyStoragePoolCommand(false, pool)); + } + } + private void duplicateCacheStoreRecordsToRegionStore(long storeId) { _templateStoreDao.duplicateCacheRecordsOnRegionStore(storeId); _snapshotStoreDao.duplicateCacheRecordsOnRegionStore(storeId); diff --git a/test/integration/smoke/test_disk_offerings.py b/test/integration/smoke/test_disk_offerings.py index d0d3433e96c1..660dd30024d7 100644 --- a/test/integration/smoke/test_disk_offerings.py +++ b/test/integration/smoke/test_disk_offerings.py @@ -19,7 +19,6 @@ #Import Local Modules import marvin from marvin.cloudstackTestCase import * -from marvin.cloudstackAPI import * from marvin.lib.utils import * from marvin.lib.base import * from marvin.lib.common import * @@ -134,7 +133,7 @@ def test_02_create_sparse_type_disk_offering(self): @attr(hypervisor="kvm") @attr(tags = ["advanced", "basic", "eip", "sg", "advancedns", "simulator", "smoke"]) def test_04_create_fat_type_disk_offering(self): - """Test to create a sparse type disk offering""" + """Test to create a sparse type disk offering""" # Validate the following: # 1. createDiskOfferings should return valid info for new offering diff --git a/test/integration/smoke/test_disk_provisioning_types.py b/test/integration/smoke/test_disk_provisioning_types.py new file mode 100644 index 000000000000..f6f290e6d7fb --- /dev/null +++ b/test/integration/smoke/test_disk_provisioning_types.py @@ -0,0 +1,142 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.lib.utils import cleanup_resources +from marvin.lib.base import DiskOffering, Iso, Account, VirtualMachine, ServiceOffering, Volume +from marvin.codes import FAILED +from marvin.lib.common import list_disk_offering, get_zone, get_suitable_test_template, get_domain +from marvin.cloudstackAPI import listStoragePools, updateStorageCapabilities +from nose.plugins.attrib import attr + + +class TestDiskProvisioningTypes(cloudstackTestCase): + + def setUp(self): + self.services = self.testClient.getParsedTestDataConfig() + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests()) + self.domain = get_domain(self.apiclient) + self.services['mode'] = self.zone.networktype + self.hypervisor = self.hypervisor = self.testClient.getHypervisorInfo() + + template = get_suitable_test_template( + self.apiclient, + self.zone.id, + self.services["ostype"], + self.hypervisor + ) + + if template == FAILED: + assert False, "get_suitable_test_template() failed to return template with description %s" % self.services["ostype"] + + self.account = Account.create( + self.apiclient, + self.services["account"], + domainid=self.domain.id + ) + + self.services["small"]["zoneid"] = self.zone.id + self.services["small"]["template"] = template.id + + self.services["iso1"]["zoneid"] = self.zone.id + + iso = Iso.create( + self.apiclient, + self.services["iso1"], + account=self.account.name, + domainid=self.account.domainid + ) + + self.cleanup = [ + self.account + ] + + + def tearDown(self): + cleanup_resources(self.apiclient, self.cleanup) + + @attr(tags=["advanced", "basic", "eip", "sg", "advancedns", "smoke"], required_hardware="false") + def test_01_vm_with_thin_disk_offering(self): + self.runner("thin") + + @attr(tags=["advanced", "basic", "eip", "sg", "advancedns", "smoke"], required_hardware="false") + def test_02_vm_with_fat_disk_offering(self): + self.runner("fat") + + @attr(tags=["advanced", "basic", "eip", "sg", "advancedns", "smoke"], required_hardware="false") + def test_03_vm_with_sparse_disk_offering(self): + self.runner("sparse") + + @attr(tags=["advanced", "basic", "eip", "sg", "advancedns", "smoke"], required_hardware="false") + def test_05_update_cmd(self): + cmd = listStoragePools.listStoragePoolsCmd() + storagePools = self.apiclient.listStoragePools(cmd) + pool = storagePools[0] + cmd = updateStorageCapabilities.updateStorageCapabilitiesCmd() + cmd.id = pool.id + response = self.apiclient.updateStorageCapabilities(cmd) + self.assertEqual( + response['success'], + True, + "Check Updated storage pool capabilities" + ) + + def runner(self, provisioning_type): + self.services["disk_offering"]['provisioningtype'] = provisioning_type + self.services["small"]['size'] = "1" + disk_offering = DiskOffering.create( + self.apiclient, + self.services["disk_offering"], + custom=True, + ) + self.cleanup.append(disk_offering) + + self.debug("Created Disk offering with ID: %s" % disk_offering.id) + + self.services["service_offerings"]["small"]["provisioningtype"] = provisioning_type + small_offering = ServiceOffering.create( + self.apiclient, + self.services["service_offerings"]["small"] + ) + + self.cleanup.append(small_offering) + + self.debug("Created service offering with ID: %s" % small_offering.id) + + virtual_machine = VirtualMachine.create( + self.apiclient, + self.services["small"], + accountid=self.account.name, + domainid=self.account.domainid, + serviceofferingid=small_offering.id, + diskofferingid=disk_offering.id, + mode=self.services["mode"] + ) + + self.debug("Created virtual machine with ID: %s" % virtual_machine.id) + + volumes = Volume.list(self.apiclient, virtualMachineId=virtual_machine.id, listAll='true') + + for volume in volumes: + if volume["type"] == "DATADISK": + VirtualMachine.detach_volume(virtual_machine, self.apiclient, volume) + currentVolume = Volume({}) + currentVolume.id = volume.id + Volume.resize(currentVolume, self.apiclient, size='2') + VirtualMachine.attach_volume(virtual_machine, self.apiclient, volume) diff --git a/tools/apidoc/gen_toc.py b/tools/apidoc/gen_toc.py index 9107b7cba84b..8423539ea438 100644 --- a/tools/apidoc/gen_toc.py +++ b/tools/apidoc/gen_toc.py @@ -95,7 +95,7 @@ 'StorageMaintenance': 'Storage Pool', 'StoragePool': 'Storage Pool', 'StorageProvider': 'Storage Pool', - 'syncStoragePool': 'Storage Pool', + 'syncStorageCapabilites' : 'Storage Pool', 'SecurityGroup': 'Security Group', 'SSH': 'SSH', 'register': 'Registration', diff --git a/tools/marvin/marvin/lib/base.py b/tools/marvin/marvin/lib/base.py index 916af64d96cc..37ef92888885 100755 --- a/tools/marvin/marvin/lib/base.py +++ b/tools/marvin/marvin/lib/base.py @@ -635,6 +635,9 @@ def create(cls, apiclient, services, templateid=None, accountid=None, if rootdiskcontroller: cmd.details[0]["rootDiskController"] = rootdiskcontroller + if "size" in services: + cmd.size = services["size"] + if group: cmd.group = group @@ -2296,6 +2299,9 @@ def create(cls, apiclient, services, tags=None, domainid=None, cacheMode=None, * if "offerha" in services: cmd.offerha = services["offerha"] + if "provisioningtype" in services: + cmd.provisioningtype = services["provisioningtype"] + if "dynamicscalingenabled" in services: cmd.dynamicscalingenabled = services["dynamicscalingenabled"] diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index 0d01931989bf..ca05ea9e9a52 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -36,6 +36,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; +import com.cloud.storage.Storage; import com.cloud.utils.exception.CloudRuntimeException; import com.vmware.vim25.InvalidStateFaultMsg; import com.vmware.vim25.RuntimeFaultFaultMsg; @@ -777,7 +778,7 @@ public boolean createFullCloneWithSpecificDisk(String cloneName, ManagedObjectRe return false; } - public boolean createFullClone(String cloneName, ManagedObjectReference morFolder, ManagedObjectReference morResourcePool, ManagedObjectReference morDs) + public boolean createFullClone(String cloneName, ManagedObjectReference morFolder, ManagedObjectReference morResourcePool, ManagedObjectReference morDs, Storage.ProvisioningType diskProvisioningType) throws Exception { VirtualMachineCloneSpec cloneSpec = new VirtualMachineCloneSpec(); @@ -788,6 +789,9 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde relocSpec.setDatastore(morDs); relocSpec.setPool(morResourcePool); + + setDiskProvsioningType(relocSpec, morDs, diskProvisioningType); + ManagedObjectReference morTask = _context.getService().cloneVMTask(_mor, morFolder, cloneName, cloneSpec); boolean result = _context.getVimClient().waitForTask(morTask); @@ -801,6 +805,33 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde return false; } + private void setDiskProvsioningType(VirtualMachineRelocateSpec relocSpec, ManagedObjectReference morDs, + Storage.ProvisioningType diskProvisioningType) throws Exception { + if (diskProvisioningType == null){ + return; + } + List relocateDisks = relocSpec.getDisk(); + List disks = this.getVirtualDisks(); + for (VirtualDisk disk: disks){ + VirtualDiskFlatVer2BackingInfo backing = (VirtualDiskFlatVer2BackingInfo) disk.getBacking(); + if (diskProvisioningType == Storage.ProvisioningType.FAT) { + backing.setThinProvisioned(false); + backing.setEagerlyScrub(true); + } else if (diskProvisioningType == Storage.ProvisioningType.THIN) { + backing.setThinProvisioned(true); + } else if (diskProvisioningType == Storage.ProvisioningType.SPARSE) { + backing.setThinProvisioned(false); + backing.setEagerlyScrub(false); + } + + VirtualMachineRelocateSpecDiskLocator diskLocator = new VirtualMachineRelocateSpecDiskLocator(); + diskLocator.setDiskId(disk.getKey()); + diskLocator.setDiskBackingInfo(backing); + diskLocator.setDatastore(morDs); + relocateDisks.add(diskLocator); + } + } + public boolean createLinkedClone(String cloneName, ManagedObjectReference morBaseSnapshot, ManagedObjectReference morFolder, ManagedObjectReference morResourcePool, ManagedObjectReference morDs) throws Exception { diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualStorageObjectManagerMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualStorageObjectManagerMO.java index d5f4eb3af060..c4c93a03b11f 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualStorageObjectManagerMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualStorageObjectManagerMO.java @@ -16,10 +16,11 @@ // under the License. package com.cloud.hypervisor.vmware.mo; +import com.cloud.storage.Storage; import com.vmware.vim25.ID; import com.vmware.vim25.TaskInfo; import com.vmware.vim25.VStorageObject; -import com.vmware.vim25.VirtualDiskType; +import com.vmware.vim25.BaseConfigInfoDiskFileBackingInfoProvisioningType; import com.vmware.vim25.VslmCreateSpec; import com.vmware.vim25.VslmCreateSpecDiskFileBackingSpec; import org.apache.log4j.Logger; @@ -60,12 +61,21 @@ public VStorageObject retrieveVirtualDisk (ID id, ManagedObjectReference morDS) return _context.getService().retrieveVStorageObject(_mor, id, morDS); } - public VStorageObject createDisk(ManagedObjectReference morDS, VirtualDiskType diskType, long currentSizeInBytes, String datastoreFilepath, String filename) throws Exception { + public VStorageObject createDisk(ManagedObjectReference morDS, Storage.ProvisioningType diskProvisioningType, long currentSizeInBytes, String datastoreFilepath, String filename) throws Exception { long currentSizeInMB = currentSizeInBytes/(1024*1024); VslmCreateSpecDiskFileBackingSpec diskFileBackingSpec = new VslmCreateSpecDiskFileBackingSpec(); diskFileBackingSpec.setDatastore(morDS); - diskFileBackingSpec.setProvisioningType(diskType.value()); + if (diskProvisioningType != null) { + if (diskProvisioningType == Storage.ProvisioningType.FAT) { + diskFileBackingSpec.setProvisioningType(BaseConfigInfoDiskFileBackingInfoProvisioningType.EAGER_ZEROED_THICK.value()); + } else if (diskProvisioningType == Storage.ProvisioningType.THIN) { + diskFileBackingSpec.setProvisioningType(BaseConfigInfoDiskFileBackingInfoProvisioningType.THIN.value()); + } else if (diskProvisioningType == Storage.ProvisioningType.SPARSE) { + diskFileBackingSpec.setProvisioningType(BaseConfigInfoDiskFileBackingInfoProvisioningType.LAZY_ZEROED_THICK.value()); + } + } + // path should be just the folder name. For example, instead of '[datastore1] folder1/filename.vmdk' you would just do 'folder1'. // path is introduced from 6.7. In 6.5 disk will be created in the default folder "fcd" diskFileBackingSpec.setPath(null); From ec5682b2a862c24beb939772d7de046bb27de9c0 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 8 Feb 2021 13:06:36 +0200 Subject: [PATCH 02/33] Review changes --- .../main/java/com/cloud/storage/Storage.java | 10 +++ .../com/cloud/storage/StorageService.java | 2 +- .../storage/UpdateStorageCapabilitiesCmd.java | 4 +- .../api/GetStoragePoolCapabilitiesAnswer.java | 19 ++++++ .../GetStoragePoolCapabilitiesCommand.java | 21 ++++++ .../storage/to/PrimaryDataStoreTO.java | 9 +++ .../com/cloud/storage/StorageManager.java | 2 +- .../datastore/db/StoragePoolDetailVO.java | 4 ++ .../motion/AncientDataMotionStrategy.java | 24 ++++--- .../motion/AncientDataMotionStrategyTest.java | 2 +- .../AbstractStoragePoolAllocator.java | 24 ++++--- .../provider/DefaultHostListener.java | 21 +++--- .../vmware/resource/VmwareResource.java | 67 ++++++++++++++++--- .../resource/VmwareStorageProcessor.java | 23 +++++-- .../VmwareStorageSubsystemCommandHandler.java | 4 ++ ...oudStackPrimaryDataStoreLifeCycleImpl.java | 8 +++ .../com/cloud/storage/StorageManagerImpl.java | 23 ++++++- .../vmware/mo/VirtualMachineMO.java | 6 +- 18 files changed, 219 insertions(+), 54 deletions(-) create mode 100644 core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java create mode 100644 core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java diff --git a/api/src/main/java/com/cloud/storage/Storage.java b/api/src/main/java/com/cloud/storage/Storage.java index 362cc2cac296..91724ba1ab0b 100644 --- a/api/src/main/java/com/cloud/storage/Storage.java +++ b/api/src/main/java/com/cloud/storage/Storage.java @@ -76,6 +76,16 @@ public String getFileExtension() { } + public static enum Capability { + HARDWARE_ACCELERATION("hardwareAcceleration"); + + private final String capability; + + private Capability(String capability) { + this.capability = capability; + } + } + public static enum ProvisioningType { THIN("thin"), SPARSE("sparse"), diff --git a/api/src/main/java/com/cloud/storage/StorageService.java b/api/src/main/java/com/cloud/storage/StorageService.java index c15ea10c2689..65a09476cd24 100644 --- a/api/src/main/java/com/cloud/storage/StorageService.java +++ b/api/src/main/java/com/cloud/storage/StorageService.java @@ -105,6 +105,6 @@ public interface StorageService { ImageStore updateImageStoreStatus(Long id, Boolean readonly); - void syncHardwareCapability(long poolId); + void updateStorageCapabilities(long poolId); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java index ab1b5256ecbd..a8d0c4546f0b 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -30,7 +30,7 @@ import org.apache.cloudstack.api.response.SuccessResponse; import org.apache.cloudstack.context.CallContext; -@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs hardware acceleration capabilities of storage pools", +@APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs capabilities of storage pools", responseObject = SuccessResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) public class UpdateStorageCapabilitiesCmd extends BaseCmd { @@ -62,7 +62,7 @@ public void setPoolId(Long poolId) { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { - _storageService.syncHardwareCapability(poolId); + _storageService.updateStorageCapabilities(poolId); SuccessResponse response = new SuccessResponse(getCommandName()); response.setSuccess(true); this.setResponseObject(response); diff --git a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java new file mode 100644 index 000000000000..bf1ae64e23db --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java @@ -0,0 +1,19 @@ +package com.cloud.agent.api; + +public class GetStoragePoolCapabilitiesAnswer extends Answer { + public GetStoragePoolCapabilitiesAnswer(GetStoragePoolCapabilitiesCommand cmd) { + super(cmd); + + poolInfo = new StoragePoolInfo(); + } + + public StoragePoolInfo getPoolInfo() { + return poolInfo; + } + + public void setPoolInfo(StoragePoolInfo poolInfo) { + this.poolInfo = poolInfo; + } + + private StoragePoolInfo poolInfo; +} diff --git a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java new file mode 100644 index 000000000000..7da8d75154b0 --- /dev/null +++ b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java @@ -0,0 +1,21 @@ +package com.cloud.agent.api; + +import com.cloud.agent.api.to.StorageFilerTO; + +public class GetStoragePoolCapabilitiesCommand extends Command { + + public StorageFilerTO getPool() { + return pool; + } + + public void setPool(StorageFilerTO pool) { + this.pool = pool; + } + + private StorageFilerTO pool; + + @Override + public boolean executeInSequence() { + return false; + } +} diff --git a/core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java b/core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java index 0bb5b7977703..9df2a6c955bc 100644 --- a/core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java +++ b/core/src/main/java/org/apache/cloudstack/storage/to/PrimaryDataStoreTO.java @@ -52,6 +52,7 @@ public class PrimaryDataStoreTO implements DataStoreTO { private Map details; private static final String pathSeparator = "/"; private Boolean fullCloneFlag; + private Boolean diskProvisioningStrictnessFlag; private final boolean isManaged; public PrimaryDataStoreTO(PrimaryDataStore dataStore) { @@ -163,4 +164,12 @@ public void setFullCloneFlag(Boolean fullCloneFlag) { public boolean isManaged() { return isManaged; } + + public Boolean getDiskProvisioningStrictnessFlag() { + return diskProvisioningStrictnessFlag; + } + + public void setDiskProvisioningStrictnessFlag(Boolean diskProvisioningStrictnessFlag) { + this.diskProvisioningStrictnessFlag = diskProvisioningStrictnessFlag; + } } diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 96cf333cb19a..47917b8c51bb 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -145,7 +145,7 @@ public interface StorageManager extends StorageService { ConfigKey MaxDataMigrationWaitTime = new ConfigKey("Advanced", Integer.class, "max.data.migration.wait.time", "15", "Maximum wait time for a data migration task before spawning a new SSVM", false, ConfigKey.Scope.Global); - + ConfigKey DiskProvisioningStrictness = new ConfigKey("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings if set to true.", true, ConfigKey.Scope.Zone); /** * Returns a comma separated list of tags for the specified storage pool * @param poolId diff --git a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolDetailVO.java b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolDetailVO.java index 8a746ff9d1d1..8c1428bbd157 100644 --- a/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolDetailVO.java +++ b/engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/StoragePoolDetailVO.java @@ -70,6 +70,10 @@ public String getName() { return name; } + public void setValue(String value) { + this.value = value; + } + @Override public String getValue() { return value; diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index c49ffba0b82b..e7775846c540 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -86,6 +86,9 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { @Inject StorageCacheManager cacheMgr; + @Inject + StorageManager storageManager; + @Override public StrategyPriority canHandle(DataObject srcData, DataObject destData) { return StrategyPriority.DEFAULT; @@ -156,7 +159,7 @@ protected Answer copyObject(DataObject srcData, DataObject destData, Host destHo srcForCopy = cacheData = cacheMgr.createCacheObject(srcData, destScope); } - CopyCommand cmd = new CopyCommand(srcForCopy.getTO(), addFullCloneFlagOnVMwareDest(destData.getTO()), primaryStorageDownloadWait, + CopyCommand cmd = new CopyCommand(srcForCopy.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(destData.getTO()), primaryStorageDownloadWait, VirtualMachineManager.ExecuteInSequence.value()); EndPoint ep = destHost != null ? RemoteHostEndPoint.getHypervisorHostEndPoint(destHost) : selector.select(srcForCopy, destData); if (ep == null) { @@ -210,18 +213,19 @@ protected Answer copyObject(DataObject srcData, DataObject destData, Host destHo * @param dataTO Dest data store TO * @return dataTO including fullCloneFlag, if provided */ - protected DataTO addFullCloneFlagOnVMwareDest(DataTO dataTO) { + protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO dataTO) { if (dataTO != null && dataTO.getHypervisorType().equals(Hypervisor.HypervisorType.VMware)){ DataStoreTO dataStoreTO = dataTO.getDataStore(); if (dataStoreTO != null && dataStoreTO instanceof PrimaryDataStoreTO){ PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStoreTO; - Boolean value = CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId()); - primaryDataStoreTO.setFullCloneFlag(value); + primaryDataStoreTO.setFullCloneFlag(CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId())); + primaryDataStoreTO.setDiskProvisioningStrictnessFlag(storageManager.DiskProvisioningStrictness.valueIn(primaryDataStoreTO.getId())); } } return dataTO; } + protected Answer copyObject(DataObject srcData, DataObject destData) { return copyObject(srcData, destData, null); } @@ -278,7 +282,7 @@ protected Answer copyVolumeFromSnapshot(DataObject snapObj, DataObject volObj) { ep = selector.select(srcData, volObj); } - CopyCommand cmd = new CopyCommand(srcData.getTO(), addFullCloneFlagOnVMwareDest(volObj.getTO()), _createVolumeFromSnapshotWait, VirtualMachineManager.ExecuteInSequence.value()); + CopyCommand cmd = new CopyCommand(srcData.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(volObj.getTO()), _createVolumeFromSnapshotWait, VirtualMachineManager.ExecuteInSequence.value()); Answer answer = null; if (ep == null) { String errMsg = "No remote endpoint to send command, check if host or ssvm is down?"; @@ -301,7 +305,7 @@ protected Answer copyVolumeFromSnapshot(DataObject snapObj, DataObject volObj) { } protected Answer cloneVolume(DataObject template, DataObject volume) { - CopyCommand cmd = new CopyCommand(template.getTO(), addFullCloneFlagOnVMwareDest(volume.getTO()), 0, VirtualMachineManager.ExecuteInSequence.value()); + CopyCommand cmd = new CopyCommand(template.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(volume.getTO()), 0, VirtualMachineManager.ExecuteInSequence.value()); try { EndPoint ep = selector.select(volume.getDataStore()); Answer answer = null; @@ -373,7 +377,7 @@ protected Answer copyVolumeBetweenPools(DataObject srcData, DataObject destData) objOnImageStore.processEvent(Event.CopyingRequested); - CopyCommand cmd = new CopyCommand(objOnImageStore.getTO(), addFullCloneFlagOnVMwareDest(destData.getTO()), _copyvolumewait, VirtualMachineManager.ExecuteInSequence.value()); + CopyCommand cmd = new CopyCommand(objOnImageStore.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(destData.getTO()), _copyvolumewait, VirtualMachineManager.ExecuteInSequence.value()); EndPoint ep = selector.select(objOnImageStore, destData); if (ep == null) { String errMsg = "No remote endpoint to send command, check if host or ssvm is down?"; @@ -526,7 +530,7 @@ protected Answer createTemplateFromSnapshot(DataObject srcData, DataObject destD ep = selector.select(srcData, destData); } - CopyCommand cmd = new CopyCommand(srcData.getTO(), addFullCloneFlagOnVMwareDest(destData.getTO()), _createprivatetemplatefromsnapshotwait, VirtualMachineManager.ExecuteInSequence.value()); + CopyCommand cmd = new CopyCommand(srcData.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(destData.getTO()), _createprivatetemplatefromsnapshotwait, VirtualMachineManager.ExecuteInSequence.value()); Answer answer = null; if (ep == null) { String errMsg = "No remote endpoint to send command, check if host or ssvm is down?"; @@ -562,7 +566,7 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) { Scope selectedScope = pickCacheScopeForCopy(srcData, destData); cacheData = cacheMgr.getCacheObject(srcData, selectedScope); - CopyCommand cmd = new CopyCommand(srcData.getTO(), addFullCloneFlagOnVMwareDest(destData.getTO()), _backupsnapshotwait, VirtualMachineManager.ExecuteInSequence.value()); + CopyCommand cmd = new CopyCommand(srcData.getTO(), addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(destData.getTO()), _backupsnapshotwait, VirtualMachineManager.ExecuteInSequence.value()); cmd.setCacheTO(cacheData.getTO()); cmd.setOptions(options); EndPoint ep = selector.select(srcData, destData); @@ -574,7 +578,7 @@ protected Answer copySnapshot(DataObject srcData, DataObject destData) { answer = ep.sendMessage(cmd); } } else { - addFullCloneFlagOnVMwareDest(destData.getTO()); + addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(destData.getTO()); CopyCommand cmd = new CopyCommand(srcData.getTO(), destData.getTO(), _backupsnapshotwait, VirtualMachineManager.ExecuteInSequence.value()); cmd.setOptions(options); EndPoint ep = selector.select(srcData, destData, StorageAction.BACKUPSNAPSHOT); diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java index dccb6b445e56..969f58cd2bb8 100755 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java @@ -86,7 +86,7 @@ private void replaceVmwareCreateCloneFullField() throws Exception { @Test public void testAddFullCloneFlagOnVMwareDest(){ - strategy.addFullCloneFlagOnVMwareDest(dataTO); + strategy.addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(dataTO); verify(dataStoreTO).setFullCloneFlag(FULL_CLONE_FLAG); } diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index d29d1666df48..f99f22b3d926 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -63,7 +63,6 @@ public abstract class AbstractStoragePoolAllocator extends AdapterBase implement protected BigDecimal storageOverprovisioningFactor = new BigDecimal(1); protected String allocationAlgorithm = "random"; protected long extraBytesPerVolume = 0; - protected boolean diskProvisioningTypeStrictness = false; @Inject protected DataStoreManager dataStoreMgr; @Inject protected PrimaryDataStoreDao storagePoolDao; @Inject protected VolumeDao volumeDao; @@ -86,10 +85,6 @@ public boolean configure(String name, Map params) throws Configu if (allocationAlgorithm != null) { this.allocationAlgorithm = allocationAlgorithm; } - String strictness = configs.get("disk.provisioning.type.strictness"); - if (strictness != null) { - diskProvisioningTypeStrictness = strictness.equals("true"); - } return true; } return false; @@ -220,11 +215,8 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh, return false; } - if (diskProvisioningTypeStrictness){ - StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported"); - if (!dskCh.getProvisioningType().equals("thin") && hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true")){ - return false; - } + if (!checkDiskProvisioningSupport(dskCh, pool)) { + return false; } if(!checkHypervisorCompatibility(dskCh.getHypervisorType(), dskCh.getType(), pool.getPoolType())){ @@ -269,6 +261,18 @@ protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh, return storageMgr.storagePoolHasEnoughIops(requestVolumes, pool) && storageMgr.storagePoolHasEnoughSpace(requestVolumes, pool, plan.getClusterId()); } + private boolean checkDiskProvisioningSupport(DiskProfile dskCh, StoragePool pool) { + if (dskCh.getHypervisorType() != null && dskCh.getHypervisorType() == HypervisorType.VMware && pool.getPoolType() == Storage.StoragePoolType.NetworkFilesystem && + storageMgr.DiskProvisioningStrictness.valueIn(pool.getDataCenterId())) { + StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + if (!dskCh.getProvisioningType().toString().equals("thin") && + (hardwareAcceleration == null || (hardwareAcceleration.getValue() != null && !hardwareAcceleration.getValue().equals("true")))) { + return false; + } + } + return true; + } + /* Check StoragePool and Volume type compatibility for the hypervisor */ diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index 534f99267324..54e22f40ce19 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -26,10 +26,13 @@ import com.cloud.exception.StorageConflictException; import com.cloud.storage.DataStoreRole; import com.cloud.storage.Storage; +import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; +import com.cloud.storage.StoragePoolStatus; +import com.cloud.storage.StorageService; import com.cloud.storage.dao.StoragePoolHostDao; -import com.cloud.storage.StorageManager; +import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener; @@ -59,6 +62,9 @@ public class DefaultHostListener implements HypervisorHostListener { StoragePoolDetailsDao storagePoolDetailsDao; @Inject StorageManager storageManager; + StoragePoolTagsDao storagePoolTagsDao; + @Inject + StorageService storageService; @Override public boolean hostAdded(long hostId) { @@ -67,7 +73,7 @@ public boolean hostAdded(long hostId) { @Override public boolean hostConnect(long hostId, long poolId) throws StorageConflictException { - StoragePool pool = (StoragePool)this.dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary); + StoragePool pool = (StoragePool) this.dataStoreMgr.getDataStore(poolId, DataStoreRole.Primary); ModifyStoragePoolCommand cmd = new ModifyStoragePoolCommand(true, pool); final Answer answer = agentMgr.easySend(hostId, cmd); @@ -84,7 +90,7 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep assert (answer instanceof ModifyStoragePoolAnswer) : "Well, now why won't you actually return the ModifyStoragePoolAnswer when it's ModifyStoragePoolCommand? Pool=" + pool.getId() + "Host=" + hostId; - ModifyStoragePoolAnswer mspAnswer = (ModifyStoragePoolAnswer)answer; + ModifyStoragePoolAnswer mspAnswer = (ModifyStoragePoolAnswer) answer; if (mspAnswer.getLocalDatastoreName() != null && pool.isShared()) { String datastoreName = mspAnswer.getLocalDatastoreName(); List localStoragePools = this.primaryStoreDao.listLocalStoragePoolByPath(pool.getDataCenterId(), datastoreName); @@ -103,6 +109,8 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep storageManager.syncDatastoreClusterStoragePool(poolId, ((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren(), hostId); } + storageService.updateStorageCapabilities(poolId); + s_logger.info("Connection established between storage pool " + pool + " and host " + hostId); return true; } @@ -126,13 +134,6 @@ private void updateStoragePoolHostVOAndDetails(StoragePool pool, long hostId, Mo storagePoolDetailsDao.persist(storagePoolDetailVO); } } - if (mspAnswer.getPoolInfo().getDetails() != null && mspAnswer.getPoolInfo().getDetails().containsKey("hardwareAccelerationSupported")){ - StoragePoolDetailVO hardwareAccelerationSupported = storagePoolDetailsDao.findDetail(pool.getId(), "hardwareAccelerationSupported"); - if (hardwareAccelerationSupported == null) { - StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), "hardwareAccelerationSupported", mspAnswer.getPoolInfo().getDetails().get("hardwareAccelerationSupported"), false); - storagePoolDetailsDao.persist(storagePoolDetailVO); - } - } primaryStoreDao.update(pool.getId(), poolVO); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 2979e26594ca..45189fa0c881 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -48,6 +48,11 @@ import javax.naming.ConfigurationException; import javax.xml.datatype.XMLGregorianCalendar; +import com.cloud.agent.api.GetStoragePoolCapabilitiesAnswer; +import com.cloud.agent.api.GetStoragePoolCapabilitiesCommand; +import com.cloud.agent.api.to.DataTO; +import com.cloud.agent.api.to.DeployAsIsInfoTO; +import com.cloud.agent.api.ValidateVcenterDetailsCommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.storage.command.CopyCommand; import org.apache.cloudstack.storage.command.StorageSubSystemCommand; @@ -507,6 +512,8 @@ public Answer executeRequest(Command cmd) { answer = execute((ModifyTargetsCommand) cmd); } else if (clz == ModifyStoragePoolCommand.class) { answer = execute((ModifyStoragePoolCommand) cmd); + } else if (clz == GetStoragePoolCapabilitiesCommand.class) { + answer = execute((GetStoragePoolCapabilitiesCommand) cmd); } else if (clz == DeleteStoragePoolCommand.class) { answer = execute((DeleteStoragePoolCommand) cmd); } else if (clz == CopyVolumeCommand.class) { @@ -696,6 +703,9 @@ protected EnumMap examineStora if (dest.isFullCloneFlag() != null) { paramsCopy.put(VmwareStorageProcessorConfigurableFields.FULL_CLONE_FLAG, dest.isFullCloneFlag().booleanValue()); } + if (dest.getDiskProvisioningStrictnessFlag() != null) { + paramsCopy.put(VmwareStorageProcessorConfigurableFields.DISK_PROVISIONING_STRICTNESS, dest.getDiskProvisioningStrictnessFlag().booleanValue()); + } } } return paramsCopy; @@ -5031,15 +5041,54 @@ protected Answer execute(ModifyStoragePoolCommand cmd) { answer.setLocalDatastoreName(morDatastore.getValue()); } + return answer; + } catch (Throwable e) { + if (e instanceof RemoteException) { + s_logger.warn("Encounter remote exception to vCenter, invalidate VMware session context"); + + invalidateServiceContext(); + } + + String msg = "ModifyStoragePoolCommand failed due to " + VmwareHelper.getExceptionMessage(e); + + s_logger.error(msg, e); + + return new Answer(cmd, false, msg); + } + } + + protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) { + + try { + + VmwareHypervisorHost hyperHost = getHyperHost(getServiceContext()); + HostMO host = (HostMO) hyperHost; - boolean hardwareAccelerationSupportForDataStore = getHardwareAcceleationSupportForDataStore(host.getMor(), dsMo.getName()); - StoragePoolInfo poolInfo = answer.getPoolInfo(); - Map poolDetails = poolInfo.getDetails(); - if (poolDetails == null) { - poolDetails = new HashMap<>(); + + StorageFilerTO pool = cmd.getPool(); + + ManagedObjectReference morDatastore = HypervisorHostHelper.findDatastoreWithBackwardsCompatibility(hyperHost, pool.getUuid()); + + if (morDatastore == null) { + morDatastore = hyperHost.mountDatastore((pool.getType() == StoragePoolType.VMFS || pool.getType() == StoragePoolType.PreSetup || pool.getType() == StoragePoolType.DatastoreCluster), pool.getHost(), pool.getPort(), pool.getPath(), pool.getUuid().replace("-", ""), true); + } + + assert (morDatastore != null); + + DatastoreMO dsMo = new DatastoreMO(getServiceContext(), morDatastore); + + GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd); + + if (pool.getType() == StoragePoolType.NetworkFilesystem) { + boolean hardwareAccelerationSupportForDataStore = getHardwareAccelerationSupportForDataStore(host.getMor(), dsMo.getName()); + StoragePoolInfo poolInfo = answer.getPoolInfo(); + Map poolDetails = poolInfo.getDetails(); + if (poolDetails == null) { + poolDetails = new HashMap<>(); + } + poolDetails.put(Storage.Capability.HARDWARE_ACCELERATION.toString(), String.valueOf(hardwareAccelerationSupportForDataStore)); + poolInfo.setDetails(poolDetails); } - poolDetails.put("hardwareAccelerationSupported", String.valueOf(hardwareAccelerationSupportForDataStore)); - poolInfo.setDetails(poolDetails); return answer; } catch (Throwable e) { @@ -5049,7 +5098,7 @@ protected Answer execute(ModifyStoragePoolCommand cmd) { invalidateServiceContext(); } - String msg = "ModifyStoragePoolCommand failed due to " + VmwareHelper.getExceptionMessage(e); + String msg = "GetStoragePoolCapabilitiesCommand failed due to " + VmwareHelper.getExceptionMessage(e); s_logger.error(msg, e); @@ -5057,7 +5106,7 @@ protected Answer execute(ModifyStoragePoolCommand cmd) { } } - private boolean getHardwareAcceleationSupportForDataStore(ManagedObjectReference host, String dataStoreName) throws Exception { + private boolean getHardwareAccelerationSupportForDataStore(ManagedObjectReference host, String dataStoreName) throws Exception { HostConfigInfo config = getServiceContext().getVimClient().getDynamicProperty(host, "config"); List mountInfoList = config.getFileSystemVolume().getMountInfo(); for (HostFileSystemMountInfo hostFileSystemMountInfo: mountInfoList) { diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 90adcdfa02b8..b6759de39c39 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -138,7 +138,7 @@ public class VmwareStorageProcessor implements StorageProcessor { public enum VmwareStorageProcessorConfigurableFields { - NFS_VERSION("nfsVersion"), FULL_CLONE_FLAG("fullCloneFlag"); + NFS_VERSION("nfsVersion"), FULL_CLONE_FLAG("fullCloneFlag"), DISK_PROVISIONING_STRICTNESS("diskProvisioningStrictness"); private String name; @@ -157,6 +157,7 @@ public String getName() { private final VmwareHostService hostService; private boolean _fullCloneFlag; + private boolean _diskProvisioningStrictness; private final VmwareStorageMount mountService; private final VmwareResource resource; private final Integer _timeout; @@ -916,9 +917,12 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper if (volume.getVolumeType() == Volume.Type.DATADISK) vmName = volume.getName(); if (!_fullCloneFlag) { + if (_diskProvisioningStrictness) { + throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled"); + } createVMLinkedClone(vmTemplate, dcMo, vmName, morDatastore, morPool); } else { - createVMFullClone(vmTemplate, dcMo, dsMo, vmName, morDatastore, morPool, null); + createVMFullClone(vmTemplate, dcMo, dsMo, vmName, morDatastore, morPool, volume.getProvisioningType()); } VirtualMachineMO vmMo = new ClusterMO(context, morCluster).findVmOnHyperHost(vmName); @@ -944,6 +948,9 @@ private String createVMAndFolderWithVMName(VmwareContext context, VmwareHypervis _fullCloneFlag = volume.getSize() > template.getSize() ? true : _fullCloneFlag; } if (!_fullCloneFlag) { + if (_diskProvisioningStrictness) { + throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled"); + } createVMLinkedClone(vmTemplate, dcMo, vmdkName, morDatastore, morPool); } else { createVMFullClone(vmTemplate, dcMo, dsMo, vmdkName, morDatastore, morPool, volume.getProvisioningType()); @@ -1000,9 +1007,12 @@ private void createLinkedOrFullClone(TemplateObjectTO template, VolumeObjectTO v _fullCloneFlag = volume.getSize() > template.getSize() || _fullCloneFlag; } if (!_fullCloneFlag) { + if (_diskProvisioningStrictness) { + throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled"); + } createVMLinkedClone(vmMo, dcMo, cloneName, morDatastore, morPool); } else { - createVMFullClone(vmMo, dcMo, dsMo, cloneName, morDatastore, morPool, null); + createVMFullClone(vmMo, dcMo, dsMo, cloneName, morDatastore, morPool, volume.getProvisioningType()); } } @@ -3890,6 +3900,11 @@ void setFullCloneFlag(boolean value){ s_logger.debug("VmwareProcessor instance - create full clone = " + (value ? "TRUE" : "FALSE")); } + void setDiskProvisioningStricteness(boolean value){ + this._diskProvisioningStrictness = value; + s_logger.debug("VmwareProcessor instance - diskProvisioningStrictness = " + (value ? "TRUE" : "FALSE")); + } + @Override public Answer handleDownloadTemplateToPrimaryStorage(DirectDownloadCommand cmd) { return null; @@ -3951,7 +3966,7 @@ public VirtualMachineMO cloneVMFromTemplate(VmwareHypervisorHost hyperHost, Stri } s_logger.info("Cloning VM " + cloneName + " from template " + templateName + " into datastore " + templatePrimaryStoreUuid); if (!_fullCloneFlag) { - createVMLinkedClone(templateMo, dcMo, cloneName, morDatastore, morPool); + createVMLinkedClone(templateMo, dcMo, cloneName, morDatastore, morPool, null); } else { createVMFullClone(templateMo, dcMo, dsMo, cloneName, morDatastore, morPool, null); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java index 122a034288d4..2145ac0a9bb6 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java @@ -83,6 +83,10 @@ public boolean reconfigureStorageProcessor(EnumMap hosts = _storagePoolHostDao.listByPoolId(poolId); if (hosts.size() > 0) { StoragePoolHostVO host = hosts.get(0); - _agentMgr.easySend(host.getHostId(), new ModifyStoragePoolCommand(false, pool)); + GetStoragePoolCapabilitiesCommand cmd = new GetStoragePoolCapabilitiesCommand(); + cmd.setPool(new StorageFilerTO(pool)); + GetStoragePoolCapabilitiesAnswer answer = (GetStoragePoolCapabilitiesAnswer) _agentMgr.easySend(host.getHostId(), cmd); + if (answer.getPoolInfo().getDetails() != null && answer.getPoolInfo().getDetails().containsKey(Storage.Capability.HARDWARE_ACCELERATION.toString())){ + StoragePoolDetailVO hardwareAccelerationSupported = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + + if (hardwareAccelerationSupported == null) { + StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString(), answer.getPoolInfo().getDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString()), false); + _storagePoolDetailsDao.persist(storagePoolDetailVO); + } else { + hardwareAccelerationSupported.setValue(answer.getPoolInfo().getDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString())); + _storagePoolDetailsDao.update(hardwareAccelerationSupported.getId(), hardwareAccelerationSupported); + } + } } } @@ -3147,7 +3163,8 @@ public ConfigKey[] getConfigKeys() { STORAGE_POOL_CLIENT_MAX_CONNECTIONS, PRIMARY_STORAGE_DOWNLOAD_WAIT, SecStorageMaxMigrateSessions, - MaxDataMigrationWaitTime + MaxDataMigrationWaitTime, + DiskProvisioningStrictness }; } diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java index ca05ea9e9a52..04fe65dc9866 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/VirtualMachineMO.java @@ -790,7 +790,7 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde relocSpec.setDatastore(morDs); relocSpec.setPool(morResourcePool); - setDiskProvsioningType(relocSpec, morDs, diskProvisioningType); + setDiskProvisioningType(relocSpec, morDs, diskProvisioningType); ManagedObjectReference morTask = _context.getService().cloneVMTask(_mor, morFolder, cloneName, cloneSpec); @@ -805,8 +805,8 @@ public boolean createFullClone(String cloneName, ManagedObjectReference morFolde return false; } - private void setDiskProvsioningType(VirtualMachineRelocateSpec relocSpec, ManagedObjectReference morDs, - Storage.ProvisioningType diskProvisioningType) throws Exception { + private void setDiskProvisioningType(VirtualMachineRelocateSpec relocSpec, ManagedObjectReference morDs, + Storage.ProvisioningType diskProvisioningType) throws Exception { if (diskProvisioningType == null){ return; } From 362e45c33610cf04f0b890542de32a1590b91396 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 8 Feb 2021 14:50:15 +0200 Subject: [PATCH 03/33] Fixed unit test --- .../cloud/hypervisor/vmware/resource/VmwareResourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java index 3dc8d011289a..7a2a45d2f287 100644 --- a/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java +++ b/plugins/hypervisors/vmware/src/test/java/com/cloud/hypervisor/vmware/resource/VmwareResourceTest.java @@ -363,7 +363,7 @@ public void testExamineStorageSubSystemCommandFullCloneFlagForVmware(){ EnumMap params2 = _resource.examineStorageSubSystemCommandFullCloneFlagForVmware(storageCmd, params); verify(destDataTO).getDataStore(); verify(destDataStoreTO, times(2)).isFullCloneFlag(); - assertEquals(1, params2.size()); + assertEquals(2, params2.size()); assertEquals(FULL_CLONE_FLAG, params2.get(VmwareStorageProcessorConfigurableFields.FULL_CLONE_FLAG)); } From e4b7f039605f775d40a507a1409ead37e71cd335 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Tue, 9 Feb 2021 12:19:58 +0200 Subject: [PATCH 04/33] Review changes --- .../com/cloud/agent/api/StoragePoolInfo.java | 3 -- .../com/cloud/storage/StorageService.java | 2 +- .../storage/UpdateStorageCapabilitiesCmd.java | 2 +- .../api/GetStoragePoolCapabilitiesAnswer.java | 18 ++++--- .../com/cloud/storage/StorageManager.java | 5 +- .../AbstractStoragePoolAllocator.java | 2 +- .../vmware/resource/VmwareResource.java | 14 ++---- .../resource/VmwareStorageProcessor.java | 8 ++-- .../VmwareStorageSubsystemCommandHandler.java | 2 +- ...oudStackPrimaryDataStoreLifeCycleImpl.java | 6 +-- .../com/cloud/storage/StorageManagerImpl.java | 48 ++++++++++++------- test/__init__.py | 0 .../smoke/test_disk_provisioning_types.py | 6 ++- 13 files changed, 63 insertions(+), 53 deletions(-) create mode 100644 test/__init__.py diff --git a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java index 10cc77fd6ed7..a8dadf4f4709 100644 --- a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java +++ b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java @@ -108,7 +108,4 @@ public Map getDetails() { return details; } - public void setDetails(Map details) { - this.details = details; - } } diff --git a/api/src/main/java/com/cloud/storage/StorageService.java b/api/src/main/java/com/cloud/storage/StorageService.java index 65a09476cd24..b9833269b2a2 100644 --- a/api/src/main/java/com/cloud/storage/StorageService.java +++ b/api/src/main/java/com/cloud/storage/StorageService.java @@ -105,6 +105,6 @@ public interface StorageService { ImageStore updateImageStoreStatus(Long id, Boolean readonly); - void updateStorageCapabilities(long poolId); + void updateStorageCapabilities(Long poolId); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java index a8d0c4546f0b..b9293e08515f 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -40,7 +40,7 @@ public class UpdateStorageCapabilitiesCmd extends BaseCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// - @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = true, description = "Storage pool id") + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = false, description = "Storage pool id") private Long poolId; ///////////////////////////////////////////////////// diff --git a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java index bf1ae64e23db..b69050f4dd1c 100644 --- a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java @@ -1,19 +1,23 @@ package com.cloud.agent.api; +import java.util.HashMap; +import java.util.Map; + public class GetStoragePoolCapabilitiesAnswer extends Answer { + + private Map poolDetails; + public GetStoragePoolCapabilitiesAnswer(GetStoragePoolCapabilitiesCommand cmd) { super(cmd); - - poolInfo = new StoragePoolInfo(); + poolDetails = new HashMap<>(); } - public StoragePoolInfo getPoolInfo() { - return poolInfo; + public Map getPoolDetails() { + return poolDetails; } - public void setPoolInfo(StoragePoolInfo poolInfo) { - this.poolInfo = poolInfo; + public void setPoolDetails(Map poolDetails) { + this.poolDetails = poolDetails; } - private StoragePoolInfo poolInfo; } diff --git a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java index 47917b8c51bb..b91003fa5227 100644 --- a/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java +++ b/engine/components-api/src/main/java/com/cloud/storage/StorageManager.java @@ -145,7 +145,10 @@ public interface StorageManager extends StorageService { ConfigKey MaxDataMigrationWaitTime = new ConfigKey("Advanced", Integer.class, "max.data.migration.wait.time", "15", "Maximum wait time for a data migration task before spawning a new SSVM", false, ConfigKey.Scope.Global); - ConfigKey DiskProvisioningStrictness = new ConfigKey("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings if set to true.", true, ConfigKey.Scope.Zone); + ConfigKey DiskProvisioningStrictness = new ConfigKey("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", + "If set to true, the disk is created only when there is a suitable storage pool that supports the disk provisioning type specified by the service/disk offering. " + + "If set to false, the disk is created with a disk provisioning type supported by the pool. Default value is false, and this is currently supported for VMware only.", + true, ConfigKey.Scope.Zone); /** * Returns a comma separated list of tags for the specified storage pool * @param poolId diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index f99f22b3d926..93c8c79f952e 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -265,7 +265,7 @@ private boolean checkDiskProvisioningSupport(DiskProfile dskCh, StoragePool pool if (dskCh.getHypervisorType() != null && dskCh.getHypervisorType() == HypervisorType.VMware && pool.getPoolType() == Storage.StoragePoolType.NetworkFilesystem && storageMgr.DiskProvisioningStrictness.valueIn(pool.getDataCenterId())) { StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); - if (!dskCh.getProvisioningType().toString().equals("thin") && + if (!dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) && (hardwareAcceleration == null || (hardwareAcceleration.getValue() != null && !hardwareAcceleration.getValue().equals("true")))) { return false; } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 45189fa0c881..1080ff5ef36d 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -5079,16 +5079,10 @@ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) { GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd); - if (pool.getType() == StoragePoolType.NetworkFilesystem) { - boolean hardwareAccelerationSupportForDataStore = getHardwareAccelerationSupportForDataStore(host.getMor(), dsMo.getName()); - StoragePoolInfo poolInfo = answer.getPoolInfo(); - Map poolDetails = poolInfo.getDetails(); - if (poolDetails == null) { - poolDetails = new HashMap<>(); - } - poolDetails.put(Storage.Capability.HARDWARE_ACCELERATION.toString(), String.valueOf(hardwareAccelerationSupportForDataStore)); - poolInfo.setDetails(poolDetails); - } + boolean hardwareAccelerationSupportForDataStore = getHardwareAccelerationSupportForDataStore(host.getMor(), dsMo.getName()); + Map poolDetails = answer.getPoolDetails(); + poolDetails.put(Storage.Capability.HARDWARE_ACCELERATION.toString(), String.valueOf(hardwareAccelerationSupportForDataStore)); + answer.setPoolDetails(poolDetails); return answer; } catch (Throwable e) { diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index b6759de39c39..20f7902060df 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -917,7 +917,7 @@ private String cloneVMforVvols(VmwareContext context, VmwareHypervisorHost hyper if (volume.getVolumeType() == Volume.Type.DATADISK) vmName = volume.getName(); if (!_fullCloneFlag) { - if (_diskProvisioningStrictness) { + if (_diskProvisioningStrictness && volume.getProvisioningType() != ProvisioningType.THIN) { throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled"); } createVMLinkedClone(vmTemplate, dcMo, vmName, morDatastore, morPool); @@ -948,7 +948,7 @@ private String createVMAndFolderWithVMName(VmwareContext context, VmwareHypervis _fullCloneFlag = volume.getSize() > template.getSize() ? true : _fullCloneFlag; } if (!_fullCloneFlag) { - if (_diskProvisioningStrictness) { + if (_diskProvisioningStrictness && volume.getProvisioningType() != ProvisioningType.THIN) { throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled"); } createVMLinkedClone(vmTemplate, dcMo, vmdkName, morDatastore, morPool); @@ -1007,7 +1007,7 @@ private void createLinkedOrFullClone(TemplateObjectTO template, VolumeObjectTO v _fullCloneFlag = volume.getSize() > template.getSize() || _fullCloneFlag; } if (!_fullCloneFlag) { - if (_diskProvisioningStrictness) { + if (_diskProvisioningStrictness && volume.getProvisioningType() != ProvisioningType.THIN) { throw new CloudRuntimeException("Unable to create linked clones with strict disk provisioning enabled"); } createVMLinkedClone(vmMo, dcMo, cloneName, morDatastore, morPool); @@ -3900,7 +3900,7 @@ void setFullCloneFlag(boolean value){ s_logger.debug("VmwareProcessor instance - create full clone = " + (value ? "TRUE" : "FALSE")); } - void setDiskProvisioningStricteness(boolean value){ + void setDiskProvisioningStrictness(boolean value){ this._diskProvisioningStrictness = value; s_logger.debug("VmwareProcessor instance - diskProvisioningStrictness = " + (value ? "TRUE" : "FALSE")); } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java index 2145ac0a9bb6..15caa1d878ea 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageSubsystemCommandHandler.java @@ -85,7 +85,7 @@ public boolean reconfigureStorageProcessor(EnumMap hosts = _storagePoolHostDao.listByPoolId(poolId); - if (hosts.size() > 0) { - StoragePoolHostVO host = hosts.get(0); - GetStoragePoolCapabilitiesCommand cmd = new GetStoragePoolCapabilitiesCommand(); - cmd.setPool(new StorageFilerTO(pool)); - GetStoragePoolCapabilitiesAnswer answer = (GetStoragePoolCapabilitiesAnswer) _agentMgr.easySend(host.getHostId(), cmd); - if (answer.getPoolInfo().getDetails() != null && answer.getPoolInfo().getDetails().containsKey(Storage.Capability.HARDWARE_ACCELERATION.toString())){ - StoragePoolDetailVO hardwareAccelerationSupported = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); - - if (hardwareAccelerationSupported == null) { - StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString(), answer.getPoolInfo().getDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString()), false); - _storagePoolDetailsDao.persist(storagePoolDetailVO); - } else { - hardwareAccelerationSupported.setValue(answer.getPoolInfo().getDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString())); - _storagePoolDetailsDao.update(hardwareAccelerationSupported.getId(), hardwareAccelerationSupported); + public void updateStorageCapabilities(Long poolId) { + List pools = new ArrayList<>(); + if (poolId == null) { + pools = _storagePoolDao.listAll(); + } else { + pools.add(_storagePoolDao.findById(poolId)); + } + + if (pools.size() == 0) { + throw new CloudRuntimeException("No storage pools found to update."); + } + + for (StoragePoolVO pool: pools) { + // find the host + List poolIds = new ArrayList(); + poolIds.add(poolId); + List hosts = _storagePoolHostDao.findHostsConnectedToPools(poolIds); + if (hosts.size() > 0) { + GetStoragePoolCapabilitiesCommand cmd = new GetStoragePoolCapabilitiesCommand(); + cmd.setPool(new StorageFilerTO(pool)); + GetStoragePoolCapabilitiesAnswer answer = (GetStoragePoolCapabilitiesAnswer) _agentMgr.easySend(hosts.get(0), cmd); + if (answer.getPoolDetails() != null && answer.getPoolDetails().containsKey(Storage.Capability.HARDWARE_ACCELERATION.toString())) { + StoragePoolDetailVO hardwareAccelerationSupported = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + if (hardwareAccelerationSupported == null) { + StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString(), answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString()), false); + _storagePoolDetailsDao.persist(storagePoolDetailVO); + } else { + hardwareAccelerationSupported.setValue(answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString())); + _storagePoolDetailsDao.update(hardwareAccelerationSupported.getId(), hardwareAccelerationSupported); + } } } } diff --git a/test/__init__.py b/test/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/test/integration/smoke/test_disk_provisioning_types.py b/test/integration/smoke/test_disk_provisioning_types.py index f6f290e6d7fb..8fd16afd4f62 100644 --- a/test/integration/smoke/test_disk_provisioning_types.py +++ b/test/integration/smoke/test_disk_provisioning_types.py @@ -15,7 +15,7 @@ # specific language governing permissions and limitations # under the License. -from marvin.cloudstackTestCase import cloudstackTestCase +from marvin.cloudstackTestCase import cloudstackTestCase, unittest from marvin.lib.utils import cleanup_resources from marvin.lib.base import DiskOffering, Iso, Account, VirtualMachine, ServiceOffering, Volume from marvin.codes import FAILED @@ -27,6 +27,10 @@ class TestDiskProvisioningTypes(cloudstackTestCase): def setUp(self): + + if self.testClient.getHypervisorInfo().lower() != "vmware": + raise unittest.SkipTest("VMWare tests only valid on VMWare hypervisor") + self.services = self.testClient.getParsedTestDataConfig() self.apiclient = self.testClient.getApiClient() self.dbclient = self.testClient.getDbConnection() From 6c14400b9efc7120e286d3f0582baa822436da8d Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Wed, 10 Feb 2021 09:17:51 +0200 Subject: [PATCH 05/33] Added missing licenses --- .../api/GetStoragePoolCapabilitiesAnswer.java | 16 ++++++++++++++++ .../api/GetStoragePoolCapabilitiesCommand.java | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java index b69050f4dd1c..0e638067d1f8 100644 --- a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java @@ -1,3 +1,19 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. package com.cloud.agent.api; import java.util.HashMap; diff --git a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java index 7da8d75154b0..b7dd731df3eb 100644 --- a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java +++ b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesCommand.java @@ -1,3 +1,19 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. package com.cloud.agent.api; import com.cloud.agent.api.to.StorageFilerTO; From 299c3deb44da2b6ebc378a073244a4f8a01a11ff Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Thu, 11 Feb 2021 10:59:54 +0200 Subject: [PATCH 06/33] Review changes --- .../com/cloud/storage/StorageService.java | 2 +- .../storage/UpdateStorageCapabilitiesCmd.java | 19 ++++++-- .../AbstractStoragePoolAllocator.java | 2 +- .../provider/DefaultHostListener.java | 2 +- ...oudStackPrimaryDataStoreLifeCycleImpl.java | 6 +-- .../com/cloud/storage/StorageManagerImpl.java | 46 +++++++++++++++++-- tools/apidoc/gen_toc.py | 2 +- 7 files changed, 62 insertions(+), 17 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/StorageService.java b/api/src/main/java/com/cloud/storage/StorageService.java index b9833269b2a2..2597506b651b 100644 --- a/api/src/main/java/com/cloud/storage/StorageService.java +++ b/api/src/main/java/com/cloud/storage/StorageService.java @@ -105,6 +105,6 @@ public interface StorageService { ImageStore updateImageStoreStatus(Long id, Boolean readonly); - void updateStorageCapabilities(Long poolId); + void updateStorageCapabilities(Long poolId, boolean failOnChecks); } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java index b9293e08515f..ca9adf988d5d 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -29,12 +29,14 @@ import org.apache.cloudstack.api.response.StoragePoolResponse; import org.apache.cloudstack.api.response.SuccessResponse; import org.apache.cloudstack.context.CallContext; +import org.apache.log4j.Logger; @APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs capabilities of storage pools", responseObject = SuccessResponse.class, - requestHasSensitiveInfo = false, responseHasSensitiveInfo = false) + requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0") public class UpdateStorageCapabilitiesCmd extends BaseCmd { public static final String APINAME = "updateStorageCapabilities"; + private static final Logger LOG = Logger.getLogger(UpdateImageStoreCmd.class.getName()); ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// @@ -62,10 +64,17 @@ public void setPoolId(Long poolId) { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { - _storageService.updateStorageCapabilities(poolId); - SuccessResponse response = new SuccessResponse(getCommandName()); - response.setSuccess(true); - this.setResponseObject(response); + try { + _storageService.updateStorageCapabilities(poolId, poolId != null); + SuccessResponse response = new SuccessResponse(getCommandName()); + response.setSuccess(true); + this.setResponseObject(response); + } catch (Exception e) { + SuccessResponse response = new SuccessResponse(getCommandName()); + response.setSuccess(false); + this.setResponseObject(response); + LOG.error(e); + } } @Override diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 93c8c79f952e..c7df5695a776 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -266,7 +266,7 @@ private boolean checkDiskProvisioningSupport(DiskProfile dskCh, StoragePool pool storageMgr.DiskProvisioningStrictness.valueIn(pool.getDataCenterId())) { StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); if (!dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) && - (hardwareAcceleration == null || (hardwareAcceleration.getValue() != null && !hardwareAcceleration.getValue().equals("true")))) { + (hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true"))) { return false; } } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index 54e22f40ce19..1104b62f10e3 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -109,7 +109,7 @@ public boolean hostConnect(long hostId, long poolId) throws StorageConflictExcep storageManager.syncDatastoreClusterStoragePool(poolId, ((ModifyStoragePoolAnswer) answer).getDatastoreClusterChildren(), hostId); } - storageService.updateStorageCapabilities(poolId); + storageService.updateStorageCapabilities(poolId, false); s_logger.info("Connection established between storage pool " + pool + " and host " + hostId); return true; diff --git a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java index 92a2a587e7b8..f39b17076e7a 100644 --- a/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java +++ b/plugins/storage/volume/default/src/main/java/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackPrimaryDataStoreLifeCycleImpl.java @@ -359,6 +359,7 @@ public DataStore initialize(Map dsInfos) { parameters.setName(poolName); parameters.setClusterId(clusterId); parameters.setProviderName(providerName); + parameters.setHypervisorType(hypervisorType); return dataStoreHelper.createPrimaryDataStore(parameters); } @@ -404,10 +405,7 @@ protected boolean createStoragePool(long hostId, StoragePool pool) { CreateStoragePoolCommand cmd = new CreateStoragePoolCommand(true, pool); final Answer answer = agentMgr.easySend(hostId, cmd); if (answer != null && answer.getResult()) { - HypervisorType hyperVisorType = getHypervisorType(hostId); - if (pool.getPoolType() == StoragePoolType.NetworkFilesystem && hyperVisorType == HypervisorType.VMware) { - storageMgr.updateStorageCapabilities(pool.getId()); - } + storageMgr.updateStorageCapabilities(pool.getId(), false); return true; } else { primaryDataStoreDao.expunge(pool.getId()); diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 936bfcaee01b..06ad1e3761d0 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2781,12 +2781,18 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) { } @Override - public void updateStorageCapabilities(Long poolId) { + public void updateStorageCapabilities(Long poolId, boolean failOnChecks) { List pools = new ArrayList<>(); if (poolId == null) { - pools = _storagePoolDao.listAll(); + pools = _storagePoolDao.listByStatus(StoragePoolStatus.Up); } else { - pools.add(_storagePoolDao.findById(poolId)); + StoragePoolVO pool = _storagePoolDao.findById(poolId); + + if (pool == null) { + throw new CloudRuntimeException("Primary storage not found for id: " + poolId); + } + + pools.add(pool); } if (pools.size() == 0) { @@ -2794,9 +2800,41 @@ public void updateStorageCapabilities(Long poolId) { } for (StoragePoolVO pool: pools) { + + // Only checking NFS for now - required for disk provisioning type support for vmware. + if (pool.getPoolType() != StoragePoolType.NetworkFilesystem) { + if (failOnChecks) { + throw new CloudRuntimeException("Storage capabilities update only supported on NFS storage mounted."); + } + continue; + } + + if (pool.getStatus() != StoragePoolStatus.Initialized && pool.getStatus() != StoragePoolStatus.Up) { + if (failOnChecks){ + throw new CloudRuntimeException("Primary storage is not in the right state to update capabilities"); + } + continue; + } + + HypervisorType hypervisor = pool.getHypervisor(); + + if (hypervisor == null){ + if (pool.getClusterId() != null) { + ClusterVO cluster = _clusterDao.findById(pool.getClusterId()); + hypervisor = cluster.getHypervisorType(); + } + } + + if (!HypervisorType.VMware.equals(hypervisor)) { + if (failOnChecks) { + throw new CloudRuntimeException("Storage capabilities update only supported on VMWare."); + } + continue; + } + // find the host List poolIds = new ArrayList(); - poolIds.add(poolId); + poolIds.add(pool.getId()); List hosts = _storagePoolHostDao.findHostsConnectedToPools(poolIds); if (hosts.size() > 0) { GetStoragePoolCapabilitiesCommand cmd = new GetStoragePoolCapabilitiesCommand(); diff --git a/tools/apidoc/gen_toc.py b/tools/apidoc/gen_toc.py index 8423539ea438..6d7841edb4b4 100644 --- a/tools/apidoc/gen_toc.py +++ b/tools/apidoc/gen_toc.py @@ -95,7 +95,7 @@ 'StorageMaintenance': 'Storage Pool', 'StoragePool': 'Storage Pool', 'StorageProvider': 'Storage Pool', - 'syncStorageCapabilites' : 'Storage Pool', + 'updateStorageCapabilities' : 'Storage Pool', 'SecurityGroup': 'Security Group', 'SSH': 'SSH', 'register': 'Registration', From 280cd676b7d8d74c9a1359784518d5dfc03cd0f2 Mon Sep 17 00:00:00 2001 From: Spaceman1984 <49917670+Spaceman1984@users.noreply.github.com> Date: Wed, 10 Feb 2021 10:25:35 +0200 Subject: [PATCH 07/33] Update StoragePoolInfo.java Removed white space --- api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java | 1 - 1 file changed, 1 deletion(-) diff --git a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java index a8dadf4f4709..d923694a854d 100644 --- a/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java +++ b/api/src/main/java/com/cloud/agent/api/StoragePoolInfo.java @@ -107,5 +107,4 @@ public void setUuid(String uuid) { public Map getDetails() { return details; } - } From 426d37cb6ca92706a83cf1263306296007822438 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Fri, 12 Feb 2021 10:26:10 +0200 Subject: [PATCH 08/33] Review change - Getting disk provisioning strictness setting using the zone id and not the pool id --- .../storage/motion/AncientDataMotionStrategy.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index e7775846c540..b438ecc24cbf 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -23,6 +23,7 @@ import javax.inject.Inject; +import com.cloud.dc.dao.DataCenterDao; import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; @@ -86,6 +87,9 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { @Inject StorageCacheManager cacheMgr; + @Inject + DataCenterDao dataCenterDao; + @Inject StorageManager storageManager; @@ -219,7 +223,8 @@ protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO if (dataStoreTO != null && dataStoreTO instanceof PrimaryDataStoreTO){ PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStoreTO; primaryDataStoreTO.setFullCloneFlag(CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId())); - primaryDataStoreTO.setDiskProvisioningStrictnessFlag(storageManager.DiskProvisioningStrictness.valueIn(primaryDataStoreTO.getId())); + StoragePool pool = (StoragePool)dataStoreMgr.getPrimaryDataStore(dataStoreTO.getUuid()); + primaryDataStoreTO.setDiskProvisioningStrictnessFlag(storageManager.DiskProvisioningStrictness.valueIn(pool.getDataCenterId())); } } return dataTO; From c96505b50de0d32a5332d787782764d788752d54 Mon Sep 17 00:00:00 2001 From: Spaceman1984 <49917670+Spaceman1984@users.noreply.github.com> Date: Fri, 12 Feb 2021 09:34:06 +0200 Subject: [PATCH 09/33] Delete __init__.py --- test/__init__.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 test/__init__.py diff --git a/test/__init__.py b/test/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 From 7ff9850cf43b003074ded766de00ea9e6250bf62 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Fri, 12 Feb 2021 10:55:21 +0200 Subject: [PATCH 10/33] Merge fix --- .../com/cloud/hypervisor/vmware/resource/VmwareResource.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 1080ff5ef36d..66e0876c31b4 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -50,9 +50,6 @@ import com.cloud.agent.api.GetStoragePoolCapabilitiesAnswer; import com.cloud.agent.api.GetStoragePoolCapabilitiesCommand; -import com.cloud.agent.api.to.DataTO; -import com.cloud.agent.api.to.DeployAsIsInfoTO; -import com.cloud.agent.api.ValidateVcenterDetailsCommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.storage.command.CopyCommand; import org.apache.cloudstack.storage.command.StorageSubSystemCommand; From e423075664ecb95de7f348ca4393d2a7dec19b51 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Fri, 12 Feb 2021 16:14:13 +0200 Subject: [PATCH 11/33] Fixed failing test --- .../storage/motion/AncientDataMotionStrategy.java | 6 +----- .../storage/motion/AncientDataMotionStrategyTest.java | 7 +++++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java index b438ecc24cbf..e9c6fc111725 100644 --- a/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java +++ b/engine/storage/datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java @@ -23,7 +23,6 @@ import javax.inject.Inject; -import com.cloud.dc.dao.DataCenterDao; import org.apache.cloudstack.engine.subsystem.api.storage.ClusterScope; import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult; import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy; @@ -87,9 +86,6 @@ public class AncientDataMotionStrategy implements DataMotionStrategy { @Inject StorageCacheManager cacheMgr; - @Inject - DataCenterDao dataCenterDao; - @Inject StorageManager storageManager; @@ -223,7 +219,7 @@ protected DataTO addFullCloneAndDiskprovisiongStrictnessFlagOnVMwareDest(DataTO if (dataStoreTO != null && dataStoreTO instanceof PrimaryDataStoreTO){ PrimaryDataStoreTO primaryDataStoreTO = (PrimaryDataStoreTO) dataStoreTO; primaryDataStoreTO.setFullCloneFlag(CapacityManager.VmwareCreateCloneFull.valueIn(primaryDataStoreTO.getId())); - StoragePool pool = (StoragePool)dataStoreMgr.getPrimaryDataStore(dataStoreTO.getUuid()); + StoragePool pool = storageManager.getStoragePool(primaryDataStoreTO.getId()); primaryDataStoreTO.setDiskProvisioningStrictnessFlag(storageManager.DiskProvisioningStrictness.valueIn(pool.getDataCenterId())); } } diff --git a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java index 969f58cd2bb8..cd46fc511f1f 100755 --- a/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java +++ b/engine/storage/datamotion/src/test/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategyTest.java @@ -27,6 +27,8 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.any; +import com.cloud.storage.StorageManager; +import com.cloud.storage.StoragePool; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.junit.Before; @@ -57,6 +59,10 @@ public class AncientDataMotionStrategyTest { PrimaryDataStoreTO dataStoreTO; @Mock ConfigKey vmwareKey; + @Mock + StorageManager storageManager; + @Mock + StoragePool storagePool; private static final long POOL_ID = 1l; private static final Boolean FULL_CLONE_FLAG = true; @@ -72,6 +78,7 @@ public void setup() throws Exception { when(dataTO.getHypervisorType()).thenReturn(HypervisorType.VMware); when(dataTO.getDataStore()).thenReturn(dataStoreTO); when(dataStoreTO.getId()).thenReturn(POOL_ID); + when(storageManager.getStoragePool(POOL_ID)).thenReturn(storagePool); } private void replaceVmwareCreateCloneFullField() throws Exception { From 429aaaf02125139c2bd94570ecb10ffc1a62825b Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 22 Feb 2021 15:09:24 +0200 Subject: [PATCH 12/33] Added comment about parameters --- .../src/main/java/com/cloud/storage/StorageManagerImpl.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 06ad1e3761d0..050dae568247 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2780,6 +2780,11 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) { return imageStoreVO; } + /** + * + * @param poolId - Storage pool id for pool to update - if null all supported pools are updated. + * @param failOnChecks - If true, throw an error if pool type and state checks fail. + */ @Override public void updateStorageCapabilities(Long poolId, boolean failOnChecks) { List pools = new ArrayList<>(); From f7dbb108a587fe730688a87fff7f3019eba7a3da Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Tue, 23 Feb 2021 12:08:12 +0200 Subject: [PATCH 13/33] Added error log when update fails --- .../cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java | 8 ++++++++ .../cloud/hypervisor/vmware/resource/VmwareResource.java | 7 +++++-- .../main/java/com/cloud/storage/StorageManagerImpl.java | 4 ++++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java index 0e638067d1f8..65db9b6a7bae 100644 --- a/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java +++ b/core/src/main/java/com/cloud/agent/api/GetStoragePoolCapabilitiesAnswer.java @@ -28,6 +28,14 @@ public GetStoragePoolCapabilitiesAnswer(GetStoragePoolCapabilitiesCommand cmd) { poolDetails = new HashMap<>(); } + public void setResult(boolean result){ + this.result = result; + } + + public void setDetails(String details){ + this.details = details; + } + public Map getPoolDetails() { return poolDetails; } diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java index 66e0876c31b4..86bbe8a6d98a 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/hypervisor/vmware/resource/VmwareResource.java @@ -5080,6 +5080,7 @@ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) { Map poolDetails = answer.getPoolDetails(); poolDetails.put(Storage.Capability.HARDWARE_ACCELERATION.toString(), String.valueOf(hardwareAccelerationSupportForDataStore)); answer.setPoolDetails(poolDetails); + answer.setResult(true); return answer; } catch (Throwable e) { @@ -5092,8 +5093,10 @@ protected Answer execute(GetStoragePoolCapabilitiesCommand cmd) { String msg = "GetStoragePoolCapabilitiesCommand failed due to " + VmwareHelper.getExceptionMessage(e); s_logger.error(msg, e); - - return new Answer(cmd, false, msg); + GetStoragePoolCapabilitiesAnswer answer = new GetStoragePoolCapabilitiesAnswer(cmd); + answer.setResult(false); + answer.setDetails(msg); + return answer; } } diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 050dae568247..ec21215e6549 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2854,6 +2854,10 @@ public void updateStorageCapabilities(Long poolId, boolean failOnChecks) { hardwareAccelerationSupported.setValue(answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString())); _storagePoolDetailsDao.update(hardwareAccelerationSupported.getId(), hardwareAccelerationSupported); } + } else { + if (answer != null && !answer.getResult()) { + s_logger.error("Failed to update storage pool capabilities: " + answer.getDetails()); + } } } } From 382b9eb2f061056a52820a7c862e07d6f9855b45 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Tue, 23 Feb 2021 12:42:16 +0200 Subject: [PATCH 14/33] Added exception when using API --- server/src/main/java/com/cloud/storage/StorageManagerImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index ec21215e6549..318d8e6a4145 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2857,6 +2857,9 @@ public void updateStorageCapabilities(Long poolId, boolean failOnChecks) { } else { if (answer != null && !answer.getResult()) { s_logger.error("Failed to update storage pool capabilities: " + answer.getDetails()); + if (failOnChecks) { + throw new CloudRuntimeException(answer.getDetails()); + } } } } From 67726ee5f10fd21428391d6281143363c02693f2 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Thu, 25 Feb 2021 11:55:52 +0200 Subject: [PATCH 15/33] Ordering storage pool selection to prefer thick disk capable pools if available --- .../api/storage/StoragePoolAllocator.java | 2 +- .../orchestration/VolumeOrchestrator.java | 2 +- .../AbstractStoragePoolAllocator.java | 27 +++++++++++++++++-- 3 files changed, 27 insertions(+), 4 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/StoragePoolAllocator.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/StoragePoolAllocator.java index c8fcf5f3df5c..fde71fe0d4c1 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/StoragePoolAllocator.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/StoragePoolAllocator.java @@ -60,5 +60,5 @@ public interface StoragePoolAllocator extends Adapter { static int RETURN_UPTO_ALL = -1; - List reorderPools(List pools, VirtualMachineProfile vmProfile, DeploymentPlan plan); + List reorderPools(List pools, VirtualMachineProfile vmProfile, DeploymentPlan plan, DiskProfile dskCh); } 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 1c4aead5941f..5a42b3a6b93d 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 @@ -349,7 +349,7 @@ public StoragePool findChildDataStoreInDataStoreCluster(DataCenter dc, Pod pod, VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm); for (StoragePoolAllocator allocator : _storagePoolAllocators) { DataCenterDeployment plan = new DataCenterDeployment(dc.getId(), podId, clusterId, hostId, null, null); - final List poolList = allocator.reorderPools(suitablePools, profile, plan); + final List poolList = allocator.reorderPools(suitablePools, profile, plan, null); if (poolList != null && !poolList.isEmpty()) { return (StoragePool)dataStoreMgr.getDataStore(poolList.get(0).getId(), DataStoreRole.Primary); diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index c7df5695a776..e1198ade1b82 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -100,7 +100,7 @@ public List allocateToPool(DiskProfile dskCh, VirtualMachineProfile @Override public List allocateToPool(DiskProfile dskCh, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, int returnUpTo, boolean bypassStorageTypeCheck) { List pools = select(dskCh, vmProfile, plan, avoid, returnUpTo, bypassStorageTypeCheck); - return reorderPools(pools, vmProfile, plan); + return reorderPools(pools, vmProfile, plan, dskCh); } protected List reorderPoolsByCapacity(DeploymentPlan plan, @@ -167,7 +167,7 @@ protected List reorderPoolsByNumberOfVolumes(DeploymentPlan plan, L } @Override - public List reorderPools(List pools, VirtualMachineProfile vmProfile, DeploymentPlan plan) { + public List reorderPools(List pools, VirtualMachineProfile vmProfile, DeploymentPlan plan, DiskProfile dskCh) { if (pools == null) { return null; } @@ -184,9 +184,32 @@ public List reorderPools(List pools, VirtualMachinePro } else if(allocationAlgorithm.equals("firstfitleastconsumed")){ pools = reorderPoolsByCapacity(plan, pools); } + + // Hardware accelerated pools are preferred for thick disks + if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) && + !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) { + pools = reorderPoolsByDiskProvisioningType(pools, dskCh); + } + return pools; } + private List reorderPoolsByDiskProvisioningType(List pools, DiskProfile dskCh) { + List reorderedPools = new ArrayList<>(); + for (StoragePool pool: pools) { + StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + if (pool.getPoolType() == Storage.StoragePoolType.NetworkFilesystem && + (hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true"))) { + // add to the bottom of the list + reorderedPools.add(pool); + } else { + // add to the top of the list + reorderedPools.add(0, pool); + } + } + return reorderedPools; + } + protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh, DeploymentPlan plan) { if (s_logger.isDebugEnabled()) { From bbd8516d244b9c50cfd05201cf0b5d0df33f5a80 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Thu, 25 Feb 2021 16:22:31 +0200 Subject: [PATCH 16/33] Removed unused parameter --- .../storage/allocator/AbstractStoragePoolAllocator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index e1198ade1b82..beb2d140eeac 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -188,13 +188,13 @@ public List reorderPools(List pools, VirtualMachinePro // Hardware accelerated pools are preferred for thick disks if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) && !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) { - pools = reorderPoolsByDiskProvisioningType(pools, dskCh); + pools = reorderPoolsByDiskProvisioningType(pools); } return pools; } - private List reorderPoolsByDiskProvisioningType(List pools, DiskProfile dskCh) { + private List reorderPoolsByDiskProvisioningType(List pools) { List reorderedPools = new ArrayList<>(); for (StoragePool pool: pools) { StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); From a511fd1c9e1ab527a3cba944a3f296fe1b2d11cf Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Fri, 26 Feb 2021 10:31:34 +0200 Subject: [PATCH 17/33] Reordering changes --- .../AbstractStoragePoolAllocator.java | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index beb2d140eeac..3e7e49eb5820 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -185,29 +185,33 @@ public List reorderPools(List pools, VirtualMachinePro pools = reorderPoolsByCapacity(plan, pools); } - // Hardware accelerated pools are preferred for thick disks - if (dskCh != null && !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) && + if (vmProfile.getHypervisorType() == HypervisorType.VMware && !storageMgr.DiskProvisioningStrictness.valueIn(plan.getDataCenterId())) { - pools = reorderPoolsByDiskProvisioningType(pools); + pools = reorderPoolsByDiskProvisioningType(pools, dskCh); } return pools; } - private List reorderPoolsByDiskProvisioningType(List pools) { - List reorderedPools = new ArrayList<>(); - for (StoragePool pool: pools) { - StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); - if (pool.getPoolType() == Storage.StoragePoolType.NetworkFilesystem && - (hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true"))) { - // add to the bottom of the list - reorderedPools.add(pool); - } else { - // add to the top of the list - reorderedPools.add(0, pool); + private List reorderPoolsByDiskProvisioningType(List pools, DiskProfile diskProfile) { + if (diskProfile != null && !diskProfile.getProvisioningType().equals(Storage.ProvisioningType.THIN)) { + List reorderedPools = new ArrayList<>(); + int preferredIndex = 0; + for (StoragePool pool : pools) { + StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + if (pool.getPoolType() == Storage.StoragePoolType.NetworkFilesystem && + (hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true"))) { + // add to the bottom of the list + reorderedPools.add(pool); + } else { + // add to the top of the list + reorderedPools.add(preferredIndex++, pool); + } } + return reorderedPools; + } else { + return pools; } - return reorderedPools; } protected boolean filter(ExcludeList avoid, StoragePool pool, DiskProfile dskCh, DeploymentPlan plan) { From 3838035922625dc1ca144b66d84c28431809bbdc Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Fri, 5 Mar 2021 07:07:40 +0200 Subject: [PATCH 18/33] Returning storage pool details after update --- .../apache/cloudstack/api/BaseListCmd.java | 2 +- .../admin/storage/ListStoragePoolsCmd.java | 5 ++++- .../storage/UpdateStorageCapabilitiesCmd.java | 21 ++++++++----------- .../api/storage/DataStoreCapabilities.java | 6 +++++- .../com/cloud/api/query/QueryManagerImpl.java | 16 +++++++++++++- 5 files changed, 34 insertions(+), 16 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java b/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java index 36fa36fcfc9a..950f488361fb 100644 --- a/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java @@ -94,7 +94,7 @@ public Long getPageSizeVal() { if (pageSizeInt != null) { defaultPageSize = pageSizeInt.longValue(); } - if (defaultPageSize.longValue() == s_pageSizeUnlimited) { + if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) { defaultPageSize = null; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java index ed123dbb6c37..2450ac7cd6ba 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/ListStoragePoolsCmd.java @@ -99,7 +99,10 @@ public Long getId() { return id; } - ///////////////////////////////////////////////////// + public void setId(Long id) { + this.id = id; + } +///////////////////////////////////////////////////// /////////////// API Implementation/////////////////// ///////////////////////////////////////////////////// diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java index ca9adf988d5d..d758c60f1512 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -26,13 +26,13 @@ import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.response.ListResponse; import org.apache.cloudstack.api.response.StoragePoolResponse; -import org.apache.cloudstack.api.response.SuccessResponse; import org.apache.cloudstack.context.CallContext; import org.apache.log4j.Logger; @APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs capabilities of storage pools", - responseObject = SuccessResponse.class, + responseObject = StoragePoolResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0") public class UpdateStorageCapabilitiesCmd extends BaseCmd { public static final String APINAME = "updateStorageCapabilities"; @@ -64,17 +64,14 @@ public void setPoolId(Long poolId) { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { - try { - _storageService.updateStorageCapabilities(poolId, poolId != null); - SuccessResponse response = new SuccessResponse(getCommandName()); - response.setSuccess(true); - this.setResponseObject(response); - } catch (Exception e) { - SuccessResponse response = new SuccessResponse(getCommandName()); - response.setSuccess(false); - this.setResponseObject(response); - LOG.error(e); + _storageService.updateStorageCapabilities(poolId, poolId != null); + ListStoragePoolsCmd listStoragePoolCmd = new ListStoragePoolsCmd(); + if (poolId != null){ + listStoragePoolCmd.setId(poolId); } + ListResponse response = _queryService.searchForStoragePools(listStoragePoolCmd); + response.setResponseName(getCommandName()); + this.setResponseObject(response); } @Override diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java index f537d8f52028..24968d9930a1 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java @@ -40,5 +40,9 @@ public enum DataStoreCapabilities { /** * indicates that this driver supports reverting a volume to a snapshot state */ - CAN_REVERT_VOLUME_TO_SNAPSHOT + CAN_REVERT_VOLUME_TO_SNAPSHOT, + /** + * indicates that this driver supports hardware acceleration + */ + HARDWARE_ACCELERATION } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 0c8a7f794355..7a81480462ff 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -119,6 +119,8 @@ import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.apache.cloudstack.storage.datastore.db.PrimaryDataStoreDao; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailVO; +import org.apache.cloudstack.storage.datastore.db.StoragePoolDetailsDao; import org.apache.cloudstack.storage.datastore.db.StoragePoolVO; import org.apache.cloudstack.storage.datastore.db.TemplateDataStoreDao; import org.apache.commons.collections.CollectionUtils; @@ -421,6 +423,9 @@ public class QueryManagerImpl extends MutualExclusiveIdsManagerBase implements Q @Inject private PrimaryDataStoreDao _storagePoolDao; + @Inject + private StoragePoolDetailsDao _storagePoolDetailsDao; + @Inject private ProjectInvitationDao projectInvitationDao; @@ -2420,7 +2425,16 @@ public ListResponse searchForStoragePools(ListStoragePoolsC if (store != null) { DataStoreDriver driver = store.getDriver(); if (driver != null && driver.getCapabilities() != null) { - poolResponse.setCaps(driver.getCapabilities()); + Map caps = driver.getCapabilities(); + if (Storage.StoragePoolType.NetworkFilesystem.toString().equals(poolResponse.getType()) && + HypervisorType.VMware.toString().equals(poolResponse.getHypervisor())) { + StoragePoolVO pool = _storagePoolDao.findPoolByUUID(poolResponse.getId()); + StoragePoolDetailVO detail = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + if (detail != null) { + caps.put(DataStoreCapabilities.HARDWARE_ACCELERATION.toString(), detail.getValue()); + } + } + poolResponse.setCaps(caps); } } } From b59da6b22fca10207ba568811f0658d354115c05 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 8 Mar 2021 11:10:54 +0200 Subject: [PATCH 19/33] Removed multiple pool update, updated marvin test, removed duplicate enum --- .../main/java/com/cloud/storage/Storage.java | 2 +- .../storage/UpdateStorageCapabilitiesCmd.java | 14 +++++------- .../api/storage/DataStoreCapabilities.java | 1 - .../com/cloud/api/query/QueryManagerImpl.java | 2 +- .../com/cloud/storage/StorageManagerImpl.java | 3 +-- .../smoke/test_disk_provisioning_types.py | 22 +++++++++++-------- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/Storage.java b/api/src/main/java/com/cloud/storage/Storage.java index 91724ba1ab0b..407935919ca2 100644 --- a/api/src/main/java/com/cloud/storage/Storage.java +++ b/api/src/main/java/com/cloud/storage/Storage.java @@ -77,7 +77,7 @@ public String getFileExtension() { } public static enum Capability { - HARDWARE_ACCELERATION("hardwareAcceleration"); + HARDWARE_ACCELERATION("HARDWARE_ACCELERATION"); private final String capability; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java index d758c60f1512..dca0f4a658f5 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -42,7 +42,7 @@ public class UpdateStorageCapabilitiesCmd extends BaseCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// - @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = false, description = "Storage pool id") + @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = StoragePoolResponse.class, required = true, description = "Storage pool id") private Long poolId; ///////////////////////////////////////////////////// @@ -64,14 +64,12 @@ public void setPoolId(Long poolId) { @Override public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException { - _storageService.updateStorageCapabilities(poolId, poolId != null); + _storageService.updateStorageCapabilities(poolId, true); ListStoragePoolsCmd listStoragePoolCmd = new ListStoragePoolsCmd(); - if (poolId != null){ - listStoragePoolCmd.setId(poolId); - } - ListResponse response = _queryService.searchForStoragePools(listStoragePoolCmd); - response.setResponseName(getCommandName()); - this.setResponseObject(response); + listStoragePoolCmd.setId(poolId); + ListResponse listResponse = _queryService.searchForStoragePools(listStoragePoolCmd); + listResponse.setResponseName(getCommandName()); + this.setResponseObject(listResponse); } @Override diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java index 24968d9930a1..508b25a0285f 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java @@ -44,5 +44,4 @@ public enum DataStoreCapabilities { /** * indicates that this driver supports hardware acceleration */ - HARDWARE_ACCELERATION } diff --git a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java index 7a81480462ff..d034c4e844a8 100644 --- a/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java +++ b/server/src/main/java/com/cloud/api/query/QueryManagerImpl.java @@ -2431,7 +2431,7 @@ public ListResponse searchForStoragePools(ListStoragePoolsC StoragePoolVO pool = _storagePoolDao.findPoolByUUID(poolResponse.getId()); StoragePoolDetailVO detail = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); if (detail != null) { - caps.put(DataStoreCapabilities.HARDWARE_ACCELERATION.toString(), detail.getValue()); + caps.put(Storage.Capability.HARDWARE_ACCELERATION.toString(), detail.getValue()); } } poolResponse.setCaps(caps); diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index 318d8e6a4145..fec8ae0bb917 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2781,8 +2781,7 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) { } /** - * - * @param poolId - Storage pool id for pool to update - if null all supported pools are updated. + * @param poolId - Storage pool id for pool to update. * @param failOnChecks - If true, throw an error if pool type and state checks fail. */ @Override diff --git a/test/integration/smoke/test_disk_provisioning_types.py b/test/integration/smoke/test_disk_provisioning_types.py index 8fd16afd4f62..d6affc13d0a8 100644 --- a/test/integration/smoke/test_disk_provisioning_types.py +++ b/test/integration/smoke/test_disk_provisioning_types.py @@ -22,6 +22,7 @@ from marvin.lib.common import list_disk_offering, get_zone, get_suitable_test_template, get_domain from marvin.cloudstackAPI import listStoragePools, updateStorageCapabilities from nose.plugins.attrib import attr +import json class TestDiskProvisioningTypes(cloudstackTestCase): @@ -91,15 +92,18 @@ def test_03_vm_with_sparse_disk_offering(self): def test_05_update_cmd(self): cmd = listStoragePools.listStoragePoolsCmd() storagePools = self.apiclient.listStoragePools(cmd) - pool = storagePools[0] - cmd = updateStorageCapabilities.updateStorageCapabilitiesCmd() - cmd.id = pool.id - response = self.apiclient.updateStorageCapabilities(cmd) - self.assertEqual( - response['success'], - True, - "Check Updated storage pool capabilities" - ) + + for pool in storagePools: + if pool.type == 'NetworkFilesystem': + cmd = updateStorageCapabilities.updateStorageCapabilitiesCmd() + cmd.id = pool.id + response = self.apiclient.updateStorageCapabilities(cmd) + acceleration = getattr(response[0].storagecapabilities, "HARDWARE_ACCELERATION") + self.assertNotEqual( + acceleration, + None, + "Check Updated storage pool capabilities" + ) def runner(self, provisioning_type): self.services["disk_offering"]['provisioningtype'] = provisioning_type From dbc76713d92e4d80352c5aaf9254a098dc228ded Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 8 Mar 2021 11:15:36 +0200 Subject: [PATCH 20/33] Removed comment --- .../engine/subsystem/api/storage/DataStoreCapabilities.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java index 508b25a0285f..f537d8f52028 100644 --- a/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java +++ b/engine/api/src/main/java/org/apache/cloudstack/engine/subsystem/api/storage/DataStoreCapabilities.java @@ -40,8 +40,5 @@ public enum DataStoreCapabilities { /** * indicates that this driver supports reverting a volume to a snapshot state */ - CAN_REVERT_VOLUME_TO_SNAPSHOT, - /** - * indicates that this driver supports hardware acceleration - */ + CAN_REVERT_VOLUME_TO_SNAPSHOT } From 416231f33279a0ead27b4aafe4ae7920f81df43a Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 8 Mar 2021 11:20:43 +0200 Subject: [PATCH 21/33] Removed unused import --- test/integration/smoke/test_disk_provisioning_types.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/smoke/test_disk_provisioning_types.py b/test/integration/smoke/test_disk_provisioning_types.py index d6affc13d0a8..c87b2e4969cd 100644 --- a/test/integration/smoke/test_disk_provisioning_types.py +++ b/test/integration/smoke/test_disk_provisioning_types.py @@ -22,7 +22,6 @@ from marvin.lib.common import list_disk_offering, get_zone, get_suitable_test_template, get_domain from marvin.cloudstackAPI import listStoragePools, updateStorageCapabilities from nose.plugins.attrib import attr -import json class TestDiskProvisioningTypes(cloudstackTestCase): From f32162a2e2cea0e5c5cf0ea99461276430891239 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 8 Mar 2021 12:10:51 +0200 Subject: [PATCH 22/33] Removed for loop --- .../com/cloud/storage/StorageManagerImpl.java | 103 ++++++++---------- 1 file changed, 43 insertions(+), 60 deletions(-) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index fec8ae0bb917..dc8c20ae2d10 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2786,79 +2786,62 @@ public ImageStore updateImageStoreStatus(Long id, Boolean readonly) { */ @Override public void updateStorageCapabilities(Long poolId, boolean failOnChecks) { - List pools = new ArrayList<>(); - if (poolId == null) { - pools = _storagePoolDao.listByStatus(StoragePoolStatus.Up); - } else { - StoragePoolVO pool = _storagePoolDao.findById(poolId); - - if (pool == null) { - throw new CloudRuntimeException("Primary storage not found for id: " + poolId); - } - - pools.add(pool); - } + StoragePoolVO pool = _storagePoolDao.findById(poolId); - if (pools.size() == 0) { - throw new CloudRuntimeException("No storage pools found to update."); + if (pool == null) { + throw new CloudRuntimeException("Primary storage not found for id: " + poolId); } - for (StoragePoolVO pool: pools) { - - // Only checking NFS for now - required for disk provisioning type support for vmware. - if (pool.getPoolType() != StoragePoolType.NetworkFilesystem) { - if (failOnChecks) { - throw new CloudRuntimeException("Storage capabilities update only supported on NFS storage mounted."); - } - continue; + // Only checking NFS for now - required for disk provisioning type support for vmware. + if (pool.getPoolType() != StoragePoolType.NetworkFilesystem) { + if (failOnChecks) { + throw new CloudRuntimeException("Storage capabilities update only supported on NFS storage mounted."); } + } - if (pool.getStatus() != StoragePoolStatus.Initialized && pool.getStatus() != StoragePoolStatus.Up) { - if (failOnChecks){ - throw new CloudRuntimeException("Primary storage is not in the right state to update capabilities"); - } - continue; + if (pool.getStatus() != StoragePoolStatus.Initialized && pool.getStatus() != StoragePoolStatus.Up) { + if (failOnChecks){ + throw new CloudRuntimeException("Primary storage is not in the right state to update capabilities"); } + } - HypervisorType hypervisor = pool.getHypervisor(); + HypervisorType hypervisor = pool.getHypervisor(); - if (hypervisor == null){ - if (pool.getClusterId() != null) { - ClusterVO cluster = _clusterDao.findById(pool.getClusterId()); - hypervisor = cluster.getHypervisorType(); - } + if (hypervisor == null){ + if (pool.getClusterId() != null) { + ClusterVO cluster = _clusterDao.findById(pool.getClusterId()); + hypervisor = cluster.getHypervisorType(); } + } - if (!HypervisorType.VMware.equals(hypervisor)) { - if (failOnChecks) { - throw new CloudRuntimeException("Storage capabilities update only supported on VMWare."); - } - continue; + if (!HypervisorType.VMware.equals(hypervisor)) { + if (failOnChecks) { + throw new CloudRuntimeException("Storage capabilities update only supported on VMWare."); } + } - // find the host - List poolIds = new ArrayList(); - poolIds.add(pool.getId()); - List hosts = _storagePoolHostDao.findHostsConnectedToPools(poolIds); - if (hosts.size() > 0) { - GetStoragePoolCapabilitiesCommand cmd = new GetStoragePoolCapabilitiesCommand(); - cmd.setPool(new StorageFilerTO(pool)); - GetStoragePoolCapabilitiesAnswer answer = (GetStoragePoolCapabilitiesAnswer) _agentMgr.easySend(hosts.get(0), cmd); - if (answer.getPoolDetails() != null && answer.getPoolDetails().containsKey(Storage.Capability.HARDWARE_ACCELERATION.toString())) { - StoragePoolDetailVO hardwareAccelerationSupported = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); - if (hardwareAccelerationSupported == null) { - StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString(), answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString()), false); - _storagePoolDetailsDao.persist(storagePoolDetailVO); - } else { - hardwareAccelerationSupported.setValue(answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString())); - _storagePoolDetailsDao.update(hardwareAccelerationSupported.getId(), hardwareAccelerationSupported); - } + // find the host + List poolIds = new ArrayList(); + poolIds.add(pool.getId()); + List hosts = _storagePoolHostDao.findHostsConnectedToPools(poolIds); + if (hosts.size() > 0) { + GetStoragePoolCapabilitiesCommand cmd = new GetStoragePoolCapabilitiesCommand(); + cmd.setPool(new StorageFilerTO(pool)); + GetStoragePoolCapabilitiesAnswer answer = (GetStoragePoolCapabilitiesAnswer) _agentMgr.easySend(hosts.get(0), cmd); + if (answer.getPoolDetails() != null && answer.getPoolDetails().containsKey(Storage.Capability.HARDWARE_ACCELERATION.toString())) { + StoragePoolDetailVO hardwareAccelerationSupported = _storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); + if (hardwareAccelerationSupported == null) { + StoragePoolDetailVO storagePoolDetailVO = new StoragePoolDetailVO(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString(), answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString()), false); + _storagePoolDetailsDao.persist(storagePoolDetailVO); } else { - if (answer != null && !answer.getResult()) { - s_logger.error("Failed to update storage pool capabilities: " + answer.getDetails()); - if (failOnChecks) { - throw new CloudRuntimeException(answer.getDetails()); - } + hardwareAccelerationSupported.setValue(answer.getPoolDetails().get(Storage.Capability.HARDWARE_ACCELERATION.toString())); + _storagePoolDetailsDao.update(hardwareAccelerationSupported.getId(), hardwareAccelerationSupported); + } + } else { + if (answer != null && !answer.getResult()) { + s_logger.error("Failed to update storage pool capabilities: " + answer.getDetails()); + if (failOnChecks) { + throw new CloudRuntimeException(answer.getDetails()); } } } From 35a73113978203f41ccb9cd01ee66a61f4f36faa Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Mon, 8 Mar 2021 12:56:20 +0200 Subject: [PATCH 23/33] Added missing return statements for failed checks --- server/src/main/java/com/cloud/storage/StorageManagerImpl.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java index dc8c20ae2d10..3536ca527f33 100644 --- a/server/src/main/java/com/cloud/storage/StorageManagerImpl.java +++ b/server/src/main/java/com/cloud/storage/StorageManagerImpl.java @@ -2797,12 +2797,14 @@ public void updateStorageCapabilities(Long poolId, boolean failOnChecks) { if (failOnChecks) { throw new CloudRuntimeException("Storage capabilities update only supported on NFS storage mounted."); } + return; } if (pool.getStatus() != StoragePoolStatus.Initialized && pool.getStatus() != StoragePoolStatus.Up) { if (failOnChecks){ throw new CloudRuntimeException("Primary storage is not in the right state to update capabilities"); } + return; } HypervisorType hypervisor = pool.getHypervisor(); @@ -2818,6 +2820,7 @@ public void updateStorageCapabilities(Long poolId, boolean failOnChecks) { if (failOnChecks) { throw new CloudRuntimeException("Storage capabilities update only supported on VMWare."); } + return; } // find the host From 29de816fac54c9b7c868f644e71eed36d29deed9 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Wed, 17 Mar 2021 13:52:59 +0200 Subject: [PATCH 24/33] Class name change --- .../api/command/admin/storage/UpdateStorageCapabilitiesCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java index dca0f4a658f5..df05de976285 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -36,7 +36,7 @@ requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0") public class UpdateStorageCapabilitiesCmd extends BaseCmd { public static final String APINAME = "updateStorageCapabilities"; - private static final Logger LOG = Logger.getLogger(UpdateImageStoreCmd.class.getName()); + private static final Logger LOG = Logger.getLogger(UpdateStorageCapabilitiesCmd.class.getName()); ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// From 8447cb832b87248e17fcd92f115aea617d105617 Mon Sep 17 00:00:00 2001 From: Darrin Husselmann Date: Wed, 28 Apr 2021 10:16:16 +0200 Subject: [PATCH 25/33] Null pointer --- .../storage/allocator/AbstractStoragePoolAllocator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 3e7e49eb5820..21fc3270c781 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -194,7 +194,7 @@ public List reorderPools(List pools, VirtualMachinePro } private List reorderPoolsByDiskProvisioningType(List pools, DiskProfile diskProfile) { - if (diskProfile != null && !diskProfile.getProvisioningType().equals(Storage.ProvisioningType.THIN)) { + if (diskProfile != null && diskProfile.getProvisioningType() != null && !diskProfile.getProvisioningType().equals(Storage.ProvisioningType.THIN)) { List reorderedPools = new ArrayList<>(); int preferredIndex = 0; for (StoragePool pool : pools) { From 2486db78dca8e034d8ad2386174dfb11004ce654 Mon Sep 17 00:00:00 2001 From: spaceman1984 Date: Fri, 28 May 2021 16:15:26 +0200 Subject: [PATCH 26/33] Added more info when a deployment fails --- .../deploy/DeploymentPlanningManager.java | 6 +++-- .../cloud/vm/VirtualMachineManagerImpl.java | 24 +++++++++++++++---- .../cloud/entity/api/VMEntityManagerImpl.java | 10 ++++++-- .../deploy/DeploymentPlanningManagerImpl.java | 14 ++++++----- .../cloud/resource/ResourceManagerImpl.java | 11 ++++++--- .../vm/UnmanagedVMsManagerImpl.java | 9 +++++-- .../DeploymentPlanningManagerImplTest.java | 6 ++--- 7 files changed, 57 insertions(+), 23 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java b/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java index d3a5f7f73766..53dc2f02bec3 100644 --- a/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java +++ b/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java @@ -24,6 +24,8 @@ import com.cloud.vm.VirtualMachineProfile; import org.apache.cloudstack.framework.config.ConfigKey; +import java.util.List; + public interface DeploymentPlanningManager extends Manager { @@ -48,8 +50,8 @@ public interface DeploymentPlanningManager extends Manager { * * */ - DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, - ExcludeList avoids, DeploymentPlanner planner) throws InsufficientServerCapacityException, AffinityConflictException; + DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, + DeploymentPlanner planner, List errors) throws InsufficientServerCapacityException, AffinityConflictException; String finalizeReservation(DeployDestination plannedDestination, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, DeploymentPlanner planner) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index 0cb766e7b6ee..2bc4df408b25 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1117,11 +1117,16 @@ public void orchestrateStart(final String vmUuid, final Map errors = new ArrayList<>(); try { - dest = _dpMgr.planDeployment(vmProfile, plan, avoids, planner); + dest = _dpMgr.planDeployment(vmProfile, plan, avoids, planner, errors); } catch (final AffinityConflictException e2) { s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2); - throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict"); + String errorMessage = "Unable to create deployment, affinity rules associted to the VM conflict"; + for (String error: errors) { + errorMessage += "\n" + errors; + } + throw new CloudRuntimeException(errorMessage); } if (dest == null) { @@ -3372,11 +3377,14 @@ private void orchestrateMigrateAway(final String vmUuid, final long srcHostId, f excludes.addHost(hostId); DeployDestination dest = null; + + ArrayList errors = new ArrayList(); + while (true) { try { plan.setMigrationPlan(true); - dest = _dpMgr.planDeployment(profile, plan, excludes, planner); + dest = _dpMgr.planDeployment(profile, plan, excludes, planner, errors); } catch (final AffinityConflictException e2) { s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2); throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict"); @@ -3390,7 +3398,13 @@ private void orchestrateMigrateAway(final String vmUuid, final long srcHostId, f if (s_logger.isDebugEnabled()) { s_logger.debug("Unable to find destination for migrating the vm " + profile); } - throw new InsufficientServerCapacityException("Unable to find a server to migrate to.", host.getClusterId()); + + String errorMessage = "Unable to find a server to migrate to."; + for (String error: errors){ + errorMessage += "\n" + error; + } + + throw new InsufficientServerCapacityException(errorMessage, host.getClusterId()); } excludes.addHost(dest.getHost().getId()); @@ -4271,7 +4285,7 @@ public void findHostAndMigrate(final String vmUuid, final Long newSvcOfferingId, DeployDestination dest = null; try { - dest = _dpMgr.planDeployment(profile, plan, excludes, null); + dest = _dpMgr.planDeployment(profile, plan, excludes, null, new ArrayList<>()); } catch (final AffinityConflictException e2) { s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2); throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict"); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java index 02ffb37ba158..59e7caebb3e7 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java @@ -16,6 +16,7 @@ // under the License. package org.apache.cloudstack.engine.cloud.entity.api; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.UUID; @@ -199,8 +200,9 @@ public String reserveVirtualMachine(VMEntityVO vmEntityVO, DeploymentPlanner pla while (true) { DeployDestination dest = null; + List errors = new ArrayList<>(); try { - dest = _dpMgr.planDeployment(vmProfile, plan, exclude, plannerToUse); + dest = _dpMgr.planDeployment(vmProfile, plan, exclude, plannerToUse, errors); } catch (AffinityConflictException e) { throw new CloudRuntimeException("Unable to create deployment, affinity rules associated to the VM conflict"); } @@ -221,7 +223,11 @@ public String reserveVirtualMachine(VMEntityVO vmEntityVO, DeploymentPlanner pla // call retry it. return UUID.randomUUID().toString(); } else { - throw new InsufficientServerCapacityException("Unable to create a deployment for " + vmProfile, DataCenter.class, plan.getDataCenterId(), + String errorMessage = "Unable to create a deployment for " + vmProfile; + for(String error: errors ){ + errorMessage += "\n" + error; + } + throw new InsufficientServerCapacityException(errorMessage, DataCenter.class, plan.getDataCenterId(), areAffinityGroupsAssociated(vmProfile)); } } diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index 7a939ae263e4..1eb33d5e8702 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -268,7 +268,7 @@ public void setAffinityGroupProcessors(List affinityProc } @Override - public DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, DeploymentPlanner planner) + public DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, DeploymentPlanner planner, List errors) throws InsufficientServerCapacityException, AffinityConflictException { ServiceOffering offering = vmProfile.getServiceOffering(); @@ -531,9 +531,8 @@ public DeployDestination planDeployment(VirtualMachineProfile vmProfile, Deploym avoids.getPoolsToAvoid()); resetAvoidSet(plannerAvoidOutput, plannerAvoidInput); - - dest = - checkClustersforDestination(clusterList, vmProfile, plan, avoids, dc, getPlannerUsage(planner, vmProfile, plan, avoids), plannerAvoidOutput); + dest = checkClustersforDestination(clusterList, vmProfile, plan, avoids, dc, + getPlannerUsage(planner, vmProfile, plan, avoids), plannerAvoidOutput, errors); if (dest != null) { return dest; } @@ -1170,7 +1169,7 @@ public void cleanupVMReservations() { // /refactoring planner methods private DeployDestination checkClustersforDestination(List clusterList, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, DataCenter dc, - DeploymentPlanner.PlannerResourceUsage resourceUsageRequired, ExcludeList plannerAvoidOutput) { + DeploymentPlanner.PlannerResourceUsage resourceUsageRequired, ExcludeList plannerAvoidOutput, List errors) { if (s_logger.isTraceEnabled()) { s_logger.trace("ClusterId List to consider: " + clusterList); @@ -1181,6 +1180,7 @@ private DeployDestination checkClustersforDestination(List clusterList, Vi if (clusterVO.getHypervisorType() != vmProfile.getHypervisorType()) { s_logger.debug("Cluster: " + clusterId + " has HyperVisorType that does not match the VM, skipping this cluster"); + errors.add("Cluster: " + clusterId + " has HyperVisorType that does not match the VM, skipping this cluster"); avoid.addCluster(clusterVO.getId()); continue; } @@ -1230,9 +1230,11 @@ private DeployDestination checkClustersforDestination(List clusterList, Vi } } else { s_logger.debug("No suitable storagePools found under this Cluster: " + clusterId); + errors.add("No suitable storagePools found under Cluster: " + clusterId); } } else { - s_logger.debug("No suitable hosts found under this Cluster: " + clusterId); + s_logger.debug("No suitable hosts found under Cluster: " + clusterId); + errors.add("No suitable hosts found under Cluster: " + clusterId); } } diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index ca3505fb6800..dc0606cb5b7e 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1333,11 +1333,16 @@ private void migrateAwayVmWithVolumes(HostVO host, VMInstanceVO vm) { final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offeringVO, null, null); plan.setMigrationPlan(true); DeployDestination dest = null; + ArrayList errors = new ArrayList<>(); try { - dest = deploymentManager.planDeployment(profile, plan, new DeploymentPlanner.ExcludeList(), null); + dest = deploymentManager.planDeployment(profile, plan, new DeploymentPlanner.ExcludeList(), null, errors); } catch (InsufficientServerCapacityException e) { - throw new CloudRuntimeException(String.format("Maintenance failed, could not find deployment destination for VM [id=%s, name=%s].", vm.getId(), vm.getInstanceName()), - e); + + String errorMessage = String.format("Maintenance failed, could not find deployment destination for VM [id=%s, name=%s].", vm.getId(), vm.getInstanceName()); + for (String error: errors) { + errorMessage += "\n" + error; + } + throw new CloudRuntimeException(errorMessage, e); } Host destHost = dest.getHost(); diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 501ca6328020..4d5a5c80df71 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -770,10 +770,15 @@ private UserVm migrateImportedVM(HostVO sourceHost, VirtualMachineTemplate templ excludeList.addHost(sourceHost.getId()); final DataCenterDeployment plan = new DataCenterDeployment(sourceHost.getDataCenterId(), sourceHost.getPodId(), sourceHost.getClusterId(), null, null, null); DeployDestination dest = null; + ArrayList errors = new ArrayList<>(); try { - dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null); + dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null, errors); } catch (Exception e) { - LOGGER.warn(String.format("VM import failed for unmanaged vm: %s during vm migration, finding deployment destination", vm.getInstanceName()), e); + String errorMessage = String.format("VM import failed for unmanaged vm: %s during vm migration, finding deployment destination", vm.getInstanceName()); + for (String error: errors) { + errorMessage += "\n" + error; + } + LOGGER.warn(errorMessage, e); cleanupFailedImportVM(vm); throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VM import failed for unmanaged vm: %s during vm migration, finding deployment destination", vm.getInstanceName())); } diff --git a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java index 97e15de6f846..ea40bfc0ae55 100644 --- a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java +++ b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java @@ -220,7 +220,7 @@ public void dataCenterAvoidTest() throws InsufficientServerCapacityException, Af DataCenterDeployment plan = new DataCenterDeployment(dataCenterId); Mockito.when(avoids.shouldAvoid((DataCenterVO)Matchers.anyObject())).thenReturn(true); - DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); + DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null, new ArrayList<>()); assertNull("DataCenter is in avoid set, destination should be null! ", dest); } @@ -236,7 +236,7 @@ public void plannerCannotHandleTest() throws InsufficientServerCapacityException Mockito.when(avoids.shouldAvoid((DataCenterVO)Matchers.anyObject())).thenReturn(false); Mockito.when(_planner.canHandle(vmProfile, plan, avoids)).thenReturn(false); - DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); + DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null, new ArrayList<>()); assertNull("Planner cannot handle, destination should be null! ", dest); } @@ -253,7 +253,7 @@ public void emptyClusterListTest() throws InsufficientServerCapacityException, A Mockito.when(_planner.canHandle(vmProfile, plan, avoids)).thenReturn(true); Mockito.when(((DeploymentClusterPlanner)_planner).orderClusters(vmProfile, plan, avoids)).thenReturn(null); - DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); + DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null, new ArrayList<>()); assertNull("Planner cannot handle, destination should be null! ", dest); } From 990caed09725db2805da20716d94393f29562eae Mon Sep 17 00:00:00 2001 From: spaceman1984 Date: Tue, 1 Jun 2021 10:56:50 +0200 Subject: [PATCH 27/33] Null pointer --- .../storage/allocator/AbstractStoragePoolAllocator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java index 21fc3270c781..04a5224a5b80 100644 --- a/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java +++ b/engine/storage/src/main/java/org/apache/cloudstack/storage/allocator/AbstractStoragePoolAllocator.java @@ -292,8 +292,8 @@ private boolean checkDiskProvisioningSupport(DiskProfile dskCh, StoragePool pool if (dskCh.getHypervisorType() != null && dskCh.getHypervisorType() == HypervisorType.VMware && pool.getPoolType() == Storage.StoragePoolType.NetworkFilesystem && storageMgr.DiskProvisioningStrictness.valueIn(pool.getDataCenterId())) { StoragePoolDetailVO hardwareAcceleration = storagePoolDetailsDao.findDetail(pool.getId(), Storage.Capability.HARDWARE_ACCELERATION.toString()); - if (!dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) && - (hardwareAcceleration == null || !hardwareAcceleration.getValue().equals("true"))) { + if (dskCh.getProvisioningType() == null || !dskCh.getProvisioningType().equals(Storage.ProvisioningType.THIN) && + (hardwareAcceleration == null || hardwareAcceleration.getValue() == null || !hardwareAcceleration.getValue().equals("true"))) { return false; } } From 5dd383827b4e754ebfc1787395c0d8edc441375c Mon Sep 17 00:00:00 2001 From: Spaceman1984 <49917670+Spaceman1984@users.noreply.github.com> Date: Tue, 15 Jun 2021 10:34:22 +0200 Subject: [PATCH 28/33] Update api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java Co-authored-by: dahn --- api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java b/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java index 950f488361fb..8f46ebe0fbde 100644 --- a/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java @@ -94,7 +94,7 @@ public Long getPageSizeVal() { if (pageSizeInt != null) { defaultPageSize = pageSizeInt.longValue(); } - if (defaultPageSize!= null && defaultPageSize.longValue() == s_pageSizeUnlimited) { + if (s_pageSizeUnlimited.equals(defaultPageSize) { defaultPageSize = null; } From 396f89c37d60feb4a4a402e6bdc9131f533b8c7c Mon Sep 17 00:00:00 2001 From: spaceman1984 Date: Wed, 16 Jun 2021 09:03:33 +0200 Subject: [PATCH 29/33] Small bug fix on API response and added missing bracket --- api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java | 2 +- .../command/admin/storage/UpdateStorageCapabilitiesCmd.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java b/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java index 8f46ebe0fbde..bcebbb860033 100644 --- a/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/BaseListCmd.java @@ -94,7 +94,7 @@ public Long getPageSizeVal() { if (pageSizeInt != null) { defaultPageSize = pageSizeInt.longValue(); } - if (s_pageSizeUnlimited.equals(defaultPageSize) { + if (s_pageSizeUnlimited.equals(defaultPageSize)) { defaultPageSize = null; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java index df05de976285..b6fb03dd7985 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/admin/storage/UpdateStorageCapabilitiesCmd.java @@ -31,6 +31,8 @@ import org.apache.cloudstack.context.CallContext; import org.apache.log4j.Logger; +import java.util.Locale; + @APICommand(name = UpdateStorageCapabilitiesCmd.APINAME, description = "Syncs capabilities of storage pools", responseObject = StoragePoolResponse.class, requestHasSensitiveInfo = false, responseHasSensitiveInfo = false, since = "4.16.0") @@ -74,7 +76,7 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE @Override public String getCommandName() { - return APINAME; + return APINAME.toLowerCase(Locale.ROOT) + "response" ; } @Override From 72e7e8cd894551330b284070cc24f90ffca66148 Mon Sep 17 00:00:00 2001 From: spaceman1984 Date: Tue, 6 Jul 2021 11:52:13 +0200 Subject: [PATCH 30/33] Removed datastore cluster code --- .../storage/datastore/provider/DefaultHostListener.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index 1104b62f10e3..6664bdcded0d 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -29,7 +29,6 @@ import com.cloud.storage.StorageManager; import com.cloud.storage.StoragePool; import com.cloud.storage.StoragePoolHostVO; -import com.cloud.storage.StoragePoolStatus; import com.cloud.storage.StorageService; import com.cloud.storage.dao.StoragePoolHostDao; import com.cloud.storage.dao.StoragePoolTagsDao; @@ -62,7 +61,6 @@ public class DefaultHostListener implements HypervisorHostListener { StoragePoolDetailsDao storagePoolDetailsDao; @Inject StorageManager storageManager; - StoragePoolTagsDao storagePoolTagsDao; @Inject StorageService storageService; From bcb173136df3c1a308c1061ccd27b58d9063f0a3 Mon Sep 17 00:00:00 2001 From: spaceman1984 Date: Tue, 13 Jul 2021 10:58:20 +0200 Subject: [PATCH 31/33] Removed unused imports, added missing signature --- api/src/main/java/com/cloud/storage/StorageService.java | 4 +++- .../storage/datastore/provider/DefaultHostListener.java | 1 - .../com/cloud/storage/resource/VmwareStorageProcessor.java | 1 - .../src/main/java/com/cloud/server/ManagementServerImpl.java | 1 - 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/com/cloud/storage/StorageService.java b/api/src/main/java/com/cloud/storage/StorageService.java index 2597506b651b..bb086ad05cbf 100644 --- a/api/src/main/java/com/cloud/storage/StorageService.java +++ b/api/src/main/java/com/cloud/storage/StorageService.java @@ -26,8 +26,8 @@ import org.apache.cloudstack.api.command.admin.storage.DeleteImageStoreCmd; import org.apache.cloudstack.api.command.admin.storage.DeletePoolCmd; import org.apache.cloudstack.api.command.admin.storage.DeleteSecondaryStagingStoreCmd; -import org.apache.cloudstack.api.command.admin.storage.UpdateStoragePoolCmd; import org.apache.cloudstack.api.command.admin.storage.SyncStoragePoolCmd; +import org.apache.cloudstack.api.command.admin.storage.UpdateStoragePoolCmd; import com.cloud.exception.DiscoveryException; import com.cloud.exception.InsufficientCapacityException; @@ -107,4 +107,6 @@ public interface StorageService { void updateStorageCapabilities(Long poolId, boolean failOnChecks); + StoragePool syncStoragePool(SyncStoragePoolCmd cmd); + } diff --git a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java index 6664bdcded0d..30cd7ac3f2f1 100644 --- a/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java +++ b/engine/storage/volume/src/main/java/org/apache/cloudstack/storage/datastore/provider/DefaultHostListener.java @@ -31,7 +31,6 @@ import com.cloud.storage.StoragePoolHostVO; import com.cloud.storage.StorageService; import com.cloud.storage.dao.StoragePoolHostDao; -import com.cloud.storage.dao.StoragePoolTagsDao; import com.cloud.utils.exception.CloudRuntimeException; import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager; import org.apache.cloudstack.engine.subsystem.api.storage.HypervisorHostListener; diff --git a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java index 20f7902060df..7884a03f7a28 100644 --- a/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java +++ b/plugins/hypervisors/vmware/src/main/java/com/cloud/storage/resource/VmwareStorageProcessor.java @@ -129,7 +129,6 @@ import com.vmware.vim25.VirtualDeviceConfigSpecOperation; import com.vmware.vim25.VirtualDisk; import com.vmware.vim25.VirtualDiskFlatVer2BackingInfo; -import com.vmware.vim25.VirtualDiskType; import com.vmware.vim25.VirtualMachineConfigSpec; import com.vmware.vim25.VmConfigInfo; import com.vmware.vim25.VmfsDatastoreExpandSpec; diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index f888fd72fd4b..99a1ab91ec3b 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -208,7 +208,6 @@ import org.apache.cloudstack.api.command.admin.storage.UpdateImageStoreCmd; import org.apache.cloudstack.api.command.admin.storage.UpdateStorageCapabilitiesCmd; import org.apache.cloudstack.api.command.admin.storage.UpdateStoragePoolCmd; -import org.apache.cloudstack.api.command.admin.storage.SyncStoragePoolCmd; import org.apache.cloudstack.api.command.admin.swift.AddSwiftCmd; import org.apache.cloudstack.api.command.admin.swift.ListSwiftsCmd; import org.apache.cloudstack.api.command.admin.systemvm.DestroySystemVmCmd; From 63cbc88a888b87f67d3e77cdce3ddf9412cc3fe5 Mon Sep 17 00:00:00 2001 From: spaceman1984 Date: Tue, 13 Jul 2021 13:16:34 +0200 Subject: [PATCH 32/33] Removed duplicate config key --- .../src/main/java/com/cloud/server/ManagementServerImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/src/main/java/com/cloud/server/ManagementServerImpl.java b/server/src/main/java/com/cloud/server/ManagementServerImpl.java index 99a1ab91ec3b..34b5076c2687 100644 --- a/server/src/main/java/com/cloud/server/ManagementServerImpl.java +++ b/server/src/main/java/com/cloud/server/ManagementServerImpl.java @@ -747,7 +747,6 @@ public class ManagementServerImpl extends ManagerBase implements ManagementServe static final ConfigKey sshKeyLength = new ConfigKey("Advanced", Integer.class, "ssh.key.length", "2048", "Specifies custom SSH key length (bit)", true, ConfigKey.Scope.Global); static final ConfigKey humanReadableSizes = new ConfigKey("Advanced", Boolean.class, "display.human.readable.sizes", "true", "Enables outputting human readable byte sizes to logs and usage records.", false, ConfigKey.Scope.Global); public static final ConfigKey customCsIdentifier = new ConfigKey("Advanced", String.class, "custom.cs.identifier", UUID.randomUUID().toString().split("-")[0].substring(4), "Custom identifier for the cloudstack installation", true, ConfigKey.Scope.Global); - static final ConfigKey diskProvisioningStrictness = new ConfigKey("Storage", Boolean.class, "disk.provisioning.type.strictness", "false", "Use storage pools only with supported disk provisioning types for disk/service offerings.", true, ConfigKey.Scope.Zone); @Inject public AccountManager _accountMgr; @@ -3458,7 +3457,7 @@ public String getConfigComponentName() { @Override public ConfigKey[] getConfigKeys() { - return new ConfigKey[] {vmPasswordLength, sshKeyLength, humanReadableSizes, diskProvisioningStrictness, customCsIdentifier}; + return new ConfigKey[] {vmPasswordLength, sshKeyLength, humanReadableSizes, customCsIdentifier}; } protected class EventPurgeTask extends ManagedContextRunnable { From 9c55d287d5f230c7a829e4e257466f9cb3360d8e Mon Sep 17 00:00:00 2001 From: spaceman1984 Date: Fri, 16 Jul 2021 11:30:22 +0200 Subject: [PATCH 33/33] Revert "Added more info when a deployment fails" This reverts commit 2486db78dca8e034d8ad2386174dfb11004ce654. --- .../deploy/DeploymentPlanningManager.java | 6 ++--- .../cloud/vm/VirtualMachineManagerImpl.java | 24 ++++--------------- .../cloud/entity/api/VMEntityManagerImpl.java | 10 ++------ .../deploy/DeploymentPlanningManagerImpl.java | 14 +++++------ .../cloud/resource/ResourceManagerImpl.java | 11 +++------ .../vm/UnmanagedVMsManagerImpl.java | 9 ++----- .../DeploymentPlanningManagerImplTest.java | 6 ++--- 7 files changed, 23 insertions(+), 57 deletions(-) diff --git a/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java b/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java index 53dc2f02bec3..d3a5f7f73766 100644 --- a/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java +++ b/engine/components-api/src/main/java/com/cloud/deploy/DeploymentPlanningManager.java @@ -24,8 +24,6 @@ import com.cloud.vm.VirtualMachineProfile; import org.apache.cloudstack.framework.config.ConfigKey; -import java.util.List; - public interface DeploymentPlanningManager extends Manager { @@ -50,8 +48,8 @@ public interface DeploymentPlanningManager extends Manager { * * */ - DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, - DeploymentPlanner planner, List errors) throws InsufficientServerCapacityException, AffinityConflictException; + DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, + ExcludeList avoids, DeploymentPlanner planner) throws InsufficientServerCapacityException, AffinityConflictException; String finalizeReservation(DeployDestination plannedDestination, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, DeploymentPlanner planner) diff --git a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java index e8f60d29b152..eca0852060d8 100755 --- a/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1118,16 +1118,11 @@ public void orchestrateStart(final String vmUuid, final Map errors = new ArrayList<>(); try { - dest = _dpMgr.planDeployment(vmProfile, plan, avoids, planner, errors); + dest = _dpMgr.planDeployment(vmProfile, plan, avoids, planner); } catch (final AffinityConflictException e2) { s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2); - String errorMessage = "Unable to create deployment, affinity rules associted to the VM conflict"; - for (String error: errors) { - errorMessage += "\n" + errors; - } - throw new CloudRuntimeException(errorMessage); + throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict"); } if (dest == null) { @@ -3378,14 +3373,11 @@ private void orchestrateMigrateAway(final String vmUuid, final long srcHostId, f excludes.addHost(hostId); DeployDestination dest = null; - - ArrayList errors = new ArrayList(); - while (true) { try { plan.setMigrationPlan(true); - dest = _dpMgr.planDeployment(profile, plan, excludes, planner, errors); + dest = _dpMgr.planDeployment(profile, plan, excludes, planner); } catch (final AffinityConflictException e2) { s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2); throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict"); @@ -3399,13 +3391,7 @@ private void orchestrateMigrateAway(final String vmUuid, final long srcHostId, f if (s_logger.isDebugEnabled()) { s_logger.debug("Unable to find destination for migrating the vm " + profile); } - - String errorMessage = "Unable to find a server to migrate to."; - for (String error: errors){ - errorMessage += "\n" + error; - } - - throw new InsufficientServerCapacityException(errorMessage, host.getClusterId()); + throw new InsufficientServerCapacityException("Unable to find a server to migrate to.", host.getClusterId()); } excludes.addHost(dest.getHost().getId()); @@ -4286,7 +4272,7 @@ public void findHostAndMigrate(final String vmUuid, final Long newSvcOfferingId, DeployDestination dest = null; try { - dest = _dpMgr.planDeployment(profile, plan, excludes, null, new ArrayList<>()); + dest = _dpMgr.planDeployment(profile, plan, excludes, null); } catch (final AffinityConflictException e2) { s_logger.warn("Unable to create deployment, affinity rules associted to the VM conflict", e2); throw new CloudRuntimeException("Unable to create deployment, affinity rules associted to the VM conflict"); diff --git a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java index 59e7caebb3e7..02ffb37ba158 100644 --- a/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java +++ b/engine/orchestration/src/main/java/org/apache/cloudstack/engine/cloud/entity/api/VMEntityManagerImpl.java @@ -16,7 +16,6 @@ // under the License. package org.apache.cloudstack.engine.cloud.entity.api; -import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.UUID; @@ -200,9 +199,8 @@ public String reserveVirtualMachine(VMEntityVO vmEntityVO, DeploymentPlanner pla while (true) { DeployDestination dest = null; - List errors = new ArrayList<>(); try { - dest = _dpMgr.planDeployment(vmProfile, plan, exclude, plannerToUse, errors); + dest = _dpMgr.planDeployment(vmProfile, plan, exclude, plannerToUse); } catch (AffinityConflictException e) { throw new CloudRuntimeException("Unable to create deployment, affinity rules associated to the VM conflict"); } @@ -223,11 +221,7 @@ public String reserveVirtualMachine(VMEntityVO vmEntityVO, DeploymentPlanner pla // call retry it. return UUID.randomUUID().toString(); } else { - String errorMessage = "Unable to create a deployment for " + vmProfile; - for(String error: errors ){ - errorMessage += "\n" + error; - } - throw new InsufficientServerCapacityException(errorMessage, DataCenter.class, plan.getDataCenterId(), + throw new InsufficientServerCapacityException("Unable to create a deployment for " + vmProfile, DataCenter.class, plan.getDataCenterId(), areAffinityGroupsAssociated(vmProfile)); } } diff --git a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java index 073d5c23bb2f..afcaacf01436 100644 --- a/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java +++ b/server/src/main/java/com/cloud/deploy/DeploymentPlanningManagerImpl.java @@ -268,7 +268,7 @@ public void setAffinityGroupProcessors(List affinityProc } @Override - public DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, DeploymentPlanner planner, List errors) + public DeployDestination planDeployment(VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoids, DeploymentPlanner planner) throws InsufficientServerCapacityException, AffinityConflictException { ServiceOffering offering = vmProfile.getServiceOffering(); @@ -531,8 +531,9 @@ public DeployDestination planDeployment(VirtualMachineProfile vmProfile, Deploym avoids.getPoolsToAvoid()); resetAvoidSet(plannerAvoidOutput, plannerAvoidInput); - dest = checkClustersforDestination(clusterList, vmProfile, plan, avoids, dc, - getPlannerUsage(planner, vmProfile, plan, avoids), plannerAvoidOutput, errors); + + dest = + checkClustersforDestination(clusterList, vmProfile, plan, avoids, dc, getPlannerUsage(planner, vmProfile, plan, avoids), plannerAvoidOutput); if (dest != null) { return dest; } @@ -1169,7 +1170,7 @@ public void cleanupVMReservations() { // /refactoring planner methods private DeployDestination checkClustersforDestination(List clusterList, VirtualMachineProfile vmProfile, DeploymentPlan plan, ExcludeList avoid, DataCenter dc, - DeploymentPlanner.PlannerResourceUsage resourceUsageRequired, ExcludeList plannerAvoidOutput, List errors) { + DeploymentPlanner.PlannerResourceUsage resourceUsageRequired, ExcludeList plannerAvoidOutput) { if (s_logger.isTraceEnabled()) { s_logger.trace("ClusterId List to consider: " + clusterList); @@ -1180,7 +1181,6 @@ private DeployDestination checkClustersforDestination(List clusterList, Vi if (clusterVO.getHypervisorType() != vmProfile.getHypervisorType()) { s_logger.debug("Cluster: " + clusterId + " has HyperVisorType that does not match the VM, skipping this cluster"); - errors.add("Cluster: " + clusterId + " has HyperVisorType that does not match the VM, skipping this cluster"); avoid.addCluster(clusterVO.getId()); continue; } @@ -1230,11 +1230,9 @@ private DeployDestination checkClustersforDestination(List clusterList, Vi } } else { s_logger.debug("No suitable storagePools found under this Cluster: " + clusterId); - errors.add("No suitable storagePools found under Cluster: " + clusterId); } } else { - s_logger.debug("No suitable hosts found under Cluster: " + clusterId); - errors.add("No suitable hosts found under Cluster: " + clusterId); + s_logger.debug("No suitable hosts found under this Cluster: " + clusterId); } } diff --git a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java index 09f6f281e500..488be99e32da 100755 --- a/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java +++ b/server/src/main/java/com/cloud/resource/ResourceManagerImpl.java @@ -1334,16 +1334,11 @@ private void migrateAwayVmWithVolumes(HostVO host, VMInstanceVO vm) { final VirtualMachineProfile profile = new VirtualMachineProfileImpl(vm, null, offeringVO, null, null); plan.setMigrationPlan(true); DeployDestination dest = null; - ArrayList errors = new ArrayList<>(); try { - dest = deploymentManager.planDeployment(profile, plan, new DeploymentPlanner.ExcludeList(), null, errors); + dest = deploymentManager.planDeployment(profile, plan, new DeploymentPlanner.ExcludeList(), null); } catch (InsufficientServerCapacityException e) { - - String errorMessage = String.format("Maintenance failed, could not find deployment destination for VM [id=%s, name=%s].", vm.getId(), vm.getInstanceName()); - for (String error: errors) { - errorMessage += "\n" + error; - } - throw new CloudRuntimeException(errorMessage, e); + throw new CloudRuntimeException(String.format("Maintenance failed, could not find deployment destination for VM [id=%s, name=%s].", vm.getId(), vm.getInstanceName()), + e); } Host destHost = dest.getHost(); diff --git a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java index 4d5a5c80df71..501ca6328020 100644 --- a/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/vm/UnmanagedVMsManagerImpl.java @@ -770,15 +770,10 @@ private UserVm migrateImportedVM(HostVO sourceHost, VirtualMachineTemplate templ excludeList.addHost(sourceHost.getId()); final DataCenterDeployment plan = new DataCenterDeployment(sourceHost.getDataCenterId(), sourceHost.getPodId(), sourceHost.getClusterId(), null, null, null); DeployDestination dest = null; - ArrayList errors = new ArrayList<>(); try { - dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null, errors); + dest = deploymentPlanningManager.planDeployment(profile, plan, excludeList, null); } catch (Exception e) { - String errorMessage = String.format("VM import failed for unmanaged vm: %s during vm migration, finding deployment destination", vm.getInstanceName()); - for (String error: errors) { - errorMessage += "\n" + error; - } - LOGGER.warn(errorMessage, e); + LOGGER.warn(String.format("VM import failed for unmanaged vm: %s during vm migration, finding deployment destination", vm.getInstanceName()), e); cleanupFailedImportVM(vm); throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, String.format("VM import failed for unmanaged vm: %s during vm migration, finding deployment destination", vm.getInstanceName())); } diff --git a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java index ea40bfc0ae55..97e15de6f846 100644 --- a/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java +++ b/server/src/test/java/com/cloud/deploy/DeploymentPlanningManagerImplTest.java @@ -220,7 +220,7 @@ public void dataCenterAvoidTest() throws InsufficientServerCapacityException, Af DataCenterDeployment plan = new DataCenterDeployment(dataCenterId); Mockito.when(avoids.shouldAvoid((DataCenterVO)Matchers.anyObject())).thenReturn(true); - DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null, new ArrayList<>()); + DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); assertNull("DataCenter is in avoid set, destination should be null! ", dest); } @@ -236,7 +236,7 @@ public void plannerCannotHandleTest() throws InsufficientServerCapacityException Mockito.when(avoids.shouldAvoid((DataCenterVO)Matchers.anyObject())).thenReturn(false); Mockito.when(_planner.canHandle(vmProfile, plan, avoids)).thenReturn(false); - DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null, new ArrayList<>()); + DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); assertNull("Planner cannot handle, destination should be null! ", dest); } @@ -253,7 +253,7 @@ public void emptyClusterListTest() throws InsufficientServerCapacityException, A Mockito.when(_planner.canHandle(vmProfile, plan, avoids)).thenReturn(true); Mockito.when(((DeploymentClusterPlanner)_planner).orderClusters(vmProfile, plan, avoids)).thenReturn(null); - DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null, new ArrayList<>()); + DeployDestination dest = _dpm.planDeployment(vmProfile, plan, avoids, null); assertNull("Planner cannot handle, destination should be null! ", dest); }