Hello Jason Liao,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/33187
to review the following change.
Change subject: engine: NUMA feature queries and actions validation
......................................................................
engine: NUMA feature queries and actions validation
Add validation for add/update/remove VM NUMA node.
Change-Id: I9c299405ec5d82ada713ed3d220554bf3055c145
Bug-Url: https://bugzilla.redhat.com/1069303
Signed-off-by: Jason Liao <chuan.l...@hp.com>
---
A
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AddVmNumaNodesCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/RemoveVmNumaNodesCommand.java
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/UpdateVmNumaNodesCommand.java
M
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.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 packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
8 files changed, 155 insertions(+), 60 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/87/33187/1
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
new file mode 100644
index 0000000..a524fed
--- /dev/null
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AbstractVmNumaNodeCommand.java
@@ -0,0 +1,77 @@
+package org.ovirt.engine.core.bll.numa.vm;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.ovirt.engine.core.bll.VmCommand;
+import org.ovirt.engine.core.common.action.VmNumaNodeOperationParameters;
+import org.ovirt.engine.core.common.businessentities.NumaTuneMode;
+import org.ovirt.engine.core.common.businessentities.VM;
+import org.ovirt.engine.core.common.businessentities.VdsNumaNode;
+import org.ovirt.engine.core.common.businessentities.VmNumaNode;
+import org.ovirt.engine.core.common.config.Config;
+import org.ovirt.engine.core.common.config.ConfigValues;
+import org.ovirt.engine.core.common.errors.VdcBllMessages;
+import org.ovirt.engine.core.common.utils.Pair;
+import org.ovirt.engine.core.compat.Guid;
+import org.ovirt.engine.core.dao.VdsNumaNodeDAO;
+import org.ovirt.engine.core.dao.VmNumaNodeDAO;
+
+public abstract class AbstractVmNumaNodeCommand<T extends
VmNumaNodeOperationParameters> extends VmCommand<T> {
+
+ public AbstractVmNumaNodeCommand(T parameters) {
+ super(parameters);
+ }
+
+ protected VdsNumaNodeDAO getVdsNumaNodeDao() {
+ return getDbFacade().getVdsNumaNodeDAO();
+ }
+
+ protected VmNumaNodeDAO getVmNumaNodeDao() {
+ return getDbFacade().getVmNumaNodeDAO();
+ }
+
+ @Override
+ protected boolean canDoAction() {
+ List<VmNumaNode> vmNumaNodes = getParameters().getVmNumaNodeList();
+ if (vmNumaNodes == null || vmNumaNodes.size() == 0) {
+ // if VM do not contain any NUMA node, skip checking
+ return true;
+ }
+
+ VM vm = getVm();
+ boolean pinHost = !Config.<Boolean>
getValue(ConfigValues.SupportNUMAMigration);
+ Guid vdsId = vm.getDedicatedVmForVds();
+ if (pinHost && vdsId == null) {
+ return
failCanDoAction(VdcBllMessages.VM_NUMA_PINNED_VDS_NOT_EXIST);
+ }
+
+ List<VdsNumaNode> hostNumaNodes = new ArrayList<>();
+ if (pinHost) {
+ hostNumaNodes =
getVdsNumaNodeDao().getAllVdsNumaNodeByVdsId(vdsId);
+ if (hostNumaNodes == null || hostNumaNodes.size() == 0) {
+ return
failCanDoAction(VdcBllMessages.VM_NUMA_PINNED_VDS_NODE_EMPTY);
+ }
+ }
+
+ boolean memStrict = vm.getNumaTuneMode() == NumaTuneMode.STRICT;
+ for (VmNumaNode vmNumaNode : vmNumaNodes) {
+ for (Pair<Guid, Pair<Boolean, Integer>> pair :
vmNumaNode.getVdsNumaNodeList()) {
+ if (pair.getSecond() == null || pair.getSecond().getSecond()
== null) {
+ return
failCanDoAction(VdcBllMessages.VM_NUMA_NODE_PINNED_INDEX_ERROR);
+ }
+
+ Integer index = pair.getSecond().getSecond();
+ for (VdsNumaNode vdsNumaNode : hostNumaNodes) {
+ if (vdsNumaNode.getIndex() == index) {
+ if (memStrict && vmNumaNode.getMemTotal() >
vdsNumaNode.getMemTotal()) {
+ return
failCanDoAction(VdcBllMessages.VM_NUMA_NODE_MEMRORY_ERROR);
+ }
+ break;
+ }
+ }
+ }
+ }
+ return true;
+ }
+}
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AddVmNumaNodesCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AddVmNumaNodesCommand.java
index f1c9246..d43389c 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AddVmNumaNodesCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/AddVmNumaNodesCommand.java
@@ -3,7 +3,6 @@
import java.util.ArrayList;
import java.util.List;
-import org.ovirt.engine.core.bll.VmCommand;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -13,7 +12,7 @@
import org.ovirt.engine.core.common.utils.Pair;
import org.ovirt.engine.core.compat.Guid;
-public class AddVmNumaNodesCommand<T extends VmNumaNodeOperationParameters>
extends VmCommand<T> {
+public class AddVmNumaNodesCommand<T extends VmNumaNodeOperationParameters>
extends AbstractVmNumaNodeCommand<T> {
public AddVmNumaNodesCommand(T parameters) {
super(parameters);
@@ -21,41 +20,39 @@
@Override
protected void executeCommand() {
- boolean succeeded = false;
- try {
- Guid vmId = getParameters().getVmId();
- Guid vdsId = getVm().getDedicatedVmForVds();
- List<VdsNumaNode> vdsNumaNodes = new ArrayList<VdsNumaNode>();
- if (vdsId != null) {
- vdsNumaNodes =
getDbFacade().getVdsNumaNodeDAO().getAllVdsNumaNodeByVdsId(vdsId);
- }
- List<VmNumaNode> vmNumaNodes = getParameters().getVmNumaNodeList();
- List<VdsNumaNode> nodes = new ArrayList<VdsNumaNode>();
- for (VmNumaNode vmNumaNode : vmNumaNodes) {
- for (Pair<Guid, Pair<Boolean, Integer>> pair :
vmNumaNode.getVdsNumaNodeList()) {
- int index = pair.getSecond().getSecond();
- for (VdsNumaNode vdsNumaNode : vdsNumaNodes) {
- if (vdsNumaNode.getIndex() == index) {
- pair.setFirst(vdsNumaNode.getId());
- pair.getSecond().setFirst(true);
- break;
- }
+ Guid vmId = getParameters().getVmId();
+ List<VmNumaNode> vmNumaNodes = getParameters().getVmNumaNodeList();
+ Guid vdsId = getVm().getDedicatedVmForVds();
+ List<VdsNumaNode> vdsNumaNodes = new ArrayList<>();
+ if (vdsId != null) {
+ vdsNumaNodes = getVdsNumaNodeDao().getAllVdsNumaNodeByVdsId(vdsId);
+ }
+
+ List<VdsNumaNode> nodes = new ArrayList<>();
+ for (VmNumaNode vmNumaNode : vmNumaNodes) {
+ for (Pair<Guid, Pair<Boolean, Integer>> pair :
vmNumaNode.getVdsNumaNodeList()) {
+ int index = pair.getSecond().getSecond();
+ for (VdsNumaNode vdsNumaNode : vdsNumaNodes) {
+ if (vdsNumaNode.getIndex() == index) {
+ pair.setFirst(vdsNumaNode.getId());
+ pair.getSecond().setFirst(true);
+ break;
}
}
- nodes.add((VdsNumaNode) vmNumaNode);
}
- getDbFacade().getVmNumaNodeDAO().massSaveNumaNode(nodes, null,
vmId);
- // Used for restful API for reture first NUMA node GUID
- setActionReturnValue(nodes.get(0).getId());
- succeeded = true;
- } finally {
- setSucceeded(succeeded);
+ nodes.add(vmNumaNode);
}
+ getVmNumaNodeDao().massSaveNumaNode(nodes, null, vmId);
+
+ // Used for restful API for reture first NUMA node GUID
+ setActionReturnValue(nodes.get(0).getId());
+
+ setSucceeded(true);
}
@Override
public List<PermissionSubject> getPermissionCheckSubjects() {
- List<PermissionSubject> permissionList = new
ArrayList<PermissionSubject>();
+ List<PermissionSubject> permissionList = new ArrayList<>();
permissionList.add(new PermissionSubject(getParameters().getVmId(),
VdcObjectType.VM,
getActionType().getActionGroup()));
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/RemoveVmNumaNodesCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/RemoveVmNumaNodesCommand.java
index b99fb84..762dcb5 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/RemoveVmNumaNodesCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/RemoveVmNumaNodesCommand.java
@@ -3,15 +3,15 @@
import java.util.ArrayList;
import java.util.List;
-import org.ovirt.engine.core.bll.VmCommand;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
import org.ovirt.engine.core.common.action.VmNumaNodeOperationParameters;
import org.ovirt.engine.core.common.businessentities.VmNumaNode;
+import org.ovirt.engine.core.common.errors.VdcBllMessages;
import org.ovirt.engine.core.compat.Guid;
-public class RemoveVmNumaNodesCommand<T extends VmNumaNodeOperationParameters>
extends VmCommand<T> {
+public class RemoveVmNumaNodesCommand<T extends VmNumaNodeOperationParameters>
extends AbstractVmNumaNodeCommand<T> {
public RemoveVmNumaNodesCommand(T parameters) {
super(parameters);
@@ -22,11 +22,11 @@
boolean succeeded = false;
try {
List<VmNumaNode> vmNumaNodes = getParameters().getVmNumaNodeList();
- List<Guid> guids = new ArrayList<Guid>();
+ List<Guid> guids = new ArrayList<>();
for (VmNumaNode node : vmNumaNodes) {
guids.add(node.getId());
}
-
getDbFacade().getVmNumaNodeDAO().massRemoveNumaNodeByNumaNodeId(guids);
+ getVmNumaNodeDao().massRemoveNumaNodeByNumaNodeId(guids);
succeeded = true;
} finally {
setSucceeded(succeeded);
@@ -34,8 +34,16 @@
}
@Override
+ protected boolean canDoAction() {
+ if (getVm() == null) {
+ return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND);
+ }
+ return true;
+ }
+
+ @Override
public List<PermissionSubject> getPermissionCheckSubjects() {
- List<PermissionSubject> permissionList = new
ArrayList<PermissionSubject>();
+ List<PermissionSubject> permissionList = new ArrayList<>();
permissionList.add(new PermissionSubject(getParameters().getVmId(),
VdcObjectType.VM,
getActionType().getActionGroup()));
diff --git
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/UpdateVmNumaNodesCommand.java
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/UpdateVmNumaNodesCommand.java
index 2bfff5f..2738a3e 100644
---
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/UpdateVmNumaNodesCommand.java
+++
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/numa/vm/UpdateVmNumaNodesCommand.java
@@ -3,7 +3,6 @@
import java.util.ArrayList;
import java.util.List;
-import org.ovirt.engine.core.bll.VmCommand;
import org.ovirt.engine.core.bll.utils.PermissionSubject;
import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.VdcObjectType;
@@ -13,7 +12,7 @@
import org.ovirt.engine.core.common.utils.Pair;
import org.ovirt.engine.core.compat.Guid;
-public class UpdateVmNumaNodesCommand<T extends VmNumaNodeOperationParameters>
extends VmCommand<T> {
+public class UpdateVmNumaNodesCommand<T extends VmNumaNodeOperationParameters>
extends AbstractVmNumaNodeCommand<T> {
public UpdateVmNumaNodesCommand(T parameters) {
super(parameters);
@@ -21,38 +20,35 @@
@Override
protected void executeCommand() {
- boolean succeeded = false;
- try {
- Guid vdsId = getVm().getDedicatedVmForVds();
- List<VdsNumaNode> vdsNumaNodes = new ArrayList<VdsNumaNode>();
- if (vdsId != null) {
- vdsNumaNodes =
getDbFacade().getVdsNumaNodeDAO().getAllVdsNumaNodeByVdsId(vdsId);
- }
- List<VmNumaNode> vmNumaNodes = getParameters().getVmNumaNodeList();
- List<VdsNumaNode> nodes = new ArrayList<VdsNumaNode>();
- for (VmNumaNode vmNumaNode : vmNumaNodes) {
- for (Pair<Guid, Pair<Boolean, Integer>> pair :
vmNumaNode.getVdsNumaNodeList()) {
- int index = pair.getSecond().getSecond();
- for (VdsNumaNode vdsNumaNode : vdsNumaNodes) {
- if (vdsNumaNode.getIndex() == index) {
- pair.setFirst(vdsNumaNode.getId());
- pair.getSecond().setFirst(true);
- break;
- }
+ List<VmNumaNode> vmNumaNodes = getParameters().getVmNumaNodeList();
+ Guid vdsId = getVm().getDedicatedVmForVds();
+ List<VdsNumaNode> vdsNumaNodes = new ArrayList<>();
+ if (vdsId != null) {
+ vdsNumaNodes = getVdsNumaNodeDao().getAllVdsNumaNodeByVdsId(vdsId);
+ }
+
+ List<VdsNumaNode> nodes = new ArrayList<>();
+ for (VmNumaNode vmNumaNode : vmNumaNodes) {
+ for (Pair<Guid, Pair<Boolean, Integer>> pair :
vmNumaNode.getVdsNumaNodeList()) {
+ int index = pair.getSecond().getSecond();
+ for (VdsNumaNode vdsNumaNode : vdsNumaNodes) {
+ if (vdsNumaNode.getIndex() == index) {
+ pair.setFirst(vdsNumaNode.getId());
+ pair.getSecond().setFirst(true);
+ break;
}
}
- nodes.add((VdsNumaNode) vmNumaNode);
}
- getDbFacade().getVmNumaNodeDAO().massUpdateNumaNode(nodes);
- succeeded = true;
- } finally {
- setSucceeded(succeeded);
+ nodes.add(vmNumaNode);
}
+ getVmNumaNodeDao().massUpdateNumaNode(nodes);
+
+ setSucceeded(true);
}
@Override
public List<PermissionSubject> getPermissionCheckSubjects() {
- List<PermissionSubject> permissionList = new
ArrayList<PermissionSubject>();
+ List<PermissionSubject> permissionList = new ArrayList<>();
permissionList.add(new PermissionSubject(getParameters().getVmId(),
VdcObjectType.VM,
getActionType().getActionGroup()));
diff --git
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
index aef76d0..df651a7 100644
---
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
+++
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
@@ -1936,6 +1936,13 @@
@OptionBehaviourAttribute(behaviour =
OptionBehaviour.CommaSeparatedStringArray)
UnsupportedLocalesFilterOverrides,
+ /**
+ * Defines the parameter name used by numa migration on/off
+ */
+ @TypeConverterAttribute(Boolean.class)
+ @DefaultValueAttribute("false")
+ SupportNUMAMigration,
+
@TypeConverterAttribute(List.class)
@DefaultValueAttribute("")
@OptionBehaviourAttribute(behaviour =
OptionBehaviour.CommaSeparatedStringArray)
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 43deaa6..5082c88 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
@@ -380,6 +380,10 @@
VM_PINNING_PCPU_DOES_NOT_EXIST(ErrorType.BAD_PARAMETERS),
VM_PINNING_DUPLICATE_DEFINITION(ErrorType.BAD_PARAMETERS),
VM_PINNING_PINNED_TO_NO_CPU(ErrorType.BAD_PARAMETERS),
+ VM_NUMA_PINNED_VDS_NOT_EXIST(ErrorType.BAD_PARAMETERS),
+ VM_NUMA_PINNED_VDS_NODE_EMPTY(ErrorType.BAD_PARAMETERS),
+ VM_NUMA_NODE_PINNED_INDEX_ERROR(ErrorType.BAD_PARAMETERS),
+ VM_NUMA_NODE_MEMRORY_ERROR(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 5d140ae..0faa2b2 100644
---
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -703,6 +703,10 @@
VM_PINNING_PCPU_DOES_NOT_EXIST=CPU pinning validation failed - CPU does not
exist in host.
VM_PINNING_DUPLICATE_DEFINITION=Cannot configure CPU pinning twice to the same
vCPU.
VM_PINNING_PINNED_TO_NO_CPU=Cannot pin a vCPU to no pCPU.
+VM_NUMA_PINNED_VDS_NOT_EXIST=Cannot pin NUMA nodes when VM is not pin to a VDS.
+VM_NUMA_PINNED_VDS_NODE_EMPTY=Cannot pin NUMA nodes when VDS NUMA nodes is
empty.
+VM_NUMA_NODE_PINNED_INDEX_ERROR=NUMA node pinned index error.
+VM_NUMA_NODE_MEMRORY_ERROR=NUMA node memory error.
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/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
index 7e50340..035761ef 100644
--- a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
+++ b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
@@ -820,6 +820,8 @@
select fn_db_add_config_value('HostStorageLeaseAliveCheckingInterval', '90',
'general');
+select fn_db_add_config_value('NUMAMigration','false','general');
+
------------------------------------------------------------------------------------
-- Update with override section
------------------------------------------------------------------------------------
--
To view, visit http://gerrit.ovirt.org/33187
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9c299405ec5d82ada713ed3d220554bf3055c145
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.5
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Jason Liao <chuan.l...@hp.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
