Skip to content

Re-reserve released NICs when a VM resyncs to Running to restore nics_count #14460

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

Draft
Pearl1594 wants to merge 1 commit into
base: 4.22
Choose a base branch
from
+45 −0
Draft

Re-reserve released NICs when a VM resyncs to Running to restore nics_count #14460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions .../java/org/apache/cloudstack/engine/orchestration/service/NetworkOrchestrationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,12 @@ NicProfile prepareNic(VirtualMachineProfile vmProfile, DeployDestination dest, R
throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException, InsufficientCapacityException,
ResourceUnavailableException;

/**
* Re-reserves the Allocated nics of a VM that is Running, e.g. after its nics were released on a power report
* that was later reverted, and restores the active nics count of their networks.
*/
void reconcileNicsForRunningVm(VirtualMachineProfile vmProfile, DeployDestination dest, ReservationContext context);

/**
* Removes the provided nic from the given vm
* @param vm
Expand Down
15 changes: 15 additions & 0 deletions engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5432,6 +5432,7 @@ private void handlePowerOnReportWithNoPendingJobsOnVM(final VMInstanceVO vm) {
ActionEventUtils.onActionEvent(User.UID_SYSTEM, Account.ACCOUNT_ID_SYSTEM, vm.getDomainId(),
EventTypes.EVENT_VM_START, "Out of band VM power on", vm.getId(), getApiCommandResourceTypeForVm(vm).toString());
logger.info("VM {} is sync-ed to at Running state according to power-on report from hypervisor.", vm.getInstanceName());
reconcileNicsForRunningVm(vm);

@DaanHoogland DaanHoogland Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need it in the stopped||stopping block? if the VM never reports as running again we have a spare nic in our nic count. should it be added to the start block instead.

@weizhouapache weizhouapache Oct 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when a running vm is reported as stopped, cloudstack will send a command to force-stop it (I guess it is to ensure the vm is stopped) , and release the nic when done.

as far as I know, nics_count is not used any more to determine if the network need to be shutdown. the goal of this PR is to restore the nic state, not nics_count. PR title should be updated.

break;

case Destroyed:
Expand All @@ -5456,6 +5457,20 @@ private void handlePowerOnReportWithNoPendingJobsOnVM(final VMInstanceVO vm) {
}
}

private void reconcileNicsForRunningVm(final VMInstanceVO vm) {
final VMInstanceVO runningVm = _vmDao.findById(vm.getId());
if (runningVm == null || runningVm.getType() != VirtualMachine.Type.User || runningVm.getState() != State.Running || runningVm.getHostId() == null

@weizhouapache weizhouapache Oct 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the issue impact DomainRouter ?

@Pearl1594 Pearl1594 Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't because if it's not userVM we return.

@weizhouapache weizhouapache Oct 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the VR should not impact the network garbage collector.
is it possble that VR nic is also released when it is reported as Stopped unexpectedly ?

@Pearl1594 Pearl1594 Oct 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh misinterpreted your statement earlier - I thought you meant does the fix impact VRs - my bad 🤦
That said, I dont think VR was VR is impacted by the issue, and even if it did, we never really took into account VRs nic count, so this fix should only be limited to userVMs.

@weizhouapache weizhouapache Oct 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's true indeed

|| _nicsDao.listByVmId(runningVm.getId()).stream().noneMatch(nic -> nic.getState() == Nic.State.Allocated)) {
return;
}
final Host host = _hostDao.findById(runningVm.getHostId());
final DataCenter dc = _entityMgr.findById(DataCenter.class, runningVm.getDataCenterId());
final User user = _entityMgr.findById(User.class, User.UID_SYSTEM);
final Account account = _entityMgr.findById(Account.class, Account.ACCOUNT_ID_SYSTEM);
final ReservationContext context = new ReservationContextImpl(null, null, user, account);
_networkMgr.reconcileNicsForRunningVm(new VirtualMachineProfileImpl(runningVm), new DeployDestination(dc, null, null, host), context);
}

private void handlePowerOffReportWithNoPendingJobsOnVM(final VMInstanceVO vm) {
switch (vm.getState()) {
case Starting:
Expand Down
20 changes: 20 additions & 0 deletions ...tration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2187,6 +2187,26 @@ public int compare(final NicVO nic1, final NicVO nic2) {
}
}

@Override
public void reconcileNicsForRunningVm(final VirtualMachineProfile vmProfile, final DeployDestination dest, final ReservationContext context) {
for (final NicVO nic : _nicDao.listByVmId(vmProfile.getId())) {
if (nic.getState() != Nic.State.Allocated) {
continue;
}
try {
final Pair<NetworkGuru, NetworkVO> implemented = implementNetwork(nic.getNetworkId(), dest, context);
if (implemented == null || implemented.first() == null) {
logger.warn("Unable to implement Network ID={} to reconcile NIC {} of Running Instance {}", nic.getNetworkId(), nic, vmProfile);
continue;
}
prepareNic(vmProfile, dest, context, nic.getId(), implemented.second());
logger.info("Re-reserved NIC {} of Running Instance {}", nic, vmProfile);
} catch (final Exception e) {
logger.warn("Failed to reconcile NIC {} of Running Instance {}", nic, vmProfile, e);
}
}
}

@Override
public NicProfile prepareNic(final VirtualMachineProfile vmProfile, final DeployDestination dest, final ReservationContext context, final long nicId, final Network network)
throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException, InsufficientCapacityException,
Expand Down
4 changes: 4 additions & 0 deletions server/src/test/java/com/cloud/vpc/MockNetworkManagerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,10 @@ public Pair<NicProfile, Integer> allocateNic(NicProfile requested, Network netwo
return null;
}

@Override
public void reconcileNicsForRunningVm(VirtualMachineProfile vmProfile, DeployDestination dest, ReservationContext context) {
}

@Override
public NicProfile prepareNic(VirtualMachineProfile vmProfile, DeployDestination dest, ReservationContext context, long nicId, Network network)
throws InsufficientVirtualNetworkCapacityException, InsufficientAddressCapacityException, ConcurrentOperationException, InsufficientCapacityException,
Expand Down
Loading