Dudi Maroshi has uploaded a new change for review. Change subject: engine: validate (vm NUMA nodes) <= (vm CPU cores) ......................................................................
engine: validate (vm NUMA nodes) <= (vm CPU cores) Add validation in canDoAction(), in UpdateVmCommand. Validate (vm NUMA nodes> <= (vm CPU cores), even if called from REST api Generate a detailed error message: [Cannot edit VM. Assigned 2 NUMA nodes for 1 CPU cores. Cannot assign more NUMA nodes then CPU cores.] Bug-Url: https://bugzilla.redhat.com/1220122 Change-Id: I4589721f3d9fa76509d2931b351c5517c06df334 Signed-off-by: Dudi Maroshi <d...@redhat.com> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java M backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java M backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties M frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java M frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties 7 files changed, 66 insertions(+), 2 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/25/40825/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java index 5a39cc2..0d4d44d 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java @@ -637,6 +637,14 @@ return false; } + // validate NUMA nodes count not more then CPUs + if (getParameters().getVm().getMigrationSupport() == MigrationSupport.PINNED_TO_HOST && + !validate(VmHandler.checkVmNumaNodesIntegrity(getParameters().getVm(), + getParameters().getVm(), + getParameters().isUpdateNuma()))) { + return false; + } + return true; } 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 8d2ae81..fbdf0e8 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 @@ -635,6 +635,12 @@ getVmId()))) { return false; } + if (getParameters().getVm().getMigrationSupport() == MigrationSupport.PINNED_TO_HOST && + !validate(VmHandler.checkVmNumaNodesIntegrity(getParameters().getVm(), + getVm(), + getParameters().isUpdateNuma()))) { + return false; + } return true; } 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 91f3709..dd14e75 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 @@ -873,8 +873,52 @@ } /** - * preferred supports single pinned vnuma node (without that VM fails to run in libvirt). - * used by add/update VM commands + * Cannot have more vm NUMA nodes than vm CPU core + * + * @param isNumaChanged + */ + public static ValidationResult checkVmNumaNodesIntegrity(VM paramsVm, VM actualVm, boolean isNumaChanged) { + /* calculate the actual NUMA nodes */ + List<VmNumaNode> paramVmNumaNodes = paramsVm.getvNumaNodeList(); + boolean emptyParamVmNumaNodes = (paramVmNumaNodes == null) || (paramVmNumaNodes.isEmpty()); + /* origVmNumaNodes = NUMA nodes list prior to update. */ + List<VmNumaNode> origVmNumaNodes = + DbFacade.getInstance().getVmNumaNodeDAO().getAllVmNumaNodeByVmId(actualVm.getId()); + boolean emptyOrigVmNumaNodes = (origVmNumaNodes == null) || (origVmNumaNodes.isEmpty()); + + int NUMAnodesCount = 0; + + /* return valid if no NUMA nodes */ + if (emptyParamVmNumaNodes && emptyOrigVmNumaNodes) { + return ValidationResult.VALID; + } + /* if no NUMA nodes in parameters, but there are NUMA nodes in previous vm */ + if (emptyParamVmNumaNodes && !emptyOrigVmNumaNodes) { + /* REST-api always provide emptyParamVmNumaNodes */ + /* REST-api modifies NUMA nodes via: addVmNumaNodeCommand/updateVmNumaNodeCommand */ + /* count NUMA nodes in previous vm, by default */ + NUMAnodesCount = origVmNumaNodes.size(); + /* if GUI update to reset NUMA nodes */ + if (isNumaChanged == true) + return ValidationResult.VALID; // no NUMA nodes. + } + if (!emptyParamVmNumaNodes) {// An update to NUMA nodes + NUMAnodesCount = paramVmNumaNodes.size(); + } + + int cpuCount = paramsVm.getNumOfCpus(); // REST-api assigns cpuCount to parameters. + if (cpuCount < NUMAnodesCount) { + return new ValidationResult(VdcBllMessages.VM_NUMA_NODE_MORE_NODES_THAN_CPUS, + String.format("$numaNodes %d", NUMAnodesCount), + String.format("$cpus %d", cpuCount)); + } + + return ValidationResult.VALID; + } + + /** + * preferred supports single pinned vnuma node (without that VM fails to run in libvirt). used by add/update VM + * commands */ public static ValidationResult checkNumaPreferredTuneMode(NumaTuneMode numaTuneMode, List<VmNumaNode> vmNumaNodes, diff --git a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java index 61e6c9a..bea7b2f 100644 --- a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java +++ b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java @@ -395,6 +395,7 @@ VM_NUMA_NODE_PINNED_INDEX_ERROR(ErrorType.BAD_PARAMETERS), VM_NUMA_NODE_MEMRORY_ERROR(ErrorType.BAD_PARAMETERS), VM_NUMA_NODE_PREFERRED_NOT_PINNED_TO_SINGLE_NODE(ErrorType.BAD_PARAMETERS), + VM_NUMA_NODE_MORE_NODES_THAN_CPUS(ErrorType.BAD_PARAMETERS), CANNOT_PREVIEW_ACTIVE_SNAPSHOT(ErrorType.BAD_PARAMETERS), VM_CANNOT_SUSPENDE_HAS_RUNNING_TASKS(ErrorType.CONFLICT), VM_CANNOT_REMOVE_HAS_RUNNING_TASKS(ErrorType.CONFLICT), diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties index ece031f..2b4553f 100644 --- a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties +++ b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties @@ -719,6 +719,7 @@ VM_NUMA_NODE_PINNED_INDEX_ERROR=NUMA node pinned index error. VM_NUMA_NODE_MEMRORY_ERROR=NUMA node memory error. VM_NUMA_NODE_PREFERRED_NOT_PINNED_TO_SINGLE_NODE=Preferred NUMA tune mode is allowed for a single pinned Virtual NUMA Node. +VM_NUMA_NODE_MORE_NODES_THAN_CPUS=Cannot ${action} ${type}. Assigned ${numaNodes} NUMA nodes for ${cpus} CPU cores. Cannot assign more NUMA nodes than CPU cores. ACTION_TYPE_FAILED_TEMPLATE_NOT_FOUND_ON_EXPORT_DOMAIN=Cannot export VM. Template ${TemplateName} does not exist on the export domain. if you want to export VM without its Template please use TemplateMustExists=false ACTION_TYPE_FAILED_VM_NOT_FOUND_ON_EXPORT_DOMAIN=Cannot delete VM, VM not exists in export domain ACTION_NOT_SUPPORTED_FOR_CLUSTER_POOL_LEVEL=The Action ${action} ${type} is not supported for this Cluster or Data Center compatibility version 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 667bfd3..7a73682 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 @@ -3294,6 +3294,9 @@ @DefaultStringValue("Preferred NUMA tune mode is allowed for a single pinned Virtual NUMA Node.") String VM_NUMA_NODE_PREFERRED_NOT_PINNED_TO_SINGLE_NODE(); + @DefaultStringValue("Cannot ${action} ${type}. Assigned ${numaNodes} NUMA nodes for ${cpus} CPU cores. Cannot assign more NUMA nodes than CPU cores.") + String VM_NUMA_NODE_MORE_NODES_THAN_CPUS(); + @DefaultStringValue("$detailMessage it is not a Hosted Engine host.") String VAR__DETAIL__NOT_HE_HOST(); diff --git a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties index 8a761d7..a2bbcb2 100644 --- a/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties +++ b/frontend/webadmin/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties @@ -1250,3 +1250,4 @@ CPU_TYPE_UNSUPPORTED_FOR_THE_GUEST_OS=The guest OS doesn't support the following CPUs: ${unsupportedCpus}. Its possible to change the cluster cpu or set a different one per VM BALLOON_REQUESTED_ON_NOT_SUPPORTED_ARCH=Cannot ${action} ${type}. Balloon is not supported on '${clusterArch}' architecture. +VM_NUMA_NODE_MORE_NODES_THAN_CPUS=Cannot ${action} ${type}. Assigned ${numaNodes} NUMA nodes for ${cpus} CPU cores. Cannot assign more NUMA nodes than CPU cores. -- To view, visit https://gerrit.ovirt.org/40825 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4589721f3d9fa76509d2931b351c5517c06df334 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: ovirt-engine-3.5.3 Gerrit-Owner: Dudi Maroshi <d...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
