From 839709d0c1b7d18493963f658ed340715e7e6f2a Mon Sep 17 00:00:00 2001 From: Liran Rotenberg Date: Tue, 28 Jun 2022 16:06:09 +0300 Subject: [PATCH] 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 --- .../org/ovirt/engine/core/bll/UpdateVmCommand.java | 12 +++++------- .../java/org/ovirt/engine/core/bll/VmHandler.java | 10 ---------- .../engine/core/common/errors/EngineMessage.java | 1 - .../org/ovirt/engine/core/dao/VmNumaNodeDao.java | 8 ++++++++ .../ovirt/engine/core/dao/VmNumaNodeDaoImpl.java | 7 +++++++ .../core/vdsbroker/monitoring/VmAnalyzer.java | 12 ++++++++++++ .../core/vdsbroker/monitoring/VmsMonitoring.java | 13 +++++++++++++ .../org/ovirt/engine/ui/frontend/AppErrors.java | 2 -- .../ovirt/engine/ui/frontend/AppErrors.properties | 1 - packaging/dbscripts/numa_sp.sql | 13 +++++++++++++ 10 files changed, 58 insertions(+), 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 index af0095446927..1abd80ea997a 100644 --- 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 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 index d7b698c7de75..fc0541d944c8 100644 --- 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/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java index e26716c2ee25..7016c01aecdf 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java +++ b/backend/manager/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 index 4ec24202647a..06f8979f0001 100644 --- 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 numaNodeIds); + + /** + * Remove all numa nodes based on the VM ID. + * + * @param vmIds + * a list of vm ids + */ + void massRemoveAllNumaNodeByVmId(List 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 index a15782a52701..cd2bf338b73b 100644 --- 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 numaNodes) { insertNumaNodeMap(numaNodes); } + @Override + public void massRemoveAllNumaNodeByVmId(List vmIds) { + getCallsHandler().executeStoredProcAsBatch("DeleteNumaNodeByVmId", vmIds.stream() + .map(vmId -> getCustomMapSqlParameterSource().addValue("vm_id", vmId)) + .collect(Collectors.toList())); + } + private Map> getAllNumaNodeCpuMap() { List> numaNodesCpus = diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmAnalyzer.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmAnalyzer.java index bc7bdc960d8d..42075d01050b 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmAnalyzer.java +++ b/backend/manager/modules/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 vmJobs; private VmStatistics statistics; private List 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/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmsMonitoring.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmsMonitoring.java index 5e62ee986aed..9eafc4561575 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/monitoring/VmsMonitoring.java +++ b/backend/manager/modules/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 vmAnalyzers) { saveVmStatistics(vmAnalyzers); saveVmInterfaceStatistics(vmAnalyzers); saveVmDiskImageStatistics(vmAnalyzers); + clearVmNuma(vmAnalyzers); } private void saveVmDiskImageStatistics(List vmAnalyzers) { @@ -326,6 +330,15 @@ private void saveVmStatistics(List vmAnalyzers) { }); } + private void clearVmNuma(List vmAnalyzers) { + List vmIds = vmAnalyzers.stream() + .map(VmAnalyzer::getClearNumaVmId) + .filter(Objects::nonNull) + .collect(Collectors.toList()); + + vmNumaNodeDao.massRemoveAllNumaNodeByVmId(vmIds); + } + protected void addUnmanagedVms(List vmAnalyzers, Guid vdsId) { List 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 index 45c9543b7e1c..8a922da46cdc 100644 --- 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/frontend/webadmin/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 660bc95f754b..b99fd4311bb6 100644 --- a/frontend/webadmin/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/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 index b0eb20f317c2..8c757abac799 100644 --- 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