cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From alena1...@apache.org
Subject git commit: updated refs/heads/4.2 to 59fedb1
Date Thu, 01 Aug 2013 23:28:05 GMT
Updated Branches:
  refs/heads/4.2 a059f596a -> 59fedb1bd


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


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/59fedb1b
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/59fedb1b
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/59fedb1b

Branch: refs/heads/4.2
Commit: 59fedb1bdc76837fc1a1d83bfcb12580d870869d
Parents: a059f59
Author: Alena Prokharchyk <alena.prokharchyk@citrix.com>
Authored: Thu Aug 1 15:34:55 2013 -0700
Committer: Alena Prokharchyk <alena.prokharchyk@citrix.com>
Committed: Thu Aug 1 16:02:35 2013 -0700

----------------------------------------------------------------------
 .../com/cloud/network/NetworkManagerImpl.java   | 13 ++--
 .../com/cloud/vm/VirtualMachineManagerImpl.java | 82 +++++++++++++-------
 2 files changed, 60 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/59fedb1b/server/src/com/cloud/network/NetworkManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java
index d2761b0..b5a1ca7 100755
--- a/server/src/com/cloud/network/NetworkManagerImpl.java
+++ b/server/src/com/cloud/network/NetworkManagerImpl.java
@@ -2313,33 +2313,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<? extends VMInstanceVO> vmProfile,
Nic nic)
             throws ConcurrentOperationException, ResourceUnavailableException {
-        NicVO nicVO = _nicDao.findById(nic.getId());
-        releaseNic(vmProfile, nicVO);
+        releaseNic(vmProfile, nic.getId());
     }
 
     @DB
-    protected void releaseNic(VirtualMachineProfile<? extends VMInstanceVO> vmProfile,
NicVO nicVO)
+    protected void releaseNic(VirtualMachineProfile<? extends VMInstanceVO> 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) {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/59fedb1b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
index eedf4d2..6d35539 100755
--- a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
+++ b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -3071,6 +3071,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),
@@ -3087,53 +3088,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){
             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 = vmGuru.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));
+
+            //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 = vmGuru.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());
+            }
 
-        //2) Release the nic
-        _networkMgr.releaseNic(vmProfile, nic);
-        s_logger.debug("Successfully released nic " + nic +  "for vm " + vm);
+            //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;
+            //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


Mime
View raw message