Fixing some of the scary bugs possible null pointer: created testSetNicDevIdIfCorrectVifIsNotNull
Signed-off-by: Hugo Trippaers <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/16aa73c2 Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/16aa73c2 Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/16aa73c2 Branch: refs/heads/master Commit: 16aa73c2c88965e3e88aa94f6d1cf1d1496f7060 Parents: e171cb1 Author: Sander Botman <[email protected]> Authored: Tue Feb 11 17:45:00 2014 +0100 Committer: Hugo Trippaers <[email protected]> Committed: Fri Feb 14 18:37:44 2014 +0100 ---------------------------------------------------------------------- .../xen/src/com/cloud/ha/XenServerFencer.java | 1 - .../xen/resource/CitrixResourceBase.java | 23 ++++++++---- .../test/com/cloud/ha/XenServerFencerTest.java | 39 ++++++++++++++++++++ .../xen/resource/CitrixResourceBaseTest.java | 20 +++++++++- 4 files changed, 73 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/16aa73c2/plugins/hypervisors/xen/src/com/cloud/ha/XenServerFencer.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/xen/src/com/cloud/ha/XenServerFencer.java b/plugins/hypervisors/xen/src/com/cloud/ha/XenServerFencer.java index 5604db8..28cba2b 100755 --- a/plugins/hypervisors/xen/src/com/cloud/ha/XenServerFencer.java +++ b/plugins/hypervisors/xen/src/com/cloud/ha/XenServerFencer.java @@ -43,7 +43,6 @@ import com.cloud.vm.VirtualMachine; @Local(value = FenceBuilder.class) public class XenServerFencer extends AdapterBase implements FenceBuilder { private static final Logger s_logger = Logger.getLogger(XenServerFencer.class); - String _name; @Inject HostDao _hostDao; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/16aa73c2/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java index cf5c6d6..788e70d 100644 --- a/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java @@ -7297,14 +7297,7 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe VM router = getVM(conn, routerName); VIF correctVif = getVifByMac(conn, router, ip.getVifMacAddress()); - if (correctVif == null) { - if (ip.isAdd()) { - throw new InternalErrorException("Failed to find DomR VIF to associate IP with."); - } else { - s_logger.debug("VIF to deassociate IP with does not exist, return success"); - } - } - ip.setNicDevId(Integer.valueOf(correctVif.getDevice(conn))); + setNicDevIdIfCorrectVifIsNotNull(conn, ip, correctVif); } } catch (Exception e) { s_logger.error("Ip Assoc failure on applying one ip due to exception: ", e); @@ -7314,6 +7307,20 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe return new ExecutionResult(true, null); } + protected void setNicDevIdIfCorrectVifIsNotNull(Connection conn, + IpAddressTO ip, VIF correctVif) throws InternalErrorException, + BadServerResponse, XenAPIException, XmlRpcException { + if (correctVif == null) { + if (ip.isAdd()) { + throw new InternalErrorException("Failed to find DomR VIF to associate IP with."); + } else { + s_logger.debug("VIF to deassociate IP with does not exist, return success"); + } + } else { + ip.setNicDevId(Integer.valueOf(correctVif.getDevice(conn))); + } + } + protected ExecutionResult prepareNetworkElementCommand(SetSourceNatCommand cmd) { Connection conn = getConnection(); String routerName = cmd.getAccessDetail(NetworkElementCommand.ROUTER_NAME); http://git-wip-us.apache.org/repos/asf/cloudstack/blob/16aa73c2/plugins/hypervisors/xen/test/com/cloud/ha/XenServerFencerTest.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/xen/test/com/cloud/ha/XenServerFencerTest.java b/plugins/hypervisors/xen/test/com/cloud/ha/XenServerFencerTest.java new file mode 100644 index 0000000..bd1d8f8 --- /dev/null +++ b/plugins/hypervisors/xen/test/com/cloud/ha/XenServerFencerTest.java @@ -0,0 +1,39 @@ +// 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.ha; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + + +public class XenServerFencerTest { + + @Test + public void testSetAndGetName() throws Exception { + XenServerFencer xenServerFencer = new XenServerFencer(); + String name = "name"; + + xenServerFencer.setName(name); + String actual = xenServerFencer.getName(); + + assertEquals(name, actual); + + } + +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/16aa73c2/plugins/hypervisors/xen/test/com/cloud/hypervisor/xen/resource/CitrixResourceBaseTest.java ---------------------------------------------------------------------- diff --git a/plugins/hypervisors/xen/test/com/cloud/hypervisor/xen/resource/CitrixResourceBaseTest.java b/plugins/hypervisors/xen/test/com/cloud/hypervisor/xen/resource/CitrixResourceBaseTest.java index c56cc6d..b54a21c 100644 --- a/plugins/hypervisors/xen/test/com/cloud/hypervisor/xen/resource/CitrixResourceBaseTest.java +++ b/plugins/hypervisors/xen/test/com/cloud/hypervisor/xen/resource/CitrixResourceBaseTest.java @@ -19,12 +19,14 @@ package com.cloud.hypervisor.xen.resource; +import static org.junit.Assert.fail; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static org.mockito.Matchers.any; import java.util.HashMap; import java.util.Iterator; @@ -41,11 +43,12 @@ import org.mockito.Spy; import com.xensource.xenapi.Connection; import com.xensource.xenapi.Host; import com.xensource.xenapi.Types; +import com.xensource.xenapi.VIF; import com.xensource.xenapi.VM; import com.xensource.xenapi.XenAPIObject; - import com.cloud.agent.api.ScaleVmAnswer; import com.cloud.agent.api.ScaleVmCommand; +import com.cloud.agent.api.to.IpAddressTO; import com.cloud.agent.api.to.VirtualMachineTO; import com.cloud.hypervisor.xen.resource.CitrixResourceBase.XsHost; @@ -171,4 +174,19 @@ public class CitrixResourceBaseTest { verify(_resource, times(1)).callHostPlugin(conn, "vmops", "add_to_VCPUs_params_live", "key", "weight", "value", "253", "vmname", "i-2-3-VM"); verify(_resource, times(1)).callHostPlugin(conn, "vmops", "add_to_VCPUs_params_live", "key", "cap", "value", "99", "vmname", "i-2-3-VM"); } + + + @Test + public void testSetNicDevIdIfCorrectVifIsNotNull() throws Exception { + IpAddressTO ip = mock (IpAddressTO.class); + when (ip.isAdd()).thenReturn(false); + VIF correctVif = null; + try { + _resource.setNicDevIdIfCorrectVifIsNotNull(conn, ip, correctVif); + } + catch (NullPointerException e) + { + fail("this test is meant to show that null pointer is not thrown"); + } + } } \ No newline at end of file
