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 <kous...@apache.org> Authored: Tue Nov 4 17:34:07 2014 +0530 Committer: Koushik Das <kous...@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
