core: block hot plug CPU with exclusive pinning

This patch prevent the user from hotplugging CPU when the VM is set with
exclusive pinning as we don't support it. Currently it will validated
only on the hot-plug command, which is less visible to the user.
With this patch, it won't be allowed and the user will be noticed.

Another addition is to consider within the validation the right pool of
shared CPUs to use.

Change-Id: Ie8d32b9e85879e6764bab63399c3d5762ba806db
Signed-off-by: Liran Rotenberg <lrotenbe@redhat.com>

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HotSetNumberOfCpusCommand.java

@@ -88,10 +88,6 @@ protected boolean validate() {
             valid = failValidation(EngineMessage.HOT_PLUG_CPU_CONFLICT,
                     String.format("%1$s", getVm().getCpuPinningPolicy().name()));
         }
-        if (getVm().getCpuPinningPolicy().isExclusive()) {
-            valid = failValidation(EngineMessage.HOT_PLUG_CPU_IS_NOT_SUPPORTED_DEDICATED,
-                    String.format("$policy %1$s", getVm().getCpuPinningPolicy().name()));
-        }
 
         return valid;
     }

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java

@@ -28,6 +28,8 @@
 import org.ovirt.engine.core.bll.quota.QuotaConsumptionParameter;
 import org.ovirt.engine.core.bll.quota.QuotaSanityParameter;
 import org.ovirt.engine.core.bll.quota.QuotaVdsDependent;
+import org.ovirt.engine.core.bll.scheduling.SlaValidator;
+import org.ovirt.engine.core.bll.scheduling.utils.VdsCpuUnitPinningHelper;
 import org.ovirt.engine.core.bll.snapshots.SnapshotVmConfigurationHelper;
 import org.ovirt.engine.core.bll.storage.domain.IsoDomainListSynchronizer;
 import org.ovirt.engine.core.bll.tasks.interfaces.CommandCallback;
@@ -189,6 +191,8 @@ public class UpdateVmCommand<T extends VmManagementParametersBase> extends VmMan
     private AffinityValidator affinityValidator;
     @Inject
     private SnapshotVmConfigurationHelper snapshotVmConfigurationHelper;
+    @Inject
+    private VdsCpuUnitPinningHelper vdsCpuUnitPinningHelper;
 
     private VM oldVm;
     private boolean quotaSanityOnly = false;
@@ -1195,8 +1199,14 @@ protected boolean validate() {
             return failValidation(EngineMessage.USE_HOST_CPU_REQUESTED_ON_UNSUPPORTED_ARCH);
         }
 
-        if (isHotSetEnabled() && !validateCPUHotplug(getParameters().getVmStaticData())) {
-            return failValidation(EngineMessage.CPU_HOTPLUG_TOPOLOGY_INVALID);
+        if (isHotSetEnabled() && getVm().isRunningOrPaused()) {
+            if (!validateCPUHotplug(getParameters().getVmStaticData())) {
+                return failValidation(EngineMessage.CPU_HOTPLUG_TOPOLOGY_INVALID);
+            }
+            if (!validateCpuPinningPolicyWithHotplug(getParameters().getVmStaticData())) {
+                return failValidation(EngineMessage.HOT_PLUG_CPU_IS_NOT_SUPPORTED_DEDICATED,
+                        String.format("$policy %1$s", getVm().getCpuPinningPolicy().name()));
+            }
         }
 
         if (!validateMemoryAlignment(getParameters().getVmStaticData())) {
@@ -1410,6 +1420,26 @@ && isHotSetEnabled()
         return true;
     }
 
+    private boolean validateCPUHotplug(VmStatic vmStaticData) {
+        // Can't set more CPUs than available on the host where VM is running.
+        boolean countThreadsAsCores = getCluster().getCountThreadsAsCores();
+        int availableCpus = SlaValidator.getEffectiveCpuCores(getVds(), countThreadsAsCores) -
+                vdsCpuUnitPinningHelper.countUnavailableCpus(
+                        getVdsManager(getVdsId()).getCpuTopology(),  countThreadsAsCores);
+        if (vmStaticData.getNumOfCpus(false) > availableCpus) {
+            return false;
+        }
+        return true;
+    }
+
+    private boolean validateCpuPinningPolicyWithHotplug(VmStatic vmStaticData) {
+        if (getVm().getCpuPinningPolicy().isExclusive()
+                && vmStaticData.getNumOfCpus(false) != getVm().getNumOfCpus(false)) {
+            return false;
+        }
+        return true;
+    }
+
     private boolean isIsoPathExists(VmStatic newVm, Guid storagePoolId) {
         if (StringUtils.isEmpty(newVm.getIsoPath())) {
             return true;

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmManagementCommandBase.java

@@ -115,19 +115,6 @@ protected boolean setAndValidateCpuProfile() {
                 getUserIdIfExternal().orElse(null)));
     }
 
-    protected boolean validateCPUHotplug(VmStatic vmStaticData) {
-        if (getVm().isRunningOrPaused()) {
-            // Can't set more CPUs than available on the host where VM is running.
-            // Potential overcommit (interference with other running VMs) will be resolved by scheduler.
-            // In alignment with the CPUPolicyUnit, VM's hyperthreading is not considered.
-            if (getVds() != null && vmStaticData.getNumOfCpus(false) > getVds().getCpuThreads()) {
-                return false;
-            }
-        }
-
-        return true;
-    }
-
     protected boolean validateMemoryAlignment(VmStatic vmStaticData) {
         if (getCluster().getArchitecture().getFamily() == ArchitectureType.ppc && vmStaticData.getMemSizeMb() % 256 != 0) {
             return failValidation(EngineMessage.MEMORY_SIZE_NOT_MULTIPLE_OF_256_ON_PPC,

backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/UpdateVmCommandTest.java

@@ -14,6 +14,7 @@
 import static org.ovirt.engine.core.common.errors.EngineMessage.ACTION_TYPE_FAILED_EDITING_HOSTED_ENGINE_IS_DISABLED;
 import static org.ovirt.engine.core.common.errors.EngineMessage.ACTION_TYPE_FAILED_VM_CANNOT_BE_HIGHLY_AVAILABLE_AND_HOSTED_ENGINE;
 
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
@@ -31,6 +32,7 @@
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
 import org.ovirt.engine.core.bll.numa.vm.NumaValidator;
+import org.ovirt.engine.core.bll.scheduling.utils.VdsCpuUnitPinningHelper;
 import org.ovirt.engine.core.bll.utils.VmDeviceUtils;
 import org.ovirt.engine.core.bll.validator.AffinityValidator;
 import org.ovirt.engine.core.bll.validator.InClusterUpgradeValidator;
@@ -52,6 +54,7 @@
 import org.ovirt.engine.core.common.businessentities.VDS;
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.VMStatus;
+import org.ovirt.engine.core.common.businessentities.VdsCpuUnit;
 import org.ovirt.engine.core.common.businessentities.VmDevice;
 import org.ovirt.engine.core.common.businessentities.VmStatic;
 import org.ovirt.engine.core.common.businessentities.storage.Disk;
@@ -77,6 +80,8 @@
 import org.ovirt.engine.core.utils.MockConfigDescriptor;
 import org.ovirt.engine.core.utils.MockConfigExtension;
 import org.ovirt.engine.core.utils.MockedConfig;
+import org.ovirt.engine.core.vdsbroker.ResourceManager;
+import org.ovirt.engine.core.vdsbroker.VdsManager;
 import org.ovirt.engine.core.vdsbroker.vdsbroker.CloudInitHandler;
 
 /** A test case for the {@link UpdateVmCommand}. */
@@ -145,6 +150,13 @@ public class UpdateVmCommandTest extends BaseCommandTest {
     private AffinityValidator affinityValidator;
     @Mock
     private VmNumaNodeDao vmNumaNodeDao;
+    @Mock
+    private VdsManager vdsManager;
+    @Mock
+    private ResourceManager resourceManager;
+    @Mock
+    private VdsCpuUnitPinningHelper vdsCpuUnitPinningHelper;
+
 
     private static Map<String, String> createMigrationMap() {
         Map<String, String> migrationMap = new HashMap<>();
@@ -263,6 +275,17 @@ public void setUp() {
         doReturn(quotaValidator).when(command).createQuotaValidator(any());
         doReturn(Boolean.TRUE).when(command).isSystemSuperUser();
 
+        List<VdsCpuUnit> cpuTopology = new ArrayList<>();
+        VDS vds = new VDS();
+        vds.setCpuCores(1);
+        vds.setId(Guid.newGuid());
+        command.setVds(vds);
+
+        when(vdsManager.getCpuTopology()).thenReturn(cpuTopology);
+        when(resourceManager.getVdsManager(vds.getId())).thenReturn(vdsManager);
+        when(resourceManager.getVdsManager(vds.getId(), false)).thenReturn(vdsManager);
+        when(vdsCpuUnitPinningHelper.countUnavailableCpus(cpuTopology, false)).thenReturn(0);
+
         command.init();
     }