Skip to content

Commit

Permalink
core: reset numa configuration with resize and pin
Browse files Browse the repository at this point in the history
When running a `Resize and Pin NUMA` VM, we create the virtual NUMA
nodes including the pinning to the physical NUMA nodes of the host.
This is a static configuration that being set on the VM.
Until now, this configuration harmed many flows to update the VM
configuration and a manual workaround needed to overcome it. Now the
NUMA configuration will be deleted once the VM shutdown and when
updating the VM, it will see the current configuration as there is no
NUMA to that VM.

Change-Id: I7d6e8200e7830dc6903a76ac219225fa359d2c53
Bug-Url: https://bugzilla.redhat.com/2074525
Signed-off-by: Liran Rotenberg <lrotenbe@redhat.com>
  • Loading branch information
liranr23 committed Jul 12, 2022
1 parent af4ac85 commit a1b7dca
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
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,15 +952,16 @@ private void updateVmNumaNodes() {
}

private void initNuma() {
if (VmCpuCountHelper.isResizeAndPinPolicy(getParameters().getVm())) {
getParameters().getVm().setvNumaNodeList(new ArrayList<>());
getParameters().setUpdateNuma(false);
return;
}
List<VmNumaNode> vNumaNodeList = vmNumaNodeDao.getAllVmNumaNodeByVmId(getParameters().getVmId());
if (getVm() != null) {
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()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,12 @@ public interface VmNumaNodeDao extends Dao {
* the numa node ids to be removed
*/
void massRemoveNumaNodeByNumaNodeId(List<Guid> numaNodeIds);

/**
* Remove all numa nodes based on the VM ID.
*
* @param vmIds
* a list of vm ids
*/
void massRemoveAllNumaNodeByVmId(List<Guid> vmIds);
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ public void massUpdateNumaNode(List<VmNumaNode> numaNodes) {
insertNumaNodeMap(numaNodes);
}

@Override
public void massRemoveAllNumaNodeByVmId(List<Guid> vmIds) {
getCallsHandler().executeStoredProcAsBatch("DeleteNumaNodeByVmId", vmIds.stream()
.map(vmId -> getCustomMapSqlParameterSource().addValue("vm_id", vmId))
.collect(Collectors.toList()));
}


private Map<Guid, List<Integer>> getAllNumaNodeCpuMap() {
List<Pair<Guid, Integer>> numaNodesCpus =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

import org.ovirt.engine.core.common.AuditLogType;
import org.ovirt.engine.core.common.action.SaveVmExternalDataParameters;
import org.ovirt.engine.core.common.businessentities.CpuPinningPolicy;
import org.ovirt.engine.core.common.businessentities.OriginType;
import org.ovirt.engine.core.common.businessentities.UnchangeableByVdsm;
import org.ovirt.engine.core.common.businessentities.VDSStatus;
Expand Down Expand Up @@ -85,6 +86,7 @@ public class VmAnalyzer {
private boolean vmBalloonDriverNotRequestedOrAvailable;
private boolean guestAgentDownAndBalloonInfalted;
private boolean guestAgentUpOrBalloonDeflated;
private boolean numaClear;
private List<VmJob> vmJobs;
private VmStatistics statistics;
private List<VmNetworkInterface> ifaces;
Expand Down Expand Up @@ -429,6 +431,7 @@ private void clearVm(VmExitStatus exitStatus, String exitMessage, VmExitReason v
alreadyDown ? dbVm.getExitMessage() : exitMessage,
alreadyDown ? dbVm.getExitReason() : vmExitReason);
dbVm.setStopReason(getVmManager().getStopReason(getVmId()));
numaClear = true;
}
saveDynamic(dbVm);
resetVmStatistics();
Expand Down Expand Up @@ -476,6 +479,7 @@ private void abortVmMigration(VmExitStatus exitStatus, String exitMessage, VmExi
auditVmMigrationAbort(exitMessage);
resourceManager.removeAsyncRunningVm(dbVm.getId());
movedToDown = true;
numaClear = true;
}

private void auditVmMigrationAbort(String exitMessage) {
Expand Down Expand Up @@ -1129,6 +1133,18 @@ public boolean isRemoveFromAsync() {
return removeFromAsync;
}

private boolean isNumaClear() {
return numaClear;
}

public Guid getClearNumaVmId() {
return isNumaClear() && isResizeAndPin() ? getVmId() : null;
}

private boolean isResizeAndPin() {
return getVmManager().getCpuPinningPolicy() == CpuPinningPolicy.RESIZE_AND_PIN_NUMA;
}

protected VmManager getVmManager() {
return resourceManager.getVmManager(dbVm.getId());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.ovirt.engine.core.dao.VdsDynamicDao;
import org.ovirt.engine.core.dao.VmDynamicDao;
import org.ovirt.engine.core.dao.VmGuestAgentInterfaceDao;
import org.ovirt.engine.core.dao.VmNumaNodeDao;
import org.ovirt.engine.core.dao.VmStatisticsDao;
import org.ovirt.engine.core.dao.network.VmNetworkInterfaceDao;
import org.ovirt.engine.core.dao.network.VmNetworkStatisticsDao;
Expand Down Expand Up @@ -65,6 +66,8 @@ public class VmsMonitoring {
private VmNetworkInterfaceDao vmNetworkInterfaceDao;
@Inject
private VdsDynamicDao vdsDynamicDao;
@Inject
private VmNumaNodeDao vmNumaNodeDao;

private static final Logger log = LoggerFactory.getLogger(VmsMonitoring.class);

Expand Down Expand Up @@ -289,6 +292,7 @@ private void flush(List<VmAnalyzer> vmAnalyzers) {
saveVmStatistics(vmAnalyzers);
saveVmInterfaceStatistics(vmAnalyzers);
saveVmDiskImageStatistics(vmAnalyzers);
clearVmNuma(vmAnalyzers);
}

private void saveVmDiskImageStatistics(List<VmAnalyzer> vmAnalyzers) {
Expand Down Expand Up @@ -326,6 +330,15 @@ private void saveVmStatistics(List<VmAnalyzer> vmAnalyzers) {
});
}

private void clearVmNuma(List<VmAnalyzer> vmAnalyzers) {
List<Guid> vmIds = vmAnalyzers.stream()
.map(VmAnalyzer::getClearNumaVmId)
.filter(Objects::nonNull)
.collect(Collectors.toList());

vmNumaNodeDao.massRemoveAllNumaNodeByVmId(vmIds);
}

protected void addUnmanagedVms(List<VmAnalyzer> vmAnalyzers, Guid vdsId) {
List<Guid> unmanagedVmIds = vmAnalyzers.stream()
.filter(VmAnalyzer::isUnmanagedVm)
Expand Down
13 changes: 13 additions & 0 deletions packaging/dbscripts/numa_sp.sql
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,19 @@ BEGIN
END;$PROCEDURE$
LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION DeleteNumaNodeByVmId (v_vm_id UUID)
RETURNS VOID AS $PROCEDURE$
BEGIN
BEGIN
DELETE
FROM numa_node
WHERE vm_id = v_vm_id;
END;

RETURN;
END;$PROCEDURE$
LANGUAGE plpgsql;

CREATE OR REPLACE FUNCTION GetNumaNodeByVdsId (v_vds_id UUID)
RETURNS SETOF numa_node_cpus_view STABLE AS $PROCEDURE$
BEGIN
Expand Down

0 comments on commit a1b7dca

Please sign in to comment.