You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ko...@apache.org on 2014/11/04 13:07:26 UTC

git commit: updated refs/heads/master to 0327c2b

Repository: cloudstack
Updated Branches:
  refs/heads/master 1d503bb85 -> 0327c2b13


CLOUDSTACK-7421
Unnecessary exception in MS logs while removing default NIC from VM. Following changes are made:
1. Changed the exception from CloudRuntimeException to InvalidParameterValueExecption.
2. Moved out validation logic to UserVMManagerImpl from VirtualMachineManagerImpl.
3. Handling InvalidParameterValueException from async API calls so that they are not logged as ERROR in MS logs.


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

Branch: refs/heads/master
Commit: 0327c2b13e9712a63d865ff755a9fec421eb0af5
Parents: 1d503bb
Author: Koushik Das <ko...@apache.org>
Authored: Tue Nov 4 17:34:07 2014 +0530
Committer: Koushik Das <ko...@apache.org>
Committed: Tue Nov 4 17:34:07 2014 +0530

----------------------------------------------------------------------
 .../com/cloud/vm/VirtualMachineManagerImpl.java | 12 ---------
 .../com/cloud/api/ApiAsyncJobDispatcher.java    |  4 ++-
 server/src/com/cloud/vm/UserVmManagerImpl.java  | 26 +++++++++++++-------
 3 files changed, 20 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0327c2b1/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
----------------------------------------------------------------------
diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
index 2b244ef..5096ed4 100755
--- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
+++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java
@@ -2974,18 +2974,6 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac
         HypervisorGuru hvGuru = _hvGuruMgr.getGuru(vmProfile.getVirtualMachine().getHypervisorType());
         VirtualMachineTO vmTO = hvGuru.implement(vmProfile);
 
-        // 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.");
-        }
-
-        // if specified nic is associated with PF/LB/Static NAT
-        if (rulesMgr.listAssociatedRulesForGuestNic(nic).size() > 0) {
-            throw new CloudRuntimeException("Failed to remove nic from " + vm + " in " + network +
-                    ", nic has associated Port forwarding or Load balancer or Static NAT rules.");
-        }
-
         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));

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0327c2b1/server/src/com/cloud/api/ApiAsyncJobDispatcher.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/api/ApiAsyncJobDispatcher.java b/server/src/com/cloud/api/ApiAsyncJobDispatcher.java
index 71adf2a..0b7d681 100644
--- a/server/src/com/cloud/api/ApiAsyncJobDispatcher.java
+++ b/server/src/com/cloud/api/ApiAsyncJobDispatcher.java
@@ -37,6 +37,7 @@ import org.apache.cloudstack.framework.jobs.AsyncJobDispatcher;
 import org.apache.cloudstack.framework.jobs.AsyncJobManager;
 import org.apache.cloudstack.jobs.JobInfo;
 
+import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.user.Account;
 import com.cloud.user.User;
 import com.cloud.utils.component.AdapterBase;
@@ -77,7 +78,6 @@ public class ApiAsyncJobDispatcher extends AdapterBase implements AsyncJobDispat
             String acctIdStr = params.get("ctxAccountId");
             String contextDetails = params.get("ctxDetails");
 
-
             Long userId = null;
             Account accountObject = null;
 
@@ -109,6 +109,8 @@ public class ApiAsyncJobDispatcher extends AdapterBase implements AsyncJobDispat
 
                 // serialize this to the async job table
                 _asyncJobMgr.completeAsyncJob(job.getId(), JobInfo.Status.SUCCEEDED, 0, ApiSerializerHelper.toSerializedString(cmdObj.getResponseObject()));
+            } catch (InvalidParameterValueException ipve) {
+                throw new ServerApiException(ApiErrorCode.PARAM_ERROR, ipve.getMessage());
             } finally {
                 CallContext.unregister();
             }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/0327c2b1/server/src/com/cloud/vm/UserVmManagerImpl.java
----------------------------------------------------------------------
diff --git a/server/src/com/cloud/vm/UserVmManagerImpl.java b/server/src/com/cloud/vm/UserVmManagerImpl.java
index abb5b7b..018c062 100755
--- a/server/src/com/cloud/vm/UserVmManagerImpl.java
+++ b/server/src/com/cloud/vm/UserVmManagerImpl.java
@@ -37,7 +37,6 @@ import javax.naming.ConfigurationException;
 
 import org.apache.commons.codec.binary.Base64;
 import org.apache.log4j.Logger;
-
 import org.apache.cloudstack.acl.ControlledEntity.ACLType;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
 import org.apache.cloudstack.affinity.AffinityGroupService;
@@ -1074,7 +1073,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
         UserVmVO vmInstance = _vmDao.findById(vmId);
         if (vmInstance == null) {
-            throw new InvalidParameterValueException("unable to find a virtual machine with id " + vmId);
+            throw new InvalidParameterValueException("Unable to find a virtual machine with id " + vmId);
         }
 
         // Check that Vm does not have VM Snapshots
@@ -1084,11 +1083,12 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
         NicVO nic = _nicDao.findById(nicId);
         if (nic == null) {
-            throw new InvalidParameterValueException("unable to find a nic with id " + nicId);
+            throw new InvalidParameterValueException("Unable to find a nic with id " + nicId);
         }
+
         NetworkVO network = _networkDao.findById(nic.getNetworkId());
         if (network == null) {
-            throw new InvalidParameterValueException("unable to find a network with id " + nic.getNetworkId());
+            throw new InvalidParameterValueException("Unable to find a network with id " + nic.getNetworkId());
         }
 
         // Perform permission check on VM
@@ -1097,19 +1097,28 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
         // Verify that zone is not Basic
         DataCenterVO dc = _dcDao.findById(vmInstance.getDataCenterId());
         if (dc.getNetworkType() == DataCenter.NetworkType.Basic) {
-            throw new CloudRuntimeException("Zone " + vmInstance.getDataCenterId() + ", has a NetworkType of Basic. Can't remove a NIC from a VM on a Basic Network");
+            throw new InvalidParameterValueException("Zone " + vmInstance.getDataCenterId() + ", has a NetworkType of Basic. Can't remove a NIC from a VM on a Basic Network");
         }
 
-        //check to see if nic is attached to VM
+        // check to see if nic is attached to VM
         if (nic.getInstanceId() != vmId) {
-            throw new InvalidParameterValueException(nic + " is not a nic on  " + vmInstance);
+            throw new InvalidParameterValueException(nic + " is not a nic on " + vmInstance);
         }
 
         // Perform account permission check on network
         _accountMgr.checkAccess(caller, AccessType.UseEntry, false, network);
 
-        boolean nicremoved = false;
+        // don't delete default NIC on a user VM
+        if (nic.isDefaultNic() && vmInstance.getType() == VirtualMachine.Type.User) {
+            throw new InvalidParameterValueException("Unable to remove nic from " + vmInstance + " in " + network + ", nic is default.");
+        }
+
+        // if specified nic is associated with PF/LB/Static NAT
+        if (_rulesMgr.listAssociatedRulesForGuestNic(nic).size() > 0) {
+            throw new InvalidParameterValueException("Unable to remove nic from " + vmInstance + " in " + network + ", nic has associated Port forwarding or Load balancer or Static NAT rules.");
+        }
 
+        boolean nicremoved = false;
         try {
             nicremoved = _itMgr.removeNicFromVm(vmInstance, nic);
         } catch (ResourceUnavailableException e) {
@@ -1125,7 +1134,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
 
         s_logger.debug("Successful removal of " + network + " from " + vmInstance);
         return _vmDao.findById(vmInstance.getId());
-
     }
 
     @Override