CLOUDSTACK-1638: Introduce NetworkMigrationResponder The location of the virtual machine is provided by DeployDestination, which will be passed in NetworkGuru#reserve and NetworkElement#prepare.
During the virtual machine migration, it actually changes DeployDestination and it looks like that it will tell that event to network components as it has NetworkManager#prepareNicForMigration. The problem is that althogh the interface has that method, NetworkManagerImpl does not tell the DeployDestination changes to network components. So IMHO, we need to add calls of NetworkGuru#reserve and NetworkElement#prepare in NetworkManagerImpl#prepareNicForMigration . And then, we also need to add calls NetworkGuru#release and NetworkElement#release after the migration, otherwise the network resources that plugin reserved will be kept even when the vm leaves off. (Sheng Yang: rebase code, add license header) Signed-off-by: Sheng Yang <sheng.y...@citrix.com> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/7260e8d8 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/7260e8d8 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/7260e8d8 Branch: refs/heads/ui-vpc-redesign Commit: 7260e8d83f07d90b48c34adaeb227de265019487 Parents: cf6045f Author: Hiroaki Kawai <ka...@stratosphere.co.jp> Authored: Fri May 3 10:43:20 2013 -0700 Committer: Sheng Yang <sheng.y...@citrix.com> Committed: Mon May 20 16:43:18 2013 -0700 ---------------------------------------------------------------------- .../cloud/network/NetworkMigrationResponder.java | 70 ++++++++++++++ server/src/com/cloud/network/NetworkManager.java | 29 ++++++- .../src/com/cloud/network/NetworkManagerImpl.java | 72 ++++++++++++++- .../com/cloud/vm/VirtualMachineManagerImpl.java | 11 ++ .../com/cloud/network/MockNetworkManagerImpl.java | 29 +++++- .../test/com/cloud/vpc/MockNetworkManagerImpl.java | 48 +++++++--- 6 files changed, 239 insertions(+), 20 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7260e8d8/api/src/com/cloud/network/NetworkMigrationResponder.java ---------------------------------------------------------------------- diff --git a/api/src/com/cloud/network/NetworkMigrationResponder.java b/api/src/com/cloud/network/NetworkMigrationResponder.java new file mode 100644 index 0000000..6283cc5 --- /dev/null +++ b/api/src/com/cloud/network/NetworkMigrationResponder.java @@ -0,0 +1,70 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package com.cloud.network; + +import com.cloud.deploy.DeployDestination; +import com.cloud.vm.NicProfile; +import com.cloud.vm.ReservationContext; +import com.cloud.vm.VirtualMachine; +import com.cloud.vm.VirtualMachineProfile; + +/** + * NetworkGuru and NetworkElements that implement this interface + * will be called during Virtual Machine migration. + */ +public interface NetworkMigrationResponder { + /** + * Prepare for migration. + * + * This method will be called per nic before the vm migration. + * @param nic + * @param network + * @param vm + * @param dest + * @param context + * @return true when operation was successful. + */ + public boolean prepareMigration(NicProfile nic, Network network, VirtualMachineProfile<? extends VirtualMachine> vm, DeployDestination dest, ReservationContext context); + + /** + * Cancel for migration preparation. + * + * This method will be called per nic when the entire vm migration + * process failed and need to release the resouces that was + * allocated at the migration preparation. + * @param nic destination nic + * @param network destination network + * @param vm destination vm profile + * @param src The context nic migrates from. + * @param dst The context nic migrates to. + */ + public void rollbackMigration(NicProfile nic, Network network, VirtualMachineProfile<? extends VirtualMachine> vm, ReservationContext src, ReservationContext dst); + + /** + * Commit the migration resource. + * + * This method will be called per nic when the entire vm migration + * process was successful. This is useful to release the resource of + * source deployment where vm has left. + * @param nic source nic + * @param network source network + * @param vm source vm profile + * @param src the context nic migrates from. + * @param dst the context nic migrates to. + */ + public void commitMigration(NicProfile nic, Network network, VirtualMachineProfile<? extends VirtualMachine> vm, ReservationContext src, ReservationContext dst); +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7260e8d8/server/src/com/cloud/network/NetworkManager.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/network/NetworkManager.java b/server/src/com/cloud/network/NetworkManager.java index 1e4fb84..05bc26e 100755 --- a/server/src/com/cloud/network/NetworkManager.java +++ b/server/src/com/cloud/network/NetworkManager.java @@ -123,7 +123,34 @@ public interface NetworkManager { Pair<NetworkGuru, NetworkVO> implementNetwork(long networkId, DeployDestination dest, ReservationContext context) throws ConcurrentOperationException, ResourceUnavailableException, InsufficientCapacityException; - <T extends VMInstanceVO> void prepareNicForMigration(VirtualMachineProfile<T> vm, DeployDestination dest); + /** + * prepares vm nic change for migration + * + * This method will be called in migration transaction before the vm migration. + * @param vm + * @param dest + */ + void prepareNicForMigration(VirtualMachineProfile<? extends VMInstanceVO> vm, DeployDestination dest); + + /** + * commit vm nic change for migration + * + * This method will be called in migration transaction after the successful + * vm migration. + * @param src + * @param dst + */ + void commitNicForMigration(VirtualMachineProfile<? extends VMInstanceVO> src, VirtualMachineProfile<? extends VMInstanceVO> dst); + + /** + * rollback vm nic change for migration + * + * This method will be called in migaration transaction after vm migration + * failure. + * @param src + * @param dst + */ + void rollbackNicForMigration(VirtualMachineProfile<? extends VMInstanceVO> src, VirtualMachineProfile<? extends VMInstanceVO> dst); boolean shutdownNetwork(long networkId, ReservationContext context, boolean cleanupElements); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7260e8d8/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 b3df0da..0f43b87 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -2000,8 +2000,9 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L } @Override - public <T extends VMInstanceVO> void prepareNicForMigration(VirtualMachineProfile<T> vm, DeployDestination dest) { + public void prepareNicForMigration(VirtualMachineProfile<? extends VMInstanceVO> vm, DeployDestination dest) { List<NicVO> nics = _nicDao.listByVmId(vm.getId()); + ReservationContext context = new ReservationContextImpl(UUID.randomUUID().toString(), null, null); for (NicVO nic : nics) { NetworkVO network = _networksDao.findById(nic.getNetworkId()); Integer networkRate = _networkModel.getNetworkRate(network.getId(), vm.getId()); @@ -2009,11 +2010,80 @@ public class NetworkManagerImpl extends ManagerBase implements NetworkManager, L NetworkGuru guru = AdapterBase.getAdapterByName(_networkGurus, network.getGuruName()); NicProfile profile = new NicProfile(nic, network, nic.getBroadcastUri(), nic.getIsolationUri(), networkRate, _networkModel.isSecurityGroupSupportedInNetwork(network), _networkModel.getNetworkTag(vm.getHypervisorType(), network)); + if(guru instanceof NetworkMigrationResponder){ + if(!((NetworkMigrationResponder) guru).prepareMigration(profile, network, vm, dest, context)){ + s_logger.error("NetworkGuru "+guru+" prepareForMigration failed."); // XXX: Transaction error + } + } + for (NetworkElement element : _networkElements) { + if(element instanceof NetworkMigrationResponder){ + if(!((NetworkMigrationResponder) element).prepareMigration(profile, network, vm, dest, context)){ + s_logger.error("NetworkElement "+element+" prepareForMigration failed."); // XXX: Transaction error + } + } + } guru.updateNicProfile(profile, network); vm.addNic(profile); } } + private NicProfile findNicProfileById(VirtualMachineProfile<? extends VMInstanceVO> vm, long id){ + for(NicProfile nic: vm.getNics()){ + if(nic.getId() == id){ + return nic; + } + } + return null; + } + + @Override + public void commitNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> src, + VirtualMachineProfile<? extends VMInstanceVO> dst) { + for(NicProfile nicSrc: src.getNics()){ + NetworkVO network = _networksDao.findById(nicSrc.getNetworkId()); + NetworkGuru guru = AdapterBase.getAdapterByName(_networkGurus, network.getGuruName()); + NicProfile nicDst = findNicProfileById(dst, nicSrc.getId()); + ReservationContext src_context = new ReservationContextImpl(nicSrc.getReservationId(), null, null); + ReservationContext dst_context = new ReservationContextImpl(nicDst.getReservationId(), null, null); + + if(guru instanceof NetworkMigrationResponder){ + ((NetworkMigrationResponder) guru).commitMigration(nicSrc, network, src, src_context, dst_context); + } + for (NetworkElement element : _networkElements) { + if(element instanceof NetworkMigrationResponder){ + ((NetworkMigrationResponder) element).commitMigration(nicSrc, network, src, src_context, dst_context); + } + } + // update the reservation id + NicVO nicVo = _nicDao.findById(nicDst.getId()); + nicVo.setReservationId(nicDst.getReservationId()); + _nicDao.persist(nicVo); + } + } + + @Override + public void rollbackNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> src, + VirtualMachineProfile<? extends VMInstanceVO> dst) { + for(NicProfile nicDst: dst.getNics()){ + NetworkVO network = _networksDao.findById(nicDst.getNetworkId()); + NetworkGuru guru = AdapterBase.getAdapterByName(_networkGurus, network.getGuruName()); + NicProfile nicSrc = findNicProfileById(src, nicDst.getId()); + ReservationContext src_context = new ReservationContextImpl(nicSrc.getReservationId(), null, null); + ReservationContext dst_context = new ReservationContextImpl(nicDst.getReservationId(), null, null); + + if(guru instanceof NetworkMigrationResponder){ + ((NetworkMigrationResponder) guru).rollbackMigration(nicDst, network, dst, src_context, dst_context); + } + for (NetworkElement element : _networkElements) { + if(element instanceof NetworkMigrationResponder){ + ((NetworkMigrationResponder) element).rollbackMigration(nicDst, network, dst, src_context, dst_context); + } + } + } + } + @Override @DB public void release(VirtualMachineProfile<? extends VMInstanceVO> vmProfile, boolean forced) throws http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7260e8d8/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 58d95ab..f487537 100755 --- a/server/src/com/cloud/vm/VirtualMachineManagerImpl.java +++ b/server/src/com/cloud/vm/VirtualMachineManagerImpl.java @@ -1414,6 +1414,11 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac alertType = AlertManager.ALERT_TYPE_CONSOLE_PROXY_MIGRATE; } + VirtualMachineProfile<VMInstanceVO> vmSrc = new VirtualMachineProfileImpl<VMInstanceVO>(vm); + for(NicProfile nic: _networkMgr.getNicProfiles(vm)){ + vmSrc.addNic(nic); + } + VirtualMachineProfile<VMInstanceVO> profile = new VirtualMachineProfileImpl<VMInstanceVO>(vm); _networkMgr.prepareNicForMigration(profile, dest); this.volumeMgr.prepareForMigration(profile, dest); @@ -1439,6 +1444,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac throw new AgentUnavailableException("Operation timed out", dstHostId); } finally { if (pfma == null) { + _networkMgr.rollbackNicForMigration(vmSrc, profile); work.setStep(Step.Done); _workDao.update(work.getId(), work); } @@ -1447,10 +1453,12 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac vm.setLastHostId(srcHostId); try { if (vm == null || vm.getHostId() == null || vm.getHostId() != srcHostId || !changeState(vm, Event.MigrationRequested, dstHostId, work, Step.Migrating)) { + _networkMgr.rollbackNicForMigration(vmSrc, profile); s_logger.info("Migration cancelled because state has changed: " + vm); throw new ConcurrentOperationException("Migration cancelled because state has changed: " + vm); } } catch (NoTransitionException e1) { + _networkMgr.rollbackNicForMigration(vmSrc, profile); s_logger.info("Migration cancelled because " + e1.getMessage()); throw new ConcurrentOperationException("Migration cancelled because " + e1.getMessage()); } @@ -1502,6 +1510,7 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } finally { if (!migrated) { s_logger.info("Migration was unsuccessful. Cleaning up: " + vm); + _networkMgr.rollbackNicForMigration(vmSrc, profile); _alertMgr.sendAlert(alertType, fromHost.getDataCenterId(), fromHost.getPodId(), "Unable to migrate vm " + vm.getInstanceName() + " from host " + fromHost.getName() + " in zone " + dest.getDataCenter().getName() + " and pod " + dest.getPod().getName(), "Migrate Command failed. Please check logs."); @@ -1516,6 +1525,8 @@ public class VirtualMachineManagerImpl extends ManagerBase implements VirtualMac } catch (NoTransitionException e) { s_logger.warn(e.getMessage()); } + }else{ + _networkMgr.commitNicForMigration(vmSrc, profile); } work.setStep(Step.Done); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7260e8d8/server/test/com/cloud/network/MockNetworkManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/network/MockNetworkManagerImpl.java b/server/test/com/cloud/network/MockNetworkManagerImpl.java index 1ac014d..e5d34fb 100755 --- a/server/test/com/cloud/network/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/network/MockNetworkManagerImpl.java @@ -278,12 +278,6 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage @Override - public <T extends VMInstanceVO> void prepareNicForMigration(VirtualMachineProfile<T> vm, DeployDestination dest) { - // TODO Auto-generated method stub - - } - - @Override public boolean destroyNetwork(long networkId, ReservationContext context) { // TODO Auto-generated method stub return false; @@ -966,4 +960,27 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage public PublicIp assignPublicIpAddressFromVlans(long dcId, Long podId, Account owner, VlanType type, List<Long> vlanDbIds, Long networkId, String requestedIp, boolean isSystem) throws InsufficientAddressCapacityException { return null; //To change body of implemented methods use File | Settings | File Templates. } + + @Override + public void prepareNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> vm, + DeployDestination dest) { + // TODO Auto-generated method stub + + } + + @Override + public void commitNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> src, + VirtualMachineProfile<? extends VMInstanceVO> dst) { + // TODO Auto-generated method stub + + } + + @Override + public void rollbackNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> src, + VirtualMachineProfile<? extends VMInstanceVO> dst) { + // TODO Auto-generated method stub + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/7260e8d8/server/test/com/cloud/vpc/MockNetworkManagerImpl.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java index 62599b8..7129273 100644 --- a/server/test/com/cloud/vpc/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/vpc/MockNetworkManagerImpl.java @@ -845,18 +845,6 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage - /* (non-Javadoc) - * @see com.cloud.network.NetworkManager#prepareNicForMigration(com.cloud.vm.VirtualMachineProfile, com.cloud.deploy.DeployDestination) - */ - @Override - public <T extends VMInstanceVO> void prepareNicForMigration(VirtualMachineProfile<T> vm, DeployDestination dest) { - // TODO Auto-generated method stub - - } - - - - /* (non-Javadoc) * @see com.cloud.network.NetworkManager#shutdownNetwork(long, com.cloud.vm.ReservationContext, boolean) @@ -1471,4 +1459,40 @@ public class MockNetworkManagerImpl extends ManagerBase implements NetworkManage } + + + + @Override + public void prepareNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> vm, + DeployDestination dest) { + // TODO Auto-generated method stub + + } + + + + + + @Override + public void commitNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> src, + VirtualMachineProfile<? extends VMInstanceVO> dst) { + // TODO Auto-generated method stub + + } + + + + + + @Override + public void rollbackNicForMigration( + VirtualMachineProfile<? extends VMInstanceVO> src, + VirtualMachineProfile<? extends VMInstanceVO> dst) { + // TODO Auto-generated method stub + + } + + }