This is an automated email from the ASF dual-hosted git repository. dahn pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudstack.git
The following commit(s) were added to refs/heads/main by this push: new d04d60b0797 [VMWare] Limit IOPS in Compute/Disk Offerings (#6386) d04d60b0797 is described below commit d04d60b0797aadb33bb29cbc73eb490a6299ac41 Author: SadiJr <sadijaci...@gmail.com> AuthorDate: Tue Jan 17 10:41:56 2023 -0300 [VMWare] Limit IOPS in Compute/Disk Offerings (#6386) --- .../hypervisor/vmware/resource/VmwareResource.java | 5 +- .../storage/resource/VmwareStorageProcessor.java | 7 +-- utils/src/main/java/com/cloud/utils/LogUtils.java | 24 ++++++++++ .../test/java/com/cloud/utils/LogUtilsTest.java | 39 +++++++++++++++ .../com/cloud/hypervisor/vmware/mo/HostMO.java | 2 + .../hypervisor/vmware/mo/HypervisorHostHelper.java | 4 +- .../hypervisor/vmware/mo/VirtualMachineMO.java | 9 +++- .../cloud/hypervisor/vmware/util/VmwareHelper.java | 21 ++++++-- .../hypervisor/vmware/util/VmwareHelperTest.java | 56 ++++++++++++++++++++++ 9 files changed, 155 insertions(+), 12 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 aabebf56bd1..5be98d4c43c 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 @@ -51,6 +51,7 @@ import com.cloud.agent.api.PatchSystemVmAnswer; import com.cloud.agent.api.PatchSystemVmCommand; import com.cloud.resource.ServerResourceBase; import com.cloud.utils.FileUtil; +import com.cloud.utils.LogUtils; import com.cloud.utils.validation.ChecksumUtil; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.storage.command.CopyCommand; @@ -2431,7 +2432,9 @@ public class VmwareResource extends ServerResourceBase implements StoragePoolRes scsiUnitNumber++; } - VirtualDevice device = VmwareHelper.prepareDiskDevice(vmMo, null, controllerKey, diskChain, volumeDsDetails.first(), deviceNumber, i + 1); + Long maxIops = volumeTO.getIopsWriteRate() + volumeTO.getIopsReadRate(); + VirtualDevice device = VmwareHelper.prepareDiskDevice(vmMo, null, controllerKey, diskChain, volumeDsDetails.first(), deviceNumber, i + 1, maxIops); + s_logger.debug(LogUtils.logGsonWithoutException("The following definitions will be used to start the VM: virtual device [%s], volume [%s].", device, volumeTO)); diskStoragePolicyId = volumeTO.getvSphereStoragePolicyId(); if (StringUtils.isNotEmpty(diskStoragePolicyId)) { 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 403daf3d4b0..444008f167e 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 @@ -50,8 +50,8 @@ import org.apache.cloudstack.storage.command.ResignatureAnswer; import org.apache.cloudstack.storage.command.ResignatureCommand; import org.apache.cloudstack.storage.command.SnapshotAndCopyAnswer; import org.apache.cloudstack.storage.command.SnapshotAndCopyCommand; -import org.apache.cloudstack.storage.command.SyncVolumePathCommand; import org.apache.cloudstack.storage.command.SyncVolumePathAnswer; +import org.apache.cloudstack.storage.command.SyncVolumePathCommand; import org.apache.cloudstack.storage.to.PrimaryDataStoreTO; import org.apache.cloudstack.storage.to.SnapshotObjectTO; import org.apache.cloudstack.storage.to.TemplateObjectTO; @@ -97,6 +97,7 @@ import com.cloud.storage.StorageLayer; import com.cloud.storage.Volume; import com.cloud.storage.template.OVAProcessor; import com.cloud.template.TemplateManager; +import com.cloud.utils.LogUtils; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.exception.CloudRuntimeException; @@ -2128,7 +2129,7 @@ public class VmwareStorageProcessor implements StorageProcessor { diskController = vmMo.getRecommendedDiskController(null); } - vmMo.attachDisk(new String[] { datastoreVolumePath }, morDs, diskController, storagePolicyId); + vmMo.attachDisk(new String[] { datastoreVolumePath }, morDs, diskController, storagePolicyId, volumeTO.getIopsReadRate() + volumeTO.getIopsWriteRate()); VirtualMachineDiskInfoBuilder diskInfoBuilder = vmMo.getDiskInfoBuilder(); VirtualMachineDiskInfo diskInfo = diskInfoBuilder.getDiskInfoByBackingFileBaseName(volumePath, dsMo.getName()); chainInfo = _gson.toJson(diskInfo); @@ -2409,7 +2410,7 @@ public class VmwareStorageProcessor implements StorageProcessor { @Override public Answer createVolume(CreateObjectCommand cmd) { - + s_logger.debug(LogUtils.logGsonWithoutException("Executing CreateObjectCommand cmd: [%s].", cmd)); VolumeObjectTO volume = (VolumeObjectTO)cmd.getData(); DataStoreTO primaryStore = volume.getDataStore(); String vSphereStoragePolicyId = volume.getvSphereStoragePolicyId(); diff --git a/utils/src/main/java/com/cloud/utils/LogUtils.java b/utils/src/main/java/com/cloud/utils/LogUtils.java index a7e325794ce..edfb53fabf5 100644 --- a/utils/src/main/java/com/cloud/utils/LogUtils.java +++ b/utils/src/main/java/com/cloud/utils/LogUtils.java @@ -20,8 +20,10 @@ package com.cloud.utils; import java.io.File; +import java.util.ArrayList; import java.util.Enumeration; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.apache.log4j.Appender; @@ -29,8 +31,11 @@ import org.apache.log4j.FileAppender; import org.apache.log4j.Logger; import org.apache.log4j.xml.DOMConfigurator; +import com.google.gson.Gson; + public class LogUtils { public static final Logger LOGGER = Logger.getLogger(LogUtils.class); + private static final Gson GSON = new Gson(); private static String configFileLocation = null; @@ -72,4 +77,23 @@ public class LogUtils { } return fileNames; } + + public static String logGsonWithoutException(String formatMessage, Object ... objects) { + List<String> gsons = new ArrayList<>(); + for (Object object : objects) { + try { + gsons.add(GSON.toJson(object)); + } catch (Exception e) { + LOGGER.debug(String.format("Failed to log object [%s] using GSON.", object != null ? object.getClass().getSimpleName() : "null")); + gsons.add("error to decode"); + } + } + try { + return String.format(formatMessage, gsons.toArray()); + } catch (Exception e) { + String errorMsg = String.format("Failed to log objects using GSON due to: [%s].", e.getMessage()); + LOGGER.error(errorMsg, e); + return errorMsg; + } + } } diff --git a/utils/src/test/java/com/cloud/utils/LogUtilsTest.java b/utils/src/test/java/com/cloud/utils/LogUtilsTest.java new file mode 100644 index 00000000000..350c9e946cd --- /dev/null +++ b/utils/src/test/java/com/cloud/utils/LogUtilsTest.java @@ -0,0 +1,39 @@ +// 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.utils; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +public class LogUtilsTest { + + @Test + public void logGsonWithoutExceptionTestLogCorrectlyPrimitives() { + String expected = "test primitives: int [1], double [1.11], float [1.2222], boolean [true], null [], char [\"c\"]."; + String log = LogUtils.logGsonWithoutException("test primitives: int [%s], double [%s], float [%s], boolean [%s], null [%s], char [%s].", + 1, 1.11d, 1.2222f, true, null, 'c'); + assertEquals(expected, log); + } + + @Test + public void logGsonWithoutExceptionTestPassWrongNumberOfArgs() { + String expected = "Failed to log objects using GSON due to: [Format specifier '%s']."; + String result = LogUtils.logGsonWithoutException("teste wrong [%s] %s args.", "blablabla"); + assertEquals(expected, result); + } +} diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java index 07bead77c1f..15d80fc79e4 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HostMO.java @@ -69,6 +69,7 @@ import com.vmware.vim25.VirtualMachineConfigSpec; import com.vmware.vim25.VirtualNicManagerNetConfig; import com.cloud.hypervisor.vmware.util.VmwareContext; import com.cloud.hypervisor.vmware.util.VmwareHelper; +import com.cloud.utils.LogUtils; import com.cloud.utils.Pair; public class HostMO extends BaseMO implements VmwareHypervisorHost { @@ -643,6 +644,7 @@ public class HostMO extends BaseMO implements VmwareHypervisorHost { @Override public boolean createVm(VirtualMachineConfigSpec vmSpec) throws Exception { assert (vmSpec != null); + s_logger.debug(LogUtils.logGsonWithoutException("Creating VM with configuration: [%s].", vmSpec)); DatacenterMO dcMo = new DatacenterMO(_context, getHyperHostDatacenter()); ManagedObjectReference morPool = getHyperHostOwnerResourcePool(); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java index 39a47b75de6..5cc88992071 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/mo/HypervisorHostHelper.java @@ -60,6 +60,7 @@ import com.cloud.network.Networks.BroadcastDomainType; import com.cloud.offering.NetworkOffering; import com.cloud.storage.Storage.StoragePoolType; import com.cloud.utils.ActionDelegate; +import com.cloud.utils.LogUtils; import com.cloud.utils.NumbersUtil; import com.cloud.utils.Pair; import com.cloud.utils.cisco.n1kv.vsm.NetconfHelper; @@ -1622,6 +1623,7 @@ public class HypervisorHostHelper { DatacenterMO dataCenterMo = new DatacenterMO(host.getContext(), host.getHyperHostDatacenter()); setVMHardwareVersion(vmConfig, clusterMo, dataCenterMo); + s_logger.debug(LogUtils.logGsonWithoutException("Creating blank VM with configuration [%s].", vmConfig)); if (host.createVm(vmConfig)) { // Here, when attempting to find the VM, we need to use the name // with which we created it. This is the only such place where @@ -2128,7 +2130,7 @@ public class HypervisorHostHelper { VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); // Reconfigure worker VM with datadisk - VirtualDevice device = VmwareHelper.prepareDiskDevice(workerVmMo, null, -1, disks, morDs, -1, 1); + VirtualDevice device = VmwareHelper.prepareDiskDevice(workerVmMo, null, -1, disks, morDs, -1, 1, null); deviceConfigSpec.setDevice(device); deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD); vmConfigSpec.getDeviceChange().add(deviceConfigSpec); 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 d26607d8209..cecdb4700dd 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 @@ -52,6 +52,7 @@ import com.cloud.hypervisor.vmware.mo.SnapshotDescriptor.SnapshotInfo; import com.cloud.hypervisor.vmware.util.VmwareContext; import com.cloud.hypervisor.vmware.util.VmwareHelper; import com.cloud.utils.ActionDelegate; +import com.cloud.utils.LogUtils; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.concurrency.NamedThreadFactory; @@ -1395,7 +1396,10 @@ public class VirtualMachineMO extends BaseMO { } public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference morDs, String diskController, String vSphereStoragePolicyId) throws Exception { + attachDisk(vmdkDatastorePathChain, morDs, diskController, vSphereStoragePolicyId, null); + } + public void attachDisk(String[] vmdkDatastorePathChain, ManagedObjectReference morDs, String diskController, String vSphereStoragePolicyId, Long maxIops) throws Exception { if(s_logger.isTraceEnabled()) s_logger.trace("vCenter API trace - attachDisk(). target MOR: " + _mor.getValue() + ", vmdkDatastorePath: " + GSON.toJson(vmdkDatastorePathChain) + ", datastore: " + morDs.getValue()); @@ -1425,7 +1429,7 @@ public class VirtualMachineMO extends BaseMO { } synchronized (_mor.getValue().intern()) { - VirtualDevice newDisk = VmwareHelper.prepareDiskDevice(this, null, controllerKey, vmdkDatastorePathChain, morDs, unitNumber, 1); + VirtualDevice newDisk = VmwareHelper.prepareDiskDevice(this, null, controllerKey, vmdkDatastorePathChain, morDs, unitNumber, 1, maxIops); if (StringUtils.isNotBlank(diskController)) { String vmdkFileName = vmdkDatastorePathChain[0]; updateVmdkAdapter(vmdkFileName, diskController); @@ -2086,7 +2090,7 @@ public class VirtualMachineMO extends BaseMO { VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec(); VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); - VirtualDevice device = VmwareHelper.prepareDiskDevice(clonedVmMo, null, -1, disks, morDs, -1, 1); + VirtualDevice device = VmwareHelper.prepareDiskDevice(clonedVmMo, null, -1, disks, morDs, -1, 1, null); deviceConfigSpec.setDevice(device); deviceConfigSpec.setOperation(VirtualDeviceConfigSpecOperation.ADD); @@ -2120,6 +2124,7 @@ public class VirtualMachineMO extends BaseMO { } public void plugDevice(VirtualDevice device) throws Exception { + s_logger.debug(LogUtils.logGsonWithoutException("Pluging device [%s] to VM [%s].", device, getVmName())); VirtualMachineConfigSpec vmConfigSpec = new VirtualMachineConfigSpec(); VirtualDeviceConfigSpec deviceConfigSpec = new VirtualDeviceConfigSpec(); deviceConfigSpec.setDevice(device); diff --git a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java index 14ea3771cd4..4a81beeff98 100644 --- a/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java +++ b/vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareHelper.java @@ -48,6 +48,7 @@ import com.cloud.hypervisor.vmware.mo.LicenseAssignmentManagerMO; import com.cloud.hypervisor.vmware.mo.VirtualEthernetCardType; import com.cloud.hypervisor.vmware.mo.VirtualMachineMO; import com.cloud.hypervisor.vmware.mo.VmwareHypervisorHost; +import com.cloud.utils.LogUtils; import com.cloud.utils.Pair; import com.cloud.utils.Ternary; import com.cloud.utils.exception.ExceptionUtil; @@ -61,6 +62,7 @@ import com.vmware.vim25.OptionValue; import com.vmware.vim25.PerfCounterInfo; import com.vmware.vim25.PerfMetricId; import com.vmware.vim25.ResourceAllocationInfo; +import com.vmware.vim25.StorageIOAllocationInfo; import com.vmware.vim25.VirtualCdrom; import com.vmware.vim25.VirtualCdromIsoBackingInfo; import com.vmware.vim25.VirtualCdromRemotePassthroughBackingInfo; @@ -87,7 +89,6 @@ import com.vmware.vim25.VirtualVmxnet2; import com.vmware.vim25.VirtualVmxnet3; public class VmwareHelper { - @SuppressWarnings("unused") private static final Logger s_logger = Logger.getLogger(VmwareHelper.class); public static final int MAX_SCSI_CONTROLLER_COUNT = 4; @@ -141,7 +142,6 @@ public class VmwareHelper { public static VirtualDevice prepareNicOpaque(VirtualMachineMO vmMo, VirtualEthernetCardType deviceType, String portGroupName, String macAddress, int contextNumber, boolean connected, boolean connectOnStart) throws Exception { - assert(vmMo.getRunningHost().hasOpaqueNSXNetwork()); VirtualEthernetCard nic = createVirtualEthernetCard(deviceType); @@ -215,8 +215,10 @@ public class VmwareHelper { // vmdkDatastorePath: [datastore name] vmdkFilePath public static VirtualDevice prepareDiskDevice(VirtualMachineMO vmMo, VirtualDisk device, int controllerKey, String vmdkDatastorePathChain[], - ManagedObjectReference morDs, int deviceNumber, int contextNumber) throws Exception { - + ManagedObjectReference morDs, int deviceNumber, int contextNumber, Long maxIops) throws Exception { + s_logger.debug(LogUtils.logGsonWithoutException("Trying to prepare disk device to virtual machine [%s], using the following details: Virtual device [%s], " + + "ManagedObjectReference [%s], ControllerKey [%s], VMDK path chain [%s], DeviceNumber [%s], ContextNumber [%s] and max IOPS [%s].", + vmMo, device, morDs, controllerKey, vmdkDatastorePathChain, deviceNumber, contextNumber, maxIops)); assert (vmdkDatastorePathChain != null); assert (vmdkDatastorePathChain.length >= 1); @@ -243,6 +245,13 @@ public class VmwareHelper { disk.setKey(-contextNumber); disk.setUnitNumber(deviceNumber); + if (maxIops != null && maxIops > 0) { + s_logger.debug(LogUtils.logGsonWithoutException("Defining [%s] as the max IOPS of disk [%s].", maxIops, disk)); + StorageIOAllocationInfo storageIOAllocationInfo = new StorageIOAllocationInfo(); + storageIOAllocationInfo.setLimit(maxIops); + disk.setStorageIOAllocation(storageIOAllocationInfo); + } + VirtualDeviceConnectInfo connectInfo = new VirtualDeviceConnectInfo(); connectInfo.setConnected(true); connectInfo.setStartConnected(true); @@ -255,6 +264,9 @@ public class VmwareHelper { setParentBackingInfo(backingInfo, morDs, parentDisks); } + s_logger.debug(LogUtils.logGsonWithoutException("Prepared disk device, to attach to virtual machine [%s], has the following details: Virtual device [%s], " + + "ManagedObjectReference [%s], ControllerKey [%s], VMDK path chain [%s], DeviceNumber [%s], ContextNumber [%s] and max IOPS [%s], is: [%s].", + vmMo, device, morDs, controllerKey, vmdkDatastorePathChain, deviceNumber, contextNumber, maxIops, disk)); return disk; } @@ -750,5 +762,4 @@ public class VmwareHelper { } return host; } - } diff --git a/vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareHelperTest.java b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareHelperTest.java new file mode 100644 index 00000000000..4417748efbf --- /dev/null +++ b/vmware-base/src/test/java/com/cloud/hypervisor/vmware/util/VmwareHelperTest.java @@ -0,0 +1,56 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package com.cloud.hypervisor.vmware.util; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.MockitoJUnitRunner; + +import com.cloud.hypervisor.vmware.mo.VirtualMachineMO; +import com.vmware.vim25.VirtualDisk; + +@RunWith(MockitoJUnitRunner.class) +public class VmwareHelperTest { + @Mock + private VirtualMachineMO virtualMachineMO; + + @Test + public void prepareDiskDeviceTestNotLimitingIOPS() throws Exception { + Mockito.when(virtualMachineMO.getIDEDeviceControllerKey()).thenReturn(1); + VirtualDisk virtualDisk = (VirtualDisk) VmwareHelper.prepareDiskDevice(virtualMachineMO, null, -1, new String[1], null, 0, 0, null); + assertEquals(null, virtualDisk.getStorageIOAllocation()); + } + + @Test + public void prepareDiskDeviceTestLimitingIOPS() throws Exception { + Mockito.when(virtualMachineMO.getIDEDeviceControllerKey()).thenReturn(1); + VirtualDisk virtualDisk = (VirtualDisk) VmwareHelper.prepareDiskDevice(virtualMachineMO, null, -1, new String[1], null, 0, 0, Long.valueOf(1000)); + assertEquals(Long.valueOf(1000), virtualDisk.getStorageIOAllocation().getLimit()); + } + + @Test + public void prepareDiskDeviceTestLimitingIOPSToZero() throws Exception { + Mockito.when(virtualMachineMO.getIDEDeviceControllerKey()).thenReturn(1); + VirtualDisk virtualDisk = (VirtualDisk) VmwareHelper.prepareDiskDevice(virtualMachineMO, null, -1, new String[1], null, 0, 0, Long.valueOf(0)); + assertEquals(null, virtualDisk.getStorageIOAllocation()); + } +}