----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18568/#review35862 -----------------------------------------------------------
I looked into it, the FenceBuilder.fenceOff() method is called from only one place (HighAvailablityManagerImpl) and right under it checks if the return is null, the returned null is handled as if it was false. Therefore this change is safe and also changing the return value to boolean type would be safe as well. - Laszlo Hornyak On Feb. 27, 2014, 8:03 a.m., Wilder Rodrigues wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18568/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2014, 8:03 a.m.) > > > Review request for cloudstack, daan Hoogland and Hugo Trippaers. > > > Repository: cloudstack-git > > > Description > ------- > > Fix for Find Bugs findings on troubling issues: returning null when expected > is boolean; adding 6 unit tests and fix 1 in the KVMFencer; comparing objects > with == instead of equals() > > > Diffs > ----- > > plugins/hypervisors/xen/src/com/cloud/ha/XenServerFencer.java 28cba2b > > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java > 48ae3ea > plugins/hypervisors/xen/test/com/cloud/ha/XenServerFencerTest.java bd1d8f8 > server/src/com/cloud/ha/KVMFencer.java 516a579 > server/test/com/cloud/ha/KVMFencerTest.java cdd13b6 > > Diff: https://reviews.apache.org/r/18568/diff/ > > > Testing > ------- > > Added 6 unit tests for XenServerFencer (removed an useless one which was > testing get/set methods) > Build completed successfully > > Tested also on DevCloud + XenServer the following: > > Create Zone + Network + Pod + cluster + Pri/Sec Storage + Console Proxy and > System VMS > Create 1 instance (tiny Linux) + Guest Network + 1 SourceNAT + 1 extra pub ip > Set up firewall for port 22 + ICMP + port forwarding 22 ==> instance > SSH into the instance > > > Thanks, > > Wilder Rodrigues > >
