Merge branch '4.11'

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java

@@ -1349,30 +1349,29 @@ public class VolumeOrchestrator extends ManagerBase implements VolumeOrchestrati
         StoragePool pool;
         for (VolumeTask task : tasks) {
             if (task.type == VolumeTaskType.NOP) {
-                pool = (StoragePool)dataStoreMgr.getDataStore(task.pool.getId(), DataStoreRole.Primary);
-
-                if (task.pool != null && task.pool.isManaged()) {
-                    long hostId = vm.getVirtualMachine().getHostId();
-                    Host host = _hostDao.findById(hostId);
-
-                    volService.grantAccess(volFactory.getVolume(task.volume.getId()), host, (DataStore)pool);
-                }
-
                 vol = task.volume;
 
-                // For a zone-wide managed storage, it is possible that the VM can be started in another
+                pool = (StoragePool)dataStoreMgr.getDataStore(task.pool.getId(), DataStoreRole.Primary);
+
+                // For zone-wide managed storage, it is possible that the VM can be started in another
                 // cluster. In that case, make sure that the volume is in the right access group.
                 if (pool.isManaged()) {
-                    long oldHostId = vm.getVirtualMachine().getLastHostId();
+                    Host lastHost = _hostDao.findById(vm.getVirtualMachine().getLastHostId());
-                    long hostId = vm.getVirtualMachine().getHostId();
+                    Host host = _hostDao.findById(vm.getVirtualMachine().getHostId());
 
-                    if (oldHostId != hostId) {
+                    long lastClusterId = lastHost == null || lastHost.getClusterId() == null ? -1 : lastHost.getClusterId();
-                        Host oldHost = _hostDao.findById(oldHostId);
+                    long clusterId = host == null || host.getClusterId() == null ? -1 : host.getClusterId();
-                        DataStore storagePool = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary);
 
-                        storageMgr.removeStoragePoolFromCluster(oldHostId, vol.get_iScsiName(), pool);
+                    if (lastClusterId != clusterId) {
+                        if (lastHost != null) {
+                            storageMgr.removeStoragePoolFromCluster(lastHost.getId(), vol.get_iScsiName(), pool);
 
-                        volService.revokeAccess(volFactory.getVolume(vol.getId()), oldHost, storagePool);
+                            DataStore storagePool = dataStoreMgr.getDataStore(pool.getId(), DataStoreRole.Primary);
+
+                            volService.revokeAccess(volFactory.getVolume(vol.getId()), lastHost, storagePool);
+                        }
+
+                        volService.grantAccess(volFactory.getVolume(vol.getId()), host, (DataStore)pool);
                     }
                 }
             } else if (task.type == VolumeTaskType.MIGRATE) {

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -1169,6 +1169,8 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             }
         }
 
+        macAddress = validateOrReplaceMacAddress(macAddress, network.getId());
+
         if(_nicDao.findByNetworkIdAndMacAddress(networkId, macAddress) != null) {
             throw new CloudRuntimeException("A NIC with this MAC address exists for network: " + network.getUuid());
         }
@@ -1239,6 +1241,19 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
         return _vmDao.findById(vmInstance.getId());
     }
 
+    /**
+     * If the given MAC address is invalid it replaces the given MAC with the next available MAC address
+     */
+    protected String validateOrReplaceMacAddress(String macAddress, long networkId) {
+        if (!NetUtils.isValidMac(macAddress)) {
+            try {
+                macAddress = _networkModel.getNextAvailableMacAddressInNetwork(networkId);
+            } catch (InsufficientAddressCapacityException e) {
+                throw new CloudRuntimeException(String.format("A MAC address cannot be generated for this NIC in the network [id=%s] ", networkId));
+            }
+        }
+        return macAddress;
+    }
 
     private void saveExtraDhcpOptions(long nicId, Map<Integer, String> dhcpOptions) {
         List<NicExtraDhcpOptionVO> nicExtraDhcpOptionVOList = dhcpOptions

server/test/com/cloud/vm/UserVmManagerImplTest.java

@@ -22,6 +22,7 @@ import java.util.HashMap;
 import org.apache.cloudstack.api.BaseCmd.HTTPMethod;
 import org.apache.cloudstack.api.command.user.vm.UpdateVMCmd;
 import org.apache.cloudstack.context.CallContext;
+import org.junit.Assert;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -34,9 +35,11 @@ import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
 
+import com.cloud.exception.InsufficientAddressCapacityException;
 import com.cloud.exception.InsufficientCapacityException;
 import com.cloud.exception.InvalidParameterValueException;
 import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.network.NetworkModel;
 import com.cloud.storage.GuestOSVO;
 import com.cloud.storage.dao.GuestOSDao;
 import com.cloud.user.Account;
@@ -70,6 +73,9 @@ public class UserVmManagerImplTest {
     @Mock
     private UserVmVO userVmVoMock;
 
+    @Mock
+    private NetworkModel networkModel;
+
     private long vmId = 1l;
 
     @Before
@@ -221,4 +227,53 @@ public class UserVmManagerImplTest {
                 Mockito.anyString(), Mockito.anyBoolean(), Mockito.any(HTTPMethod.class), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyListOf(Long.class),
                 Mockito.anyMap());
     }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressValid() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(0, "01:23:45:67:89:ab", "01:23:45:67:89:ab");
+    }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressNull() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(1, null, "01:23:45:67:89:ab");
+    }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressBlank() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(1, " ", "01:23:45:67:89:ab");
+    }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressEmpty() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(1, "", "01:23:45:67:89:ab");
+    }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressNotValidOption1() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(1, "abcdef:gh:ij:kl", "01:23:45:67:89:ab");
+    }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressNotValidOption2() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(1, "01:23:45:67:89:", "01:23:45:67:89:ab");
+    }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressNotValidOption3() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(1, "01:23:45:67:89:az", "01:23:45:67:89:ab");
+    }
+
+    @Test
+    public void validateOrReplaceMacAddressTestMacAddressNotValidOption4() throws InsufficientAddressCapacityException {
+        configureValidateOrReplaceMacAddressTest(1, "@1:23:45:67:89:ab", "01:23:45:67:89:ab");
+    }
+
+    private void configureValidateOrReplaceMacAddressTest(int times, String macAddress, String expectedMacAddress) throws InsufficientAddressCapacityException {
+        Mockito.when(networkModel.getNextAvailableMacAddressInNetwork(Mockito.anyLong())).thenReturn(expectedMacAddress);
+
+        String returnedMacAddress = userVmManagerImpl.validateOrReplaceMacAddress(macAddress, 1l);
+
+        Mockito.verify(networkModel, Mockito.times(times)).getNextAvailableMacAddressInNetwork(Mockito.anyLong());
+        Assert.assertEquals(expectedMacAddress, returnedMacAddress);
+    }
 }