Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

webadmin: Refactor NUMA for VM #488

Merged
merged 2 commits into from
Jul 5, 2022
Merged

webadmin: Refactor NUMA for VM #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e46a63b
webadmin: Refactor NUMA for VM
ljelinkova Jun 23, 2022
a103e3c
engine: let backend decide when update numa
ljelinkova Jun 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
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;
Expand Down Expand Up @@ -957,6 +958,14 @@ private void initNuma() {
getVm().setvNumaNodeList(vNumaNodeList);
}

if (getParameters().getVm().getCpuPinningPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA) {
getParameters().setUpdateNuma(false);
}
ahadas marked this conversation as resolved.
Show resolved Hide resolved

if (getParameters().isUpdateNuma() == null) {
ljelinkova marked this conversation as resolved.
Show resolved Hide resolved
getParameters().setUpdateNuma(!vNumaNodeList.equals(getParameters().getVm().getvNumaNodeList()));
}

// we always need to verify new or existing numa nodes with the updated VM configuration
if (!getParameters().isUpdateNuma()) {
getParameters().getVm().setvNumaNodeList(vNumaNodeList);
Expand Down
11 changes: 6 additions & 5 deletions .../common/src/main/java/org/ovirt/engine/core/common/action/VmManagementParametersBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public void setValue(T value) {
private VM vm;
private boolean copyTemplatePermissions;
private boolean applyChangesLater;
private boolean updateNuma;
private Boolean updateNuma;
private String vmLargeIcon;
private Version clusterLevelChangeFromVersion;
private Map<VmExternalDataKind, String> vmExternalData;
Expand Down Expand Up @@ -346,14 +346,15 @@ public void setApplyChangesLater(boolean applyChangesLater) {
}

/**
* Since NUMA configuration can be updated, this flag indicates whether client
* sends NUMA info that needs to be updated.
* This flag indicates whether NUMA info from the client should be saved.
* @return true if the backend should save the NUMA info, false if not, null if
* the clients leaves that decision on the backend
*/
public boolean isUpdateNuma() {
public Boolean isUpdateNuma() {
return updateNuma;
}

public void setUpdateNuma(boolean updateNuma) {
public void setUpdateNuma(Boolean updateNuma) {
this.updateNuma = updateNuma;
}

Expand Down
5 changes: 1 addition & 4 deletions .../restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,7 @@ public ActionParametersBase getParameters(Vm incoming,

VmManagementParametersBase params = new VmManagementParametersBase(updated);

if (incoming.isSetNumaTuneMode()) {
params.setUpdateNuma(true);
}

params.setUpdateNuma(incoming.isSetNumaTuneMode());
params.setApplyChangesLater(isNextRunRequested());
params.setMemoryHotUnplugEnabled(true);

Expand Down
21 changes: 5 additions & 16 deletions ...src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@
import org.ovirt.engine.ui.uicommonweb.models.vms.key_value.KeyValueModel;
import org.ovirt.engine.ui.uicompat.EnumTranslator;
import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs;
import org.ovirt.engine.ui.uicompat.UIConstants;

import com.google.gwt.core.client.GWT;
import com.google.gwt.event.dom.client.ClickEvent;
Expand Down Expand Up @@ -1087,6 +1088,8 @@ interface ViewUiBinder extends UiBinder<DialogTabPanel, AbstractVmPopupWidget> {
private static final CommonApplicationConstants constants = AssetProvider.getConstants();
private static final CommonApplicationMessages messages = AssetProvider.getMessages();

private static final UIConstants uiConstants = GWT.create(UIConstants.class);

private final Map<TabName, OvirtTabListItem> tabMap = new HashMap<>();

public AbstractVmPopupWidget() {
Expand Down Expand Up @@ -1352,7 +1355,7 @@ protected void initPoolSpecificWidgets() {
editPoolMaxAssignedVmsPerUserIcon =
new InfoIcon(templates.italicText(messages.maxAssignedVmsPerUserHelp()));

numaInfoIcon = new InfoIcon(SafeHtmlUtils.fromTrustedString("")); //$NON-NLS-1$
numaInfoIcon = new InfoIcon(SafeHtmlUtils.fromTrustedString(uiConstants.numaInfoMessage()));

isRngEnabledInfoIcon = new InfoIcon(SafeHtmlUtils.fromTrustedString(constants.rngDevExplanation()));

Expand Down Expand Up @@ -1733,11 +1736,8 @@ private <T> void initClusterDefaultValueListener(ClusterDefaultRenderer<T> rende
private void enableNumaSupport(final UnitVmModel model) {
numaSupportButton.setCommand(model.getNumaSupportCommand());
numaPanel.setVisible(false);
enableNumaFields(false);
model.getNumaEnabled().getEntityChangedEvent().addListener((ev, sender, args) -> {
model.getNumaSupportCommand().getPropertyChangedEvent().addListener((ev, sender, args) -> {
numaPanel.setVisible(true);
enableNumaFields(model.getNumaEnabled().getEntity());
setNumaInfoMsg(model.getNumaEnabled().getMessage());
});
}

Expand All @@ -1747,17 +1747,6 @@ private void localizeSafeHtmlFields() {
detachableMinAllocatedMemoryEditor.setExplanation(SafeHtmlUtils.fromString(constants.physMemGuarInfoIcon()));
}

private void setNumaInfoMsg(String message) {
if (message == null) {
message = ""; //$NON-NLS-1$
}
numaInfoIcon.setText(multiLineItalicSafeHtml(message));
}

private void enableNumaFields(boolean enabled) {
numaSupportButton.setEnabled(enabled);
}

@UiHandler("refreshButton")
void handleRefreshButtonClick(ClickEvent event) {
unitVmModel.getBehavior().refreshCdImages();
Expand Down
1 change: 1 addition & 0 deletions .../main/java/org/ovirt/engine/ui/uicommonweb/builders/vm/CpuPinningVmBaseToUnitBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public class CpuPinningVmBaseToUnitBuilder extends BaseSyncBuilder<VmBase, UnitV

@Override
protected void build(VmBase vm, UnitVmModel model) {
model.setOriginalCpuPinningPolicy(vm.getCpuPinningPolicy());
model.getCpuPinningPolicy().setSelectedCpuPolicy(vm.getCpuPinningPolicy());
if (vm.getCpuPinningPolicy() == CpuPinningPolicy.MANUAL) {
model.getCpuPinning().setEntity(vm.getCpuPinning());
Expand Down
8 changes: 7 additions & 1 deletion ...eb/src/main/java/org/ovirt/engine/ui/uicommonweb/builders/vm/NumaUnitToVmBaseBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.util.List;
import java.util.Optional;

import org.ovirt.engine.core.common.businessentities.CpuPinningPolicy;
import org.ovirt.engine.core.common.businessentities.VmBase;
import org.ovirt.engine.core.common.businessentities.VmNumaNode;
import org.ovirt.engine.core.common.utils.NumaUtils;
Expand All @@ -24,8 +25,13 @@ protected void build(UnitVmModel model, VmBase vm) {
return;
}

boolean isNumaPinningAllowed = model.getNumaSupportCommand().getIsExecutionAllowed();

boolean isResizeAndPinDropped = model.getOriginalCpuPinningPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA
&& model.getCpuPinningPolicy().getSelectedItem().getPolicy() != CpuPinningPolicy.RESIZE_AND_PIN_NUMA;

List<VmNumaNode> nodeList = model.getVmNumaNodes();
if (nodeList == null || nodeList.size() != nodeCount) {
if (nodeList == null || nodeList.size() != nodeCount || !isNumaPinningAllowed || isResizeAndPinDropped) {
ahadas marked this conversation as resolved.
Show resolved Hide resolved
nodeList = new ArrayList<>(nodeCount);
for (int i = 0; i < nodeCount; i++) {
VmNumaNode newNode = new VmNumaNode();
Expand Down
3 changes: 1 addition & 2 deletions ...onweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/templates/VmBaseListModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,8 +620,7 @@ public void init(Runnable callback) {
confirmModel.addRecommendationForCpuPinning(isVmAssignedToSpecificHosts, isVmCpuPinningSet);

// Handle NUMA
final boolean isVmVirtNumaSet =
model.getNumaEnabled().getEntity() && model.getNumaNodeCount().getEntity() > 0;
final boolean isVmVirtNumaSet = model.getNumaNodeCount().getEntity() > 0;
final boolean isVmVirtNumaPinned =
model.getVmNumaNodes() != null
&& !model.getVmNumaNodes().isEmpty()
Expand Down
2 changes: 1 addition & 1 deletion ...ain/java/org/ovirt/engine/ui/uicommonweb/models/vms/CloneVmFromSnapshotModelBehavior.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,6 @@ public void initStorageDomains() {
}

@Override
protected void updateNumaEnabled() {
protected void updateNumaFields() {
}
}
24 changes: 7 additions & 17 deletions ...web/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingVmModelBehavior.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public class ExistingVmModelBehavior extends VmModelBehaviorBase<UnitVmModel> {
private int hostCpu;
private VDS runningOnHost;
private Guid bootableDiskStorageDomainId;
private List<VmNumaNode> initialVmNumaNodes = new ArrayList<>();

public ExistingVmModelBehavior(VM vm) {
this.vm = vm;
Expand Down Expand Up @@ -96,18 +97,18 @@ public void initialize() {

if (vm.isNextRunConfigurationExists()) {
getModel().setVmNumaNodes(vm.getvNumaNodeList());
getModel().setInitialVmNumaNodes(vm.getvNumaNodeList());
getModel().updateNodeCount(vm.getvNumaNodeList().size());
getModel().updateCpuPinningPolicy();
initialVmNumaNodes = vm.getvNumaNodeList();
} else {
Frontend.getInstance().runQuery(QueryType.GetVmNumaNodesByVmId,
new IdQueryParameters(vm.getId()),
new AsyncQuery<QueryReturnValue>(returnValue -> {
List<VmNumaNode> nodes = returnValue.getReturnValue();
getModel().setVmNumaNodes(nodes);
getModel().setInitialVmNumaNodes(nodes);
getModel().updateNodeCount(nodes.size());
getModel().updateCpuPinningPolicy();
initialVmNumaNodes = vm.getvNumaNodeList();
}));
}
// load dedicated host names into host names list
Expand Down Expand Up @@ -501,28 +502,17 @@ protected VM getVmWithNuma() {
}

@Override
protected void updateNumaEnabled() {
super.updateNumaEnabled();
protected void updateNumaFields() {
super.updateNumaFields();
super.updateNumaPinningEnabled();

boolean isResizeAndPin =
getModel().getCpuPinningPolicy().getSelectedItem().getPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA;
updateNumaEnabledHelper(!isResizeAndPin);

if (getModel().getVmNumaNodes() != null) {
getModel().getNumaNodeCount().setEntity(getModel().getVmNumaNodes().size());
}

if (isResizeAndPin) {
if (!getModel().getTotalCPUCores().getEntity().equals(Integer.toString(getVm().getNumOfCpus()))) {
getModel().getTotalCPUCores().setEntity(Integer.toString(getVm().getNumOfCpus()));
}

if (!getModel().getNumaNodeCount().getEntity().equals(getModel().getInitialVmNumaNodes().size())) {
getModel().getNumaNodeCount().setEntity(getModel().getInitialVmNumaNodes().size());
}

if (!getModel().getVmNumaNodes().equals(getModel().getInitialVmNumaNodes())) {
getModel().setVmNumaNodes(getModel().getInitialVmNumaNodes());
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions .../main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmFromTemplateModelBehavior.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,10 @@ protected void initClusters(final List<StoragePool> dataCenters) {
false);
}

@Override
protected void updateNumaFields() {
super.updateNumaFields();
updateNumaPinningEnabled();
}

}
6 changes: 3 additions & 3 deletions ...ommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,9 +352,9 @@ public InstanceTypeManager getInstanceTypeManager() {
}

@Override
protected void updateNumaEnabled() {
super.updateNumaEnabled();
updateNumaEnabledHelper(true);
protected void updateNumaFields() {
super.updateNumaFields();
updateNumaPinningEnabled();
}

@Override
Expand Down
Loading