CLOUDSTACK-4020: lock nic entry in releaseNic method. Otherwise multiple threads can try to release the same nic at the same time, and it will lead to NPEs and backend failures

Conflicts:
	server/src/com/cloud/network/NetworkManagerImpl.java
	server/src/com/cloud/vm/VirtualMachineManagerImpl.java
This commit is contained in:
Alena Prokharchyk 2013-08-01 15:34:55 -07:00
parent 3587127df6
commit a3b86573b9
2 changed files with 61 additions and 36 deletions

View File

@ -2236,33 +2236,32 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L
ConcurrentOperationException, ResourceUnavailableException {
List<NicVO> nics = _nicDao.listByVmId(vmProfile.getId());
for (NicVO nic : nics) {
releaseNic(vmProfile, nic);
releaseNic(vmProfile, nic.getId());
}
}
@Override
@DB
public void releaseNic(VirtualMachineProfile vmProfile, Nic nic)
throws ConcurrentOperationException, ResourceUnavailableException {
NicVO nicVO = _nicDao.findById(nic.getId());
releaseNic(vmProfile, nicVO);
releaseNic(vmProfile, nic.getId());
}
@DB
protected void releaseNic(VirtualMachineProfile vmProfile, NicVO nicVO)
protected void releaseNic(VirtualMachineProfile vmProfile, long nicId)
throws ConcurrentOperationException, ResourceUnavailableException {
//lock the nic
Transaction txn = Transaction.currentTxn();
txn.start();
NicVO nic = _nicDao.lockRow(nicVO.getId(), true);
NicVO nic = _nicDao.lockRow(nicId, true);
if (nic == null) {
throw new ConcurrentOperationException("Unable to acquire lock on nic " + nic);
}
Nic.State originalState = nic.getState();
NetworkVO network = _networksDao.findById(nicVO.getNetworkId());
NetworkVO network = _networksDao.findById(nic.getNetworkId());
if (originalState == Nic.State.Reserved || originalState == Nic.State.Reserving) {
if (nic.getReservationStrategy() == Nic.ReservationStrategy.Start) {

View File

@ -2953,6 +2953,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
}
@Override
@DB
public boolean removeVmFromNetwork(VirtualMachine vm, Network network, URI broadcastUri) throws ConcurrentOperationException, ResourceUnavailableException {
VMInstanceVO vmVO = _vmDao.findById(vm.getId());
ReservationContext context = new ReservationContextImpl(null, null, _accountMgr.getActiveUser(User.UID_SYSTEM),
@ -2968,53 +2969,78 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
VirtualMachineTO vmTO = hvGuru.implement(vmProfile);
Nic nic = null;
if (broadcastUri != null) {
nic = _nicsDao.findByNetworkIdInstanceIdAndBroadcastUri(network.getId(), vm.getId(), broadcastUri.toString());
} else {
nic = _networkModel.getNicInNetwork(vm.getId(), network.getId());
}
if (nic == null) {
if (nic == null){
s_logger.warn("Could not get a nic with " + network);
return false;
}
// don't delete default NIC on a user VM
if (nic.isDefaultNic() && vm.getType() == VirtualMachine.Type.User) {
s_logger.warn("Failed to remove nic from " + vm + " in " + network + ", nic is default.");
throw new CloudRuntimeException("Failed to remove nic from " + vm + " in " + network + ", nic is default.");
}
NicProfile nicProfile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(),
_networkModel.getNetworkRate(network.getId(), vm.getId()),
_networkModel.isSecurityGroupSupportedInNetwork(network),
_networkModel.getNetworkTag(vmProfile.getVirtualMachine().getHypervisorType(), network));
//1) Unplug the nic
if (vm.getState() == State.Running) {
NicTO nicTO = toNicTO(nicProfile, vmProfile.getVirtualMachine().getHypervisorType());
s_logger.debug("Un-plugging nic for vm " + vm + " from network " + network);
boolean result = unplugNic(network, nicTO, vmTO, context, dest);
if (result) {
s_logger.debug("Nic is unplugged successfully for vm " + vm + " in network " + network);
} else {
s_logger.warn("Failed to unplug nic for the vm " + vm + " from network " + network);
return false;
//Lock on nic is needed here
Nic lock = _nicsDao.acquireInLockTable(nic.getId());
if (lock == null) {
//check if nic is still there. Return if it was released already
if (_nicsDao.findById(nic.getId()) == null) {
if (s_logger.isDebugEnabled()) {
s_logger.debug("Not need to remove the vm " + vm + " from network " + network + " as the vm doesn't have nic in this network");
}
return true;
}
} else if (vm.getState() != State.Stopped) {
s_logger.warn("Unable to remove vm " + vm + " from network " + network);
throw new ResourceUnavailableException("Unable to remove vm " + vm + " from network, is not in the right state",
DataCenter.class, vm.getDataCenterId());
throw new ConcurrentOperationException("Unable to lock nic " + nic.getId());
}
if (s_logger.isDebugEnabled()) {
s_logger.debug("Lock is acquired for nic id " + lock.getId() + " as a part of remove vm " + vm + " from network " + network);
}
try {
NicProfile nicProfile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(),
_networkModel.getNetworkRate(network.getId(), vm.getId()),
_networkModel.isSecurityGroupSupportedInNetwork(network),
_networkModel.getNetworkTag(vmProfile.getVirtualMachine().getHypervisorType(), network));
//2) Release the nic
_networkMgr.releaseNic(vmProfile, nic);
s_logger.debug("Successfully released nic " + nic + "for vm " + vm);
//1) Unplug the nic
if (vm.getState() == State.Running) {
NicTO nicTO = toNicTO(nicProfile, vmProfile.getVirtualMachine().getHypervisorType());
s_logger.debug("Un-plugging nic for vm " + vm + " from network " + network);
boolean result = unplugNic(network, nicTO, vmTO, context, dest);
if (result) {
s_logger.debug("Nic is unplugged successfully for vm " + vm + " in network " + network );
} else {
s_logger.warn("Failed to unplug nic for the vm " + vm + " from network " + network);
return false;
}
} else if (vm.getState() != State.Stopped) {
s_logger.warn("Unable to remove vm " + vm + " from network " + network);
throw new ResourceUnavailableException("Unable to remove vm " + vm + " from network, is not in the right state",
DataCenter.class, vm.getDataCenterId());
}
//3) Remove the nic
_networkMgr.removeNic(vmProfile, nic);
return true;
//2) Release the nic
_networkMgr.releaseNic(vmProfile, nic);
s_logger.debug("Successfully released nic " + nic + "for vm " + vm);
//3) Remove the nic
_networkMgr.removeNic(vmProfile, nic);
return true;
} finally {
if (lock != null) {
_nicsDao.releaseFromLockTable(lock.getId());
if (s_logger.isDebugEnabled()) {
s_logger.debug("Lock is released for nic id " + lock.getId() + " as a part of remove vm " + vm + " from network " + network);
}
}
}
}
@Override