From 22a1b8c3fc1221d706af0d79509fef2bfa1554fa Mon Sep 17 00:00:00 2001 From: Kelven Yang Date: Fri, 3 Jan 2014 17:20:09 -0800 Subject: [PATCH] CLOUDSTACK-5672: Fix VM work job serialization issues in Add/Remove nic --- api/src/com/cloud/network/Network.java | 7 +- api/src/com/cloud/vm/NicProfile.java | 80 ++++++++++--------- .../cloud/vm/VirtualMachineManagerImpl.java | 76 ++++++++---------- .../com/cloud/vm/VmWorkAddVmToNetwork.java | 20 +++-- .../com/cloud/vm/VmWorkRemoveNicFromVm.java | 16 ++-- 5 files changed, 98 insertions(+), 101 deletions(-) diff --git a/api/src/com/cloud/network/Network.java b/api/src/com/cloud/network/Network.java index 4eadd61aa25..0d916ab6af4 100644 --- a/api/src/com/cloud/network/Network.java +++ b/api/src/com/cloud/network/Network.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.network; +import java.io.Serializable; import java.net.URI; import java.util.ArrayList; import java.util.List; @@ -33,7 +34,7 @@ import com.cloud.utils.fsm.StateObject; /** * owned by an account. */ -public interface Network extends ControlledEntity, StateObject, InternalIdentity, Identity { +public interface Network extends ControlledEntity, StateObject, InternalIdentity, Identity, Serializable { public enum GuestType { Shared, @@ -248,7 +249,7 @@ public interface Network extends ControlledEntity, StateObject, I public class IpAddresses { private String ip4Address; private String ip6Address; - + public IpAddresses(String ip4Address, String ip6Address) { setIp4Address(ip4Address); setIp6Address(ip6Address); @@ -270,7 +271,7 @@ public interface Network extends ControlledEntity, StateObject, I this.ip6Address = ip6Address; } } - + String getName(); Mode getMode(); diff --git a/api/src/com/cloud/vm/NicProfile.java b/api/src/com/cloud/vm/NicProfile.java index 5970ccd24ee..ec9c2fc1514 100644 --- a/api/src/com/cloud/vm/NicProfile.java +++ b/api/src/com/cloud/vm/NicProfile.java @@ -16,6 +16,7 @@ // under the License. package com.cloud.vm; +import java.io.Serializable; import java.net.URI; import org.apache.cloudstack.api.InternalIdentity; @@ -27,7 +28,9 @@ import com.cloud.network.Networks.Mode; import com.cloud.network.Networks.TrafficType; import com.cloud.vm.Nic.ReservationStrategy; -public class NicProfile implements InternalIdentity { +public class NicProfile implements InternalIdentity, Serializable { + private static final long serialVersionUID = 4997005771736090304L; + long id; long networkId; BroadcastDomainType broadcastType; @@ -162,6 +165,7 @@ public class NicProfile implements InternalIdentity { return vmId; } + @Override public long getId() { return id; } @@ -216,28 +220,28 @@ public class NicProfile implements InternalIdentity { public NicProfile(Nic nic, Network network, URI broadcastUri, URI isolationUri, Integer networkRate, boolean isSecurityGroupEnabled, String name) { - this.id = nic.getId(); - this.networkId = network.getId(); - this.gateway = nic.getGateway(); - this.mode = network.getMode(); - this.broadcastType = network.getBroadcastDomainType(); - this.trafficType = network.getTrafficType(); - this.ip4Address = nic.getIp4Address(); - this.format = nic.getAddressFormat(); - this.ip6Address = nic.getIp6Address(); - this.macAddress = nic.getMacAddress(); - this.reservationId = nic.getReservationId(); - this.strategy = nic.getReservationStrategy(); - this.deviceId = nic.getDeviceId(); - this.defaultNic = nic.isDefaultNic(); + id = nic.getId(); + networkId = network.getId(); + gateway = nic.getGateway(); + mode = network.getMode(); + broadcastType = network.getBroadcastDomainType(); + trafficType = network.getTrafficType(); + ip4Address = nic.getIp4Address(); + format = nic.getAddressFormat(); + ip6Address = nic.getIp6Address(); + macAddress = nic.getMacAddress(); + reservationId = nic.getReservationId(); + strategy = nic.getReservationStrategy(); + deviceId = nic.getDeviceId(); + defaultNic = nic.isDefaultNic(); this.broadcastUri = broadcastUri; this.isolationUri = isolationUri; - this.netmask = nic.getNetmask(); + netmask = nic.getNetmask(); this.isSecurityGroupEnabled = isSecurityGroupEnabled; - this.vmId = nic.getInstanceId(); + vmId = nic.getInstanceId(); this.name = name; - this.ip6Cidr = nic.getIp6Cidr(); - this.ip6Gateway = nic.getIp6Gateway(); + ip6Cidr = nic.getIp6Cidr(); + ip6Gateway = nic.getIp6Gateway(); if (networkRate != null) { this.networkRate = networkRate; @@ -245,7 +249,7 @@ public class NicProfile implements InternalIdentity { } public NicProfile(ReservationStrategy strategy, String ip4Address, String macAddress, String gateway, String netmask) { - this.format = AddressFormat.Ip4; + format = AddressFormat.Ip4; this.ip4Address = ip4Address; this.macAddress = macAddress; this.gateway = gateway; @@ -274,11 +278,11 @@ public class NicProfile implements InternalIdentity { } public boolean isSecurityGroupEnabled() { - return this.isSecurityGroupEnabled; + return isSecurityGroupEnabled; } public void setSecurityGroupEnabled(boolean enabled) { - this.isSecurityGroupEnabled = enabled; + isSecurityGroupEnabled = enabled; } public String getRequestedIpv4() { @@ -286,22 +290,22 @@ public class NicProfile implements InternalIdentity { } public void deallocate() { - this.gateway = null; - this.mode = null; - this.format = null; - this.broadcastType = null; - this.trafficType = null; - this.ip4Address = null; - this.ip6Address = null; - this.macAddress = null; - this.reservationId = null; - this.strategy = null; - this.deviceId = null; - this.broadcastUri = null; - this.isolationUri = null; - this.netmask = null; - this.dns1 = null; - this.dns2 = null; + gateway = null; + mode = null; + format = null; + broadcastType = null; + trafficType = null; + ip4Address = null; + ip6Address = null; + macAddress = null; + reservationId = null; + strategy = null; + deviceId = null; + broadcastUri = null; + isolationUri = null; + netmask = null; + dns1 = null; + dns2 = null; } @Override diff --git a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java index d745692787d..9ac549c539d 100755 --- a/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/engine/orchestration/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -59,7 +59,6 @@ import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.cloudstack.framework.jobs.Outcome; import org.apache.cloudstack.framework.jobs.dao.VmWorkJobDao; import org.apache.cloudstack.framework.jobs.impl.AsyncJobVO; -import org.apache.cloudstack.framework.jobs.impl.JobSerializerHelper; import org.apache.cloudstack.framework.jobs.impl.OutcomeImpl; import org.apache.cloudstack.framework.jobs.impl.VmWorkJobVO; import org.apache.cloudstack.framework.messagebus.MessageBus; @@ -3126,25 +3125,21 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac throw new RuntimeException("Execution excetion", e); } - AsyncJobVO jobVo = _entityMgr.findById(AsyncJobVO.class, outcome.getJob().getId()); - if(jobVo.getResultCode() == JobInfo.Status.SUCCEEDED.ordinal()) { + Object jobException = _jobMgr.unmarshallResultObject(outcome.getJob()); + if (jobException != null) { + if (jobException instanceof ResourceUnavailableException) + throw (ResourceUnavailableException)jobException; + else if (jobException instanceof ConcurrentOperationException) + throw (ConcurrentOperationException)jobException; + else if (jobException instanceof InsufficientCapacityException) + throw (InsufficientCapacityException)jobException; + else if (jobException instanceof RuntimeException) + throw (RuntimeException)jobException; + else if (jobException instanceof Long) + return requested; + } - NicProfile nic = (NicProfile)JobSerializerHelper.fromObjectSerializedString(jobVo.getResult()); - return nic; - } else { - Object jobException = _jobMgr.unmarshallResultObject(outcome.getJob()); - if(jobException != null) { - if(jobException instanceof ResourceUnavailableException) - throw (ResourceUnavailableException)jobException; - else if(jobException instanceof ConcurrentOperationException) - throw (ConcurrentOperationException)jobException; - else if(jobException instanceof InsufficientCapacityException) - throw (InsufficientCapacityException)jobException; - else if(jobException instanceof RuntimeException) - throw (RuntimeException)jobException; - } - throw new RuntimeException("Job failed with unhandled exception"); - } + throw new RuntimeException("Unexpected job execution result"); } } @@ -3233,24 +3228,19 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac throw new RuntimeException("Execution excetion", e); } - AsyncJobVO jobVo = _entityMgr.findById(AsyncJobVO.class, outcome.getJob().getId()); + Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob()); + if (jobResult != null) { + if (jobResult instanceof ResourceUnavailableException) + throw (ResourceUnavailableException)jobResult; + else if (jobResult instanceof ConcurrentOperationException) + throw (ConcurrentOperationException)jobResult; + else if (jobResult instanceof RuntimeException) + throw (RuntimeException)jobResult; + else if (jobResult instanceof Boolean) + return (Boolean)jobResult; + } - if(jobVo.getResultCode() == JobInfo.Status.SUCCEEDED.ordinal()) { - Boolean result = (Boolean)JobSerializerHelper.fromObjectSerializedString(jobVo.getResult()); - return result; - } else { - Object jobResult = _jobMgr.unmarshallResultObject(outcome.getJob()); - if (jobResult != null) { - if (jobResult instanceof ResourceUnavailableException) - throw (ResourceUnavailableException)jobResult; - else if (jobResult instanceof ConcurrentOperationException) - throw (ConcurrentOperationException)jobResult; - else if (jobResult instanceof RuntimeException) - throw (RuntimeException)jobResult; - } - - throw new RuntimeException("Job failed with un-handled exception"); - } + throw new RuntimeException("Job failed with un-handled exception"); } } @@ -4558,7 +4548,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // save work context info (there are some duplications) VmWorkAddVmToNetwork workInfo = new VmWorkAddVmToNetwork(user.getId(), account.getId(), vm.getId(), - VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER, network, requested); + VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER, network.getId(), requested); workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo)); _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vm.getId()); @@ -4609,7 +4599,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac // save work context info (there are some duplications) VmWorkRemoveNicFromVm workInfo = new VmWorkRemoveNicFromVm(user.getId(), account.getId(), vm.getId(), - VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER, nic); + VirtualMachineManagerImpl.VM_WORK_JOB_HANDLER, nic.getId()); workJob.setCmdInfo(VmWorkSerializer.serialize(workInfo)); _jobMgr.submitAsyncJob(workJob, VmWorkConstants.VM_WORK_QUEUE, vm.getId()); @@ -4803,9 +4793,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.info("Unable to find vm " + work.getVmId()); } assert (vm != null); - NicProfile nic = orchestrateAddVmToNetwork(vm, work.getNetwork(), + + Network network = _networkDao.findById(work.getNetworkId()); + NicProfile nic = orchestrateAddVmToNetwork(vm, network, work.getRequestedNicProfile()); - return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(nic)); + + return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(new Long(nic.getId()))); } private Pair orchestrateRemoveNicFromVm(VmWorkRemoveNicFromVm work) throws Exception { @@ -4814,7 +4807,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac s_logger.info("Unable to find vm " + work.getVmId()); } assert (vm != null); - boolean result = orchestrateRemoveNicFromVm(vm, work.getNic()); + NicVO nic = _entityMgr.findById(NicVO.class, work.getNicId()); + boolean result = orchestrateRemoveNicFromVm(vm, nic); return new Pair(JobInfo.Status.SUCCEEDED, _jobMgr.marshallResultObject(new Boolean(result))); } diff --git a/engine/orchestration/src/com/cloud/vm/VmWorkAddVmToNetwork.java b/engine/orchestration/src/com/cloud/vm/VmWorkAddVmToNetwork.java index 25858e41bc6..e1657cb349f 100644 --- a/engine/orchestration/src/com/cloud/vm/VmWorkAddVmToNetwork.java +++ b/engine/orchestration/src/com/cloud/vm/VmWorkAddVmToNetwork.java @@ -16,26 +16,24 @@ // under the License. package com.cloud.vm; -import com.cloud.network.Network; - public class VmWorkAddVmToNetwork extends VmWork { private static final long serialVersionUID = 8861516006586736813L; - Network network; + Long networkId; NicProfile requstedNicProfile; - + public VmWorkAddVmToNetwork(long userId, long accountId, long vmId, String handlerName, - Network network, NicProfile requested) { + Long networkId, NicProfile requested) { super(userId, accountId, vmId, handlerName); - - this.network = network; + + this.networkId = networkId; requstedNicProfile = requested; } - - public Network getNetwork() { - return network; + + public Long getNetworkId() { + return networkId; } - + public NicProfile getRequestedNicProfile() { return requstedNicProfile; } diff --git a/engine/orchestration/src/com/cloud/vm/VmWorkRemoveNicFromVm.java b/engine/orchestration/src/com/cloud/vm/VmWorkRemoveNicFromVm.java index 3dfbf2706e4..67f5ad42056 100644 --- a/engine/orchestration/src/com/cloud/vm/VmWorkRemoveNicFromVm.java +++ b/engine/orchestration/src/com/cloud/vm/VmWorkRemoveNicFromVm.java @@ -19,15 +19,15 @@ package com.cloud.vm; public class VmWorkRemoveNicFromVm extends VmWork { private static final long serialVersionUID = -4265657031064437923L; - Nic nic; - - public VmWorkRemoveNicFromVm(long userId, long accountId, long vmId, String handlerName, Nic nic) { + Long nicId; + + public VmWorkRemoveNicFromVm(long userId, long accountId, long vmId, String handlerName, Long nicId) { super(userId, accountId, vmId, handlerName); - - this.nic = nic; + + this.nicId = nicId; } - - public Nic getNic() { - return nic; + + public Long getNicId() { + return nicId; } }