Skip to content

Commit

Permalink
engine: short summary under 50 chars
Browse files Browse the repository at this point in the history
When updating a VM, it was required to send updateNuma
parameter that determined if the numa node changes will be saved.

The UI had to handle the logic which started to be quite complicated
- the numa nodes changes with memory, cpu, hugepages, numa count,
numa pinning, numa tune mode change.

This patch adds the possibility to leave out that parameter and
in that case the new numa node list is compared to the one
in the database. If they differ, the new numa node list will
be saved.
  • Loading branch information
ljelinkova committed Jun 28, 2022
1 parent 2f64015 commit c7160ef
Show file tree
Hide file tree
Showing 6 changed files with 16 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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 @@ -953,6 +954,14 @@ private void initNuma() {
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()));
}

// we always need to verify new or existing numa nodes with the updated VM configuration
if (!getParameters().isUpdateNuma()) {
getParameters().getVm().setvNumaNodeList(vNumaNodeList);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ public void setValue(T value) {
private VM vm;
private boolean copyTemplatePermissions;
private boolean applyChangesLater;
private boolean updateNuma;
// if updateNuma = null, it will be calculated in the commands
private Boolean updateNuma;
private String vmLargeIcon;
private Version clusterLevelChangeFromVersion;
private Map<VmExternalDataKind, String> vmExternalData;
Expand Down Expand Up @@ -349,11 +350,11 @@ public void setApplyChangesLater(boolean applyChangesLater) {
* Since NUMA configuration can be updated, this flag indicates whether client
* sends NUMA info that needs to be updated.
*/
public boolean isUpdateNuma() {
public Boolean isUpdateNuma() {
return updateNuma;
}

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

Expand Down
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
Original file line number Diff line number Diff line change
Expand Up @@ -513,14 +513,6 @@ protected void updateNumaFields() {
if (!getModel().getTotalCPUCores().getEntity().equals(Integer.toString(getVm().getNumOfCpus()))) {
getModel().getTotalCPUCores().setEntity(Integer.toString(getVm().getNumOfCpus()));
}

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

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

Expand All @@ -530,41 +522,4 @@ public void updateMaxMemory() {
super.updateMaxMemory();
}
}

/**
* Returns true if the NUMA should be updated on the backend.
*/
@Override
public boolean shouldUpdateNuma() {

if (getModel().getCpuPinningPolicy().getSelectedItem().getPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA) {
return false;
}

if (vm.getCpuPinningPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA && getModel().getCpuPinningPolicy()
.getSelectedItem()
.getPolicy() != CpuPinningPolicy.RESIZE_AND_PIN_NUMA) {
return true;
}

if (!getModel().getNumaNodeCount().getEntity().equals(initialVmNumaNodes.size())) {
return true;
}

if (!getModel().getVmNumaNodes().equals(initialVmNumaNodes)) {
return true;
}

if (getModel().getVmNumaNodes().size() > 0) {
if (!getModel().getMemSize().getEntity().equals(vm.getMemSizeMb())) {
return true;
}

if (!getModel().getTotalCPUCores().getEntity().equals(Integer.toString(vm.getNumOfCpus()))) {
return true;
}
}

return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1884,7 +1884,8 @@ public VmManagementParametersBase fillVmManagementParameters(VmManagementParamet
params.setTpmEnabled(model.getTpmEnabled().getEntity());
params.setConsoleEnabled(model.getIsConsoleDeviceEnabled().getEntity());
params.setVirtioScsiEnabled(model.getIsVirtioScsiEnabled().getEntity());
params.setUpdateNuma(model.getBehavior().shouldUpdateNuma());
// it will be calculated on the backend
params.setUpdateNuma(null);
params.setAffinityGroups(model.getAffinityGroupList().getSelectedItems());
params.setAffinityLabels(model.getLabelList().getSelectedItems());
if (model.getIsHeadlessModeEnabled().getEntity()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1874,8 +1874,4 @@ protected DisplayType getDefaultDisplayType(Set<DisplayType> displayTypes) {
}
return displayTypes.iterator().next();
}

public boolean shouldUpdateNuma() {
return getModel().getNumaNodeCount().getEntity() > 0;
}
}

0 comments on commit c7160ef

Please sign in to comment.