Dudi Maroshi has uploaded a new change for review.
Change subject: engine: On vm replacement vialidate vm Numa config match host
Numa config
......................................................................
engine: On vm replacement vialidate vm Numa config match host Numa config
When vm is replaced from host A to host B. And has pinned NUMA configuration.
The vm NUMA configuration, is validated to match host B NUMA configuration.
The validatation is made in UpdateVmCommand at canDoAction().
The algorithm is:
1. Get destination host NUMA nodes into a set.
2. Iterate over vm NUMA nodes
2.1 If vm NUMA node is not referencing a NUMA node in the destination host set
fail the validation
Change-Id: If32c7fede611dd3d1d0efeaae41f57f8273b7e37
Bug-Url: https://bugzilla.redhat.com/1177259
Signed-off-by: Dudi Maroshi <[email protected]>
---
M
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.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
5 files changed, 101 insertions(+), 22 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/70/38770/1
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 31ed265..9ad5c58 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
@@ -5,9 +5,11 @@
import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import org.apache.commons.codec.CharEncoding;
import org.apache.commons.codec.binary.Base64;
@@ -45,6 +47,7 @@
import org.ovirt.engine.core.common.businessentities.Snapshot;
import org.ovirt.engine.core.common.businessentities.VM;
import org.ovirt.engine.core.common.businessentities.VMStatus;
+import org.ovirt.engine.core.common.businessentities.VdsNumaNode;
import org.ovirt.engine.core.common.businessentities.VmBase;
import org.ovirt.engine.core.common.businessentities.VmDevice;
import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType;
@@ -80,7 +83,7 @@
import org.ovirt.engine.core.utils.linq.Predicate;
public class UpdateVmCommand<T extends VmManagementParametersBase> extends
VmManagementCommandBase<T>
- implements QuotaVdsDependent, RenamedEntityInfoProvider{
+ implements QuotaVdsDependent, RenamedEntityInfoProvider {
private static final Base64 BASE_64 = new Base64(0, null);
private VM oldVm;
private boolean quotaSanityOnly = false;
@@ -100,7 +103,8 @@
if (isVmExist()) {
Version clusterVersion = getVdsGroup().getCompatibilityVersion();
-
getVmPropertiesUtils().separateCustomPropertiesToUserAndPredefined(clusterVersion,
parameters.getVmStaticData());
+
getVmPropertiesUtils().separateCustomPropertiesToUserAndPredefined(clusterVersion,
+ parameters.getVmStaticData());
getVmPropertiesUtils().separateCustomPropertiesToUserAndPredefined(clusterVersion,
getVm().getStaticData());
}
VmHandler.updateDefaultTimeZone(parameters.getVmStaticData());
@@ -112,7 +116,6 @@
updateParametersVmFromInstanceType();
}
-
private VmPropertiesUtils getVmPropertiesUtils() {
return VmPropertiesUtils.getInstance();
@@ -190,16 +193,19 @@
if (rngDevs.isEmpty()) {
if (getParameters().getRngDevice() != null) {
RngDeviceParameters params = new
RngDeviceParameters(getParameters().getRngDevice(), true);
- rngCommandResult =
runInternalAction(VdcActionType.AddRngDevice, params,
cloneContextAndDetachFromParent());
+ rngCommandResult =
+ runInternalAction(VdcActionType.AddRngDevice,
params, cloneContextAndDetachFromParent());
}
} else {
if (getParameters().getRngDevice() == null) {
RngDeviceParameters params = new
RngDeviceParameters(rngDevs.get(0), true);
- rngCommandResult =
runInternalAction(VdcActionType.RemoveRngDevice, params,
cloneContextAndDetachFromParent());
+ rngCommandResult =
+ runInternalAction(VdcActionType.RemoveRngDevice,
params, cloneContextAndDetachFromParent());
} else {
RngDeviceParameters params = new
RngDeviceParameters(getParameters().getRngDevice(), true);
params.getRngDevice().setDeviceId(rngDevs.get(0).getDeviceId());
- rngCommandResult =
runInternalAction(VdcActionType.UpdateRngDevice, params,
cloneContextAndDetachFromParent());
+ rngCommandResult =
+ runInternalAction(VdcActionType.UpdateRngDevice,
params, cloneContextAndDetachFromParent());
}
}
@@ -300,12 +306,16 @@
watchdogParameters.setId(getParameters().getVmId());
if (getParameters().getWatchdog() == null) {
// there is a watchdog in the vm, there should not be any,
so let's delete
- runInternalAction(VdcActionType.RemoveWatchdog,
watchdogParameters, cloneContextAndDetachFromParent());
+ runInternalAction(VdcActionType.RemoveWatchdog,
+ watchdogParameters,
+ cloneContextAndDetachFromParent());
} else {
// there is a watchdog in the vm, we have to update.
watchdogParameters.setAction(getParameters().getWatchdog().getAction());
watchdogParameters.setModel(getParameters().getWatchdog().getModel());
- runInternalAction(VdcActionType.UpdateWatchdog,
watchdogParameters, cloneContextAndDetachFromParent());
+ runInternalAction(VdcActionType.UpdateWatchdog,
+ watchdogParameters,
+ cloneContextAndDetachFromParent());
}
}
@@ -349,8 +359,10 @@
private List<GraphicsDevice> getGraphicsDevices() {
if (cachedGraphics == null) {
- cachedGraphics = getBackend()
- .runInternalQuery(VdcQueryType.GetGraphicsDevices, new
IdQueryParameters(getParameters().getVmId())).getReturnValue();
+ cachedGraphics =
+ getBackend()
+ .runInternalQuery(VdcQueryType.GetGraphicsDevices,
+ new
IdQueryParameters(getParameters().getVmId())).getReturnValue();
}
return cachedGraphics;
}
@@ -566,7 +578,8 @@
// Check if number of monitors passed is legal
if (!VmHandler.isNumOfMonitorsLegal(
-
VmHandler.getResultingVmGraphics(VmDeviceUtils.getGraphicsTypesOfEntity(getVmId()),
getParameters().getGraphicsDevices()),
+
VmHandler.getResultingVmGraphics(VmDeviceUtils.getGraphicsTypesOfEntity(getVmId()),
+ getParameters().getGraphicsDevices()),
getParameters().getVmStaticData().getNumOfMonitors(),
getReturnValue().getCanDoActionMessages())) {
return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_ILLEGAL_NUM_OF_MONITORS);
@@ -588,7 +601,7 @@
if (!AddVmCommand.checkCpuSockets(vmFromParams.getNumOfSockets(),
vmFromParams.getCpuPerSocket(),
getVdsGroup().getCompatibilityVersion()
- .toString(), getReturnValue().getCanDoActionMessages())) {
+ .toString(),
getReturnValue().getCanDoActionMessages())) {
return false;
}
@@ -599,7 +612,8 @@
}
// we save the content in base64 string
for (Map.Entry<String, String> entry :
getParameters().getVmPayload().getFiles().entrySet()) {
- entry.setValue(new
String(BASE_64.encode(entry.getValue().getBytes()),
Charset.forName(CharEncoding.UTF_8)));
+ entry.setValue(new
String(BASE_64.encode(entry.getValue().getBytes()),
+ Charset.forName(CharEncoding.UTF_8)));
}
}
@@ -622,14 +636,16 @@
// Check if the graphics and display from parameters are supported
if (!VmHandler.isGraphicsAndDisplaySupported(vmFromParams.getOs(),
-
VmHandler.getResultingVmGraphics(VmDeviceUtils.getGraphicsTypesOfEntity(getVmId()),
getParameters().getGraphicsDevices()),
+
VmHandler.getResultingVmGraphics(VmDeviceUtils.getGraphicsTypesOfEntity(getVmId()),
+ getParameters().getGraphicsDevices()),
vmFromParams.getDefaultDisplayType(),
getReturnValue().getCanDoActionMessages(),
getVdsGroup().getCompatibilityVersion())) {
return false;
}
- if
(!FeatureSupported.isMigrationSupported(getVdsGroup().getArchitecture(),
getVdsGroup().getCompatibilityVersion())
+ if
(!FeatureSupported.isMigrationSupported(getVdsGroup().getArchitecture(),
+ getVdsGroup().getCompatibilityVersion())
&& vmFromParams.getMigrationSupport() !=
MigrationSupport.PINNED_TO_HOST) {
return
failCanDoAction(VdcBllMessages.VM_MIGRATION_IS_NOT_SUPPORTED);
}
@@ -660,14 +676,16 @@
}
// Verify OS compatibility
- if
(!VmHandler.isOsTypeSupportedForVirtioScsi(vmFromParams.getOs(),
getVdsGroup().getCompatibilityVersion(),
+ if (!VmHandler.isOsTypeSupportedForVirtioScsi(vmFromParams.getOs(),
+ getVdsGroup().getCompatibilityVersion(),
getReturnValue().getCanDoActionMessages())) {
return false;
}
}
VmValidator vmValidator = createVmValidator(vmFromParams);
- if (Boolean.FALSE.equals(getParameters().isVirtioScsiEnabled()) &&
!validate(vmValidator.canDisableVirtioScsi(null))) {
+ if (Boolean.FALSE.equals(getParameters().isVirtioScsiEnabled())
+ && !validate(vmValidator.canDisableVirtioScsi(null))) {
return false;
}
@@ -691,7 +709,57 @@
return false;
}
+ // validate vm Numa pinning match host Numa nodes
+ if (vmFromParams.getMigrationSupport() ==
MigrationSupport.PINNED_TO_HOST &&
+ isReplacingNumaPinnedVm()) {
+ return
failCanDoAction(VdcBllMessages.VM_NUMA_NODE_MISMATCHED_HOST);
+ }
+
return true;
+ }
+
+ /*
+ * Identify replaced host: Match each vm Numa node Guid to dedicated host
Numa node. If there is NO match, the host
+ * was replaced.
+ */
+ private boolean isReplacingNumaPinnedVm() {
+ Guid dedicatedVdsGuid = getVm().getDedicatedVmForVds();
+ if (dedicatedVdsGuid == null)
+ return false;
+ // question for review: is it safe to assume vm run at least once
prior to replacement?
+ // question for review: Why getVm().getDynamicData().getLastVdsRunOn()
not working - relevant db column is
+ // empty, shell we fix?
+
+ // get dedicated host Numa nodes information
+ List<VdsNumaNode> hostNumaNodesList = getDbFacade().getVdsNumaNodeDAO()
+ .getAllVdsNumaNodeByVdsId(dedicatedVdsGuid);
+ if (hostNumaNodesList == null || hostNumaNodesList.size() == 0)
+ return false;
+
+ // Collect dedicatedVds Numa nodes Guids into a set
+ Set<Guid> hostNumaNodesGuidsSet = new HashSet<Guid>();
+ for (VdsNumaNode vdsNumaNode : hostNumaNodesList) {
+ hostNumaNodesGuidsSet.add(vdsNumaNode.getId());
+ }
+
+ // question for review: is it safe to read numaNodesList from
parameters
+ List<VmNumaNode> numaNodesList =
getParameters().getVmStaticData().getvNumaNodeList();
+ if (numaNodesList != null && numaNodesList.size() > 0) {
+ for (VmNumaNode vmNumaNode : numaNodesList) {
+ if (vmNumaNode.getVdsNumaNodeList() != null &&
vmNumaNode.getVdsNumaNodeList().size() > 0) {
+ Guid physicalNumaNodeGuid =
vmNumaNode.getVdsNumaNodeList().get(0).getFirst();
+ // continue if vmNumaNode is not pinned to physical Numa
node.
+ if (physicalNumaNodeGuid != null) {
+ // if vm Numa node NOT identified as dedicated host
Numa node, this is replacement command
+ if
(!hostNumaNodesGuidsSet.contains(physicalNumaNodeGuid)) {
+ return true;
+ }
+ }
+ }
+ }
+ }
+ boolean retVal = false; // debug variable for error testing
+ return retVal;
}
protected boolean isDedicatedVdsExistOnSameCluster(VmBase vm,
@@ -728,8 +796,9 @@
/**
* check if we need to use running-configuration
- * @return true if vm is running and we change field that has
@EditableOnVmStatusField annotation
- * or runningConfiguration already exist
+ *
+ * @return true if vm is running and we change field that has
@EditableOnVmStatusField annotation or
+ * runningConfiguration already exist
*/
private boolean isRunningConfigurationNeeded() {
return getVm().isNextRunConfigurationExists() ||
@@ -765,7 +834,8 @@
final boolean isDedicatedVmForVdsChanged =
!(getVm().getDedicatedVmForVds() == null ?
getParameters().getVmStaticData().getDedicatedVmForVds() == null :
-
getVm().getDedicatedVmForVds().equals(getParameters().getVmStaticData().getDedicatedVmForVds()));
+
getVm().getDedicatedVmForVds().equals(getParameters().getVmStaticData()
+ .getDedicatedVmForVds()));
final boolean isCpuPinningChanged =
!(getVm().getCpuPinning() == null ?
@@ -795,7 +865,8 @@
protected Map<String, Pair<String, String>> getExclusiveLocks() {
if (!StringUtils.isBlank(getParameters().getVm().getName())) {
return Collections.singletonMap(getParameters().getVm().getName(),
-
LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM_NAME,
VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_BEING_UPDATED));
+ LockMessagesMatchUtil.makeLockingPair(LockingGroup.VM_NAME,
+
VdcBllMessages.ACTION_TYPE_FAILED_VM_IS_BEING_UPDATED));
}
return null;
}
@@ -839,6 +910,7 @@
}
return list;
}
+
@Override
public String getEntityType() {
return VdcObjectType.VM.getVdcObjectTranslation();
@@ -856,8 +928,9 @@
@Override
public void setEntityId(AuditLogableBase logable) {
- logable.setVmId(oldVm.getId());
+ logable.setVmId(oldVm.getId());
}
+
@Override
public void addQuotaPermissionSubject(List<PermissionSubject>
quotaPermissionList) {
// if only quota sanity is checked the user may use a quota he cannot
consume
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 9d145a6..26c650f 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
@@ -415,6 +415,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_MISMATCHED_HOST(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 f651363..af5a389 100644
---
a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
+++
b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -744,6 +744,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_MISMATCHED_HOST= Cannot ${action} ${type}. Host replacement for
vm with pinned NUMA nodes, not allowed.
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 e66ea57..756eb5b 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
@@ -3460,6 +3460,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}: Host replacment for vm with
pinned NUMA nodes not allowed.")
+ String VM_NUMA_NODE_MISMATCHED_HOST();
+
@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 dc00ce6..f66b2b7 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
@@ -150,6 +150,7 @@
VMT_CANNOT_UPDATE_VERSION_NAME=Cannot update the name of Sub-Templates, Only
the Version name can be updated.
DIRECTORY_GROUP_CANNOT_REMOVE_DIRECTORY_GROUP_ATTACHED_TO_VM=Cannot remove
Directory Group. Detach Directory Group from VM first.
VM_NOT_FOUND=VM not found
+VM_NUMA_NODE_MISMATCHED_HOST= Cannot ${action} ${type}. Host replacement for
vm with pinned NUMA nodes, not allowed.
ACTION_TYPE_FAILED_CLUSTER_UNDEFINED_ARCHITECTURE=Cannot ${action} ${type}.
The cluster does not have a defined architecture.
ACTION_TYPE_FAILED_VM_IN_PREVIEW=Cannot ${action} ${type}. VM is previewing a
Snapshot.
ACTION_TYPE_FAILED_STORAGE_DELETE_VMS_IN_PREVIEW=Cannot ${action} ${type}. The
following VMs are previewing a snapshot: ${vms}.
--
To view, visit https://gerrit.ovirt.org/38770
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If32c7fede611dd3d1d0efeaae41f57f8273b7e37
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dudi Maroshi <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
