Skip to content

Commit

Permalink
core: vnic hot un\plug - avoid race
Browse files Browse the repository at this point in the history
Lack of a lock in ActivateDeactivateVmNicCommand allows several
instances of hot plug and unplug requests to run in parallel causing
inconsistency in vNIC status as well as libvirt failing the requests due
to non-unique aliases. This patch adds a lock to the command so that
each instance of the command has to finish before the next one starts.

Bug-Url: https://bugzilla.redhat.com/2084530
Change-Id: I93ac9f1f12f34526ad10b9a49790c3a2689e8c4d
Signed-off-by: Eitan Raviv <eraviv@redhat.com>
  • Loading branch information
erav committed Jun 14, 2022
1 parent 92cd8cd commit 7796492
Showing 1 changed file with 41 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;

import javax.inject.Inject;

Expand Down Expand Up @@ -54,6 +56,7 @@
import org.ovirt.engine.core.dao.provider.ProviderDao;
import org.ovirt.engine.core.utils.ReplacementUtils;
import org.ovirt.engine.core.utils.StringMapUtils;
import org.ovirt.engine.core.vdsbroker.ResourceManager;
import org.ovirt.engine.core.vdsbroker.monitoring.VmDevicesMonitoring;
import org.ovirt.engine.core.vdsbroker.vdsbroker.VdsProperties;
import org.ovirt.engine.core.vdsbroker.vdsbroker.VmInfoReturn;
Expand Down Expand Up @@ -118,6 +121,9 @@ public class ActivateDeactivateVmNicCommand<T extends ActivateDeactivateVmNicPar
@Inject
private NetworkDao networkDao;

@Inject
private ResourceManager resourceManager;

public ActivateDeactivateVmNicCommand(T parameters, CommandContext commandContext) {
super(parameters, commandContext);
setVmId(parameters.getVmId());
Expand Down Expand Up @@ -248,21 +254,43 @@ public String getInterfaceType() {

@Override
protected void executeVmCommand() {
switch (getParameters().getAction()) {
case PLUG:
plugNic();
break;
case UNPLUG:
unplugNic();
break;
default:
throw new RuntimeException("Coding error: unknown enum value");
boolean hotPlug = getVm().getStatus().isUpOrPaused();
Lock vmDevicesLock = getVmDevicesLock(hotPlug);
vmDevicesLock.lock();
try {
switch (getParameters().getAction()) {
case PLUG:
plugNic();
break;
case UNPLUG:
unplugNic();
break;
default:
throw new RuntimeException("Coding error: unknown enum value");
}

var success = handleFailoverIfNeeded();
// In any case, the device is updated
updateDevice();
setSucceeded(success);
} finally {
vmDevicesLock.unlock();
}
}

var success = handleFailoverIfNeeded();
// In any case, the device is updated
updateDevice();
setSucceeded(success);
protected Lock getVmDevicesLock(boolean runningVmChanges) {
if (!runningVmChanges) {
// dummy lock
return new ReentrantLock() {
@Override
public void lock() {
}
@Override
public void unlock() {
}
};
}
return resourceManager.getVmManager(getVmId()).getVmDevicesLock();
}

private void plugNic() {
Expand Down

0 comments on commit 7796492

Please sign in to comment.