core: reset numa configuration with resize and pin

When running a `Resize and Pin NUMA` VM, we create the virtual NUMA nodes including the pinning to the physical NUMA nodes of the host. This is a static configuration that being set on the VM. Until now, this configuration harmed many flows to update the VM configuration and a manual workaround needed to overcome it. Now the NUMA configuration will be deleted once the VM shutdown and when updating the VM, it will see the current configuration as there is no NUMA to that VM. Change-Id: I7d6e8200e7830dc6903a76ac219225fa359d2c53 Bug-Url: https://bugzilla.redhat.com/2074525 Signed-off-by: Liran Rotenberg <lrotenbe@redhat.com>
oVirt · Jul 21, 2022 · 839709d · 839709d
1 parent 9336b4f
commit 839709d
Show file tree

Hide file tree

Showing 10 changed files with 58 additions and 21 deletions.
diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
@@ -60,7 +60,6 @@
 import org.ovirt.engine.core.common.businessentities.ActionGroup;
 import org.ovirt.engine.core.common.businessentities.ArchitectureType;
 import org.ovirt.engine.core.common.businessentities.BiosType;
-import org.ovirt.engine.core.common.businessentities.CpuPinningPolicy;
 import org.ovirt.engine.core.common.businessentities.DisplayType;
 import org.ovirt.engine.core.common.businessentities.GraphicsDevice;
 import org.ovirt.engine.core.common.businessentities.GraphicsType;
@@ -953,15 +952,14 @@ private void updateVmNumaNodes() {
     }
 
     private void initNuma() {
+        if (VmCpuCountHelper.isResizeAndPinPolicy(getParameters().getVm())) {
+            getParameters().setUpdateNuma(false);
+        }
         List<VmNumaNode> vNumaNodeList = vmNumaNodeDao.getAllVmNumaNodeByVmId(getParameters().getVmId());
         if (getVm() != null) {
             getVm().setvNumaNodeList(vNumaNodeList);
         }
 
-        if (getParameters().getVm().getCpuPinningPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA) {
-            getParameters().setUpdateNuma(false);
-        }
-
         if (getParameters().isUpdateNuma() == null) {
             getParameters().setUpdateNuma(!vNumaNodeList.equals(getParameters().getVm().getvNumaNodeList()));
         }
@@ -1294,7 +1292,8 @@ protected boolean validate() {
             return failValidation(EngineMessage.TPM_DEVICE_REQUIRED_BY_OS);
         }
 
-        if (!validate(getNumaValidator().checkVmNumaNodesIntegrity(
+        if (!VmCpuCountHelper.isResizeAndPinPolicy(getParameters().getVm()) &&
+                !validate(getNumaValidator().checkVmNumaNodesIntegrity(
                 getParameters().getVm(),
                 getParameters().getVm().getvNumaNodeList()))) {
             return false;
@@ -1413,7 +1412,6 @@ && isHotSetEnabled()
             return failValidation(EngineMessage.ERROR_CANNOT_FIND_ISO_IMAGE_PATH);
         }
         if (!validate(vmHandler.validateCpuPinningPolicy(getParameters().getVmStaticData(),
-                getVm().getStaticData(),
                 getEffectiveCompatibilityVersion()))) {
             return false;
         }

diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
@@ -1640,10 +1640,6 @@ private VmNameValidator getVmNameValidator(OriginType originType) {
     }
 
     public ValidationResult validateCpuPinningPolicy(VmBase vmFromParams, Version version) {
-        return validateCpuPinningPolicy(vmFromParams, null, version);
-    }
-
-    public ValidationResult validateCpuPinningPolicy(VmBase vmFromParams, VmBase vmFromDb, Version version) {
         ValidationResult result = ValidationResult.VALID;
 
         switch (vmFromParams.getCpuPinningPolicy()) {
@@ -1653,12 +1649,6 @@ public ValidationResult validateCpuPinningPolicy(VmBase vmFromParams, VmBase vmF
             }
             break;
         case RESIZE_AND_PIN_NUMA:
-            if (vmFromDb != null
-                    && (vmFromDb.getNumOfCpus() != vmFromParams.getNumOfCpus()
-                            || !vmFromDb.getvNumaNodeList().equals(vmFromParams.getvNumaNodeList()))) {
-                result = new ValidationResult(EngineMessage.ACTION_TYPE_CANNOT_RESIZE_AND_PIN_AND_NUMA_SET);
-                break;
-            }
             boolean singleCoreHostFound = vmFromParams.getDedicatedVmForVdsList()
                     .stream()
                     .map(vdsId -> vdsDynamicDao.get(vdsId))

diff --git a/...nager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java b/...nager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java
@@ -1737,7 +1737,6 @@ public enum EngineMessage {
 
     ACTION_TYPE_CANNOT_SET_MANUAL_PINNING(ErrorType.BAD_PARAMETERS),
     ACTION_TYPE_CANNOT_RESIZE_AND_PIN_SINGLE_CORE(ErrorType.CONSTRAINT_VIOLATION),
-    ACTION_TYPE_CANNOT_RESIZE_AND_PIN_AND_NUMA_SET(ErrorType.CONFLICT),
     ACTION_TYPE_FAILED_DEDICATED_IS_NOT_SUPPORTED(ErrorType.NOT_SUPPORTED),
     ACTION_TYPE_CANNOT_SET_DEDICATED_PINNING_WITH_NUMA_NODES_PINNED(ErrorType.CONFLICT),
 

diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDao.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDao.java
@@ -53,4 +53,12 @@ public interface VmNumaNodeDao extends Dao {
      *            the numa node ids to be removed
      */
     void massRemoveNumaNodeByNumaNodeId(List<Guid> numaNodeIds);
+
+    /**
+     * Remove all numa nodes based on the VM ID.
+     *
+     * @param vmIds
+     *            a list of vm ids
+     */
+    void massRemoveAllNumaNodeByVmId(List<Guid> vmIds);
 }
diff --git a/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDaoImpl.java b/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VmNumaNodeDaoImpl.java
@@ -87,6 +87,13 @@ public void massUpdateNumaNode(List<VmNumaNode> numaNodes) {
         insertNumaNodeMap(numaNodes);
     }
 
+    @Override
+    public void massRemoveAllNumaNodeByVmId(List<Guid> vmIds) {
+        getCallsHandler().executeStoredProcAsBatch("DeleteNumaNodeByVmId", vmIds.stream()
+                .map(vmId -> getCustomMapSqlParameterSource().addValue("vm_id", vmId))
+                .collect(Collectors.toList()));
+    }
+
 
     private Map<Guid, List<Integer>> getAllNumaNodeCpuMap() {
         List<Pair<Guid, Integer>> numaNodesCpus =

diff --git a/...odules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmAnalyzer.java b/...odules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmAnalyzer.java
@@ -22,6 +22,7 @@
 
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.action.SaveVmExternalDataParameters;
+import org.ovirt.engine.core.common.businessentities.CpuPinningPolicy;
 import org.ovirt.engine.core.common.businessentities.OriginType;
 import org.ovirt.engine.core.common.businessentities.UnchangeableByVdsm;
 import org.ovirt.engine.core.common.businessentities.VDSStatus;
@@ -85,6 +86,7 @@ public class VmAnalyzer {
     private boolean vmBalloonDriverNotRequestedOrAvailable;
     private boolean guestAgentDownAndBalloonInfalted;
     private boolean guestAgentUpOrBalloonDeflated;
+    private boolean numaClear;
     private List<VmJob> vmJobs;
     private VmStatistics statistics;
     private List<VmNetworkInterface> ifaces;
@@ -429,6 +431,7 @@ private void clearVm(VmExitStatus exitStatus, String exitMessage, VmExitReason v
                         alreadyDown ? dbVm.getExitMessage() : exitMessage,
                         alreadyDown ? dbVm.getExitReason() : vmExitReason);
                 dbVm.setStopReason(getVmManager().getStopReason(getVmId()));
+                numaClear = true;
             }
             saveDynamic(dbVm);
             resetVmStatistics();
@@ -476,6 +479,7 @@ private void abortVmMigration(VmExitStatus exitStatus, String exitMessage, VmExi
         auditVmMigrationAbort(exitMessage);
         resourceManager.removeAsyncRunningVm(dbVm.getId());
         movedToDown = true;
+        numaClear = true;
     }
 
     private void auditVmMigrationAbort(String exitMessage) {
@@ -1129,6 +1133,14 @@ public boolean isRemoveFromAsync() {
         return removeFromAsync;
     }
 
+    public Guid getClearNumaVmId() {
+        return numaClear && isResizeAndPin() ? getVmId() : null;
+    }
+
+    private boolean isResizeAndPin() {
+        return getVmManager().getCpuPinningPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA;
+    }
+
     protected VmManager getVmManager() {
         return resourceManager.getVmManager(dbVm.getId());
     }

diff --git a/...les/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmsMonitoring.java b/...les/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmsMonitoring.java
@@ -22,6 +22,7 @@
 import org.ovirt.engine.core.dao.VdsDynamicDao;
 import org.ovirt.engine.core.dao.VmDynamicDao;
 import org.ovirt.engine.core.dao.VmGuestAgentInterfaceDao;
+import org.ovirt.engine.core.dao.VmNumaNodeDao;
 import org.ovirt.engine.core.dao.VmStatisticsDao;
 import org.ovirt.engine.core.dao.network.VmNetworkInterfaceDao;
 import org.ovirt.engine.core.dao.network.VmNetworkStatisticsDao;
@@ -65,6 +66,8 @@ public class VmsMonitoring {
     private VmNetworkInterfaceDao vmNetworkInterfaceDao;
     @Inject
     private VdsDynamicDao vdsDynamicDao;
+    @Inject
+    private VmNumaNodeDao vmNumaNodeDao;
 
     private static final Logger log = LoggerFactory.getLogger(VmsMonitoring.class);
 
@@ -289,6 +292,7 @@ private void flush(List<VmAnalyzer> vmAnalyzers) {
         saveVmStatistics(vmAnalyzers);
         saveVmInterfaceStatistics(vmAnalyzers);
         saveVmDiskImageStatistics(vmAnalyzers);
+        clearVmNuma(vmAnalyzers);
     }
 
     private void saveVmDiskImageStatistics(List<VmAnalyzer> vmAnalyzers) {
@@ -326,6 +330,15 @@ private void saveVmStatistics(List<VmAnalyzer> vmAnalyzers) {
         });
     }
 
+    private void clearVmNuma(List<VmAnalyzer> vmAnalyzers) {
+        List<Guid> vmIds = vmAnalyzers.stream()
+                .map(VmAnalyzer::getClearNumaVmId)
+                .filter(Objects::nonNull)
+                .collect(Collectors.toList());
+
+        vmNumaNodeDao.massRemoveAllNumaNodeByVmId(vmIds);
+    }
+
     protected void addUnmanagedVms(List<VmAnalyzer> vmAnalyzers, Guid vdsId) {
         List<Guid> unmanagedVmIds = vmAnalyzers.stream()
                 .filter(VmAnalyzer::isUnmanagedVm)

diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
@@ -3191,8 +3191,6 @@ public interface AppErrors extends ConstantsWithLookup {
 
     String ACTION_TYPE_CANNOT_RESIZE_AND_PIN_SINGLE_CORE();
 
-    String ACTION_TYPE_CANNOT_RESIZE_AND_PIN_AND_NUMA_SET();
-
     String ACTION_TYPE_FAILED_DEDICATED_IS_NOT_SUPPORTED();
 
     String ACTION_TYPE_CANNOT_SET_DEDICATED_PINNING_WITH_NUMA_NODES_PINNED();

diff --git a/...min/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/...min/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -1678,7 +1678,6 @@ CANNOT_START_BACKUP_USING_OUTDATED_HOST=Cannot ${action} ${type}. Host (${vdsNam
 ACTION_TYPE_CANNOT_CHANGE_INITIAL_RUN_DATA=Cannot ${action} ${type}. The Initial Run Data cannot be modified for Stateless Pool Virtual Machines.
 ACTION_TYPE_CANNOT_SET_MANUAL_PINNING=Cannot ${action} ${type}. The CPU Pinning policy is set to 'Manual' but no cpu pinning string is provided.
 ACTION_TYPE_CANNOT_RESIZE_AND_PIN_SINGLE_CORE=Cannot ${action} ${type}. The CPU Pinning policy 'Resize and Pin NUMA' cannot be applied to a VM that is pinned to a single core host.
-ACTION_TYPE_CANNOT_RESIZE_AND_PIN_AND_NUMA_SET=Cannot ${action} ${type}. The VM NUMA configuration cannot be applied when using the CPU Pinning policy 'Resize and Pin NUMA'.
 ACTION_TYPE_FAILED_DEDICATED_IS_NOT_SUPPORTED=Cannot ${action} ${type}. The CPU Pinning policy 'Dedicated' requires cluster compatibility version 4.7 or higher.
 ACTION_TYPE_CANNOT_SET_DEDICATED_PINNING_WITH_NUMA_NODES_PINNED=Cannot ${action} ${type}. The VM NUMA nodes cannot be manually pinned when using the CPU Pinning policy 'Dedicated'.
 ACTION_TYPE_FAILED_VM_BACKUP_NOT_EXIST=Cannot ${action} ${type}. The specified VM backup does not exist.

diff --git a/packaging/dbscripts/numa_sp.sql b/packaging/dbscripts/numa_sp.sql
@@ -130,6 +130,19 @@ BEGIN
 END;$PROCEDURE$
 LANGUAGE plpgsql;
 
+CREATE OR REPLACE FUNCTION DeleteNumaNodeByVmId (v_vm_id UUID)
+RETURNS VOID AS $PROCEDURE$
+BEGIN
+    BEGIN
+        DELETE
+        FROM numa_node
+        WHERE vm_id = v_vm_id;
+    END;
+
+    RETURN;
+END;$PROCEDURE$
+LANGUAGE plpgsql;
+
 CREATE OR REPLACE FUNCTION GetNumaNodeByVdsId (v_vds_id UUID)
 RETURNS SETOF numa_node_cpus_view STABLE AS $PROCEDURE$
 BEGIN