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/e25de54b Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/e25de54b Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/e25de54b Branch: refs/heads/statscollector-graphite Commit: e25de54b4c9cf8f55070b8aaca6494049f1fab0b Parents: e6907ed Author: Koushik Das <kous...@apache.org> Authored: Tue Nov 4 17:34:07 2014 +0530 Committer: Koushik Das <kous...@apache.org> Committed: Sat Nov 8 13:50:15 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/e25de54b/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 4b1597a..d49364f 100755 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -2978,18 +2978,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/e25de54b/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/e25de54b/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 01a692d..1a024a3 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; @@ -1090,15 +1089,16 @@ 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); } 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 @@ -1107,19 +1107,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) { @@ -1135,7 +1144,6 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir s_logger.debug("Successful removal of " + network + " from " + vmInstance); return _vmDao.findById(vmInstance.getId()); - } @Override
