You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Santhosh Edukulla <sa...@citrix.com> on 2014/07/02 10:42:17 UTC
Review Request 23226: Fixed resource leaks, null dererence issues,
performance and bugs reported by coverity
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/
-----------------------------------------------------------
Review request for cloudstack and daan Hoogland.
Bugs: coverity
https://issues.apache.org/jira/browse/coverity
Repository: cloudstack-git
Description
-------
Fixed resource leaks, null dererence issues, performance and bugs reported by coverity
Diffs
-----
engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
Diff: https://reviews.apache.org/r/23226/diff/
Testing
-------
Built the management server, ran the simulator and deployed dc.
Thanks,
Santhosh Edukulla
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Hugo Trippaers <ht...@schubergphilis.com>.
> On July 2, 2014, 12:50 p.m., Hugo Trippaers wrote:
> > engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java, line 310
> > <https://reviews.apache.org/r/23226/diff/1/?file=622487#file622487line310>
> >
> > Shouldn't this be inside the try-with-resources bit of the try keyword?
>
> Santhosh Edukulla wrote:
> Here we are taking care of closing the resource with "prepareAutoCloseStatement", compared to normal call of "prepareStatement" i believe. We used try with resource where prepare through auto close was not followed. Going ahead, we can change it to try with resource, to make it uniform, but here other leak issue is fixed i believe.
I think we should avoid using the prepareAutoCloseStatement in favor of the try-with-resources. The prepareAutoCloseStatement doesn't really close the statement unless it is called a second time if i understand the code correctly.
> On July 2, 2014, 12:50 p.m., Hugo Trippaers wrote:
> > engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java, line 71
> > <https://reviews.apache.org/r/23226/diff/1/?file=622500#file622500line71>
> >
> > To make it more clear for the readers of the code why not write 'if (params == null) { throw blabla }'
> >
> >
>
> Santhosh Edukulla wrote:
> We can, As such possibility of null referencing is fixed, going ahead, i can add a log and throw accordingly, but on second note, i assume that the calling function based upon the callable output can log and throw accordingly there, rather we take care in the called function. Let me know your thoughts.
This function is apparently called by plugins directly, so we should be prepared to handle params being null in this function. So your solution is correct, it's just coding style.. Instead of if (x != null) { do_lots_of_stuff(); } throw exception, it's better to write if (x==null) { throw exception} do_lots_of_stuff();
- Hugo
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/#review47181
-----------------------------------------------------------
On July 2, 2014, 8:44 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23226/
> -----------------------------------------------------------
>
> (Updated July 2, 2014, 8:44 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>
>
> Bugs: coverity
> https://issues.apache.org/jira/browse/coverity
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
> engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
> engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
> engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
> engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
> engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
> engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
> engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
> engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
> engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
> engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
> engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
> engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
> framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
> framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
> plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
> server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
> server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
> server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
>
> Diff: https://reviews.apache.org/r/23226/diff/
>
>
> Testing
> -------
>
> Built the management server, ran the simulator and deployed dc.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On July 2, 2014, 12:50 p.m., Hugo Trippaers wrote:
> > Good stuff, but there seem to be a number of places where the PreparedStatement should be inside the try clause to actually guarantee closing the resource i think. Can you have a look?
Hugo,
Please let me know if the comments are ok, if yes, i will push the changes.
Thanks again!
Santhosh
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/#review47181
-----------------------------------------------------------
On July 2, 2014, 8:44 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23226/
> -----------------------------------------------------------
>
> (Updated July 2, 2014, 8:44 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>
>
> Bugs: coverity
> https://issues.apache.org/jira/browse/coverity
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
> engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
> engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
> engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
> engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
> engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
> engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
> engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
> engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
> engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
> engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
> engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
> engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
> framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
> framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
> plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
> server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
> server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
> server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
>
> Diff: https://reviews.apache.org/r/23226/diff/
>
>
> Testing
> -------
>
> Built the management server, ran the simulator and deployed dc.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On July 2, 2014, 12:50 p.m., Hugo Trippaers wrote:
> > engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java, line 310
> > <https://reviews.apache.org/r/23226/diff/1/?file=622487#file622487line310>
> >
> > Shouldn't this be inside the try-with-resources bit of the try keyword?
Here we are taking care of closing the resource with "prepareAutoCloseStatement", compared to normal call of "prepareStatement" i believe. We used try with resource where prepare through auto close was not followed. Going ahead, we can change it to try with resource, to make it uniform, but here other leak issue is fixed i believe.
> On July 2, 2014, 12:50 p.m., Hugo Trippaers wrote:
> > engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java, line 71
> > <https://reviews.apache.org/r/23226/diff/1/?file=622500#file622500line71>
> >
> > To make it more clear for the readers of the code why not write 'if (params == null) { throw blabla }'
> >
> >
We can, As such possibility of null referencing is fixed, going ahead, i can add a log and throw accordingly, but on second note, i assume that the calling function based upon the callable output can log and throw accordingly there, rather we take care in the called function. Let me know your thoughts.
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/#review47181
-----------------------------------------------------------
On July 2, 2014, 8:44 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23226/
> -----------------------------------------------------------
>
> (Updated July 2, 2014, 8:44 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>
>
> Bugs: coverity
> https://issues.apache.org/jira/browse/coverity
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
> engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
> engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
> engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
> engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
> engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
> engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
> engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
> engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
> engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
> engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
> engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
> engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
> framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
> framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
> plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
> server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
> server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
> server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
>
> Diff: https://reviews.apache.org/r/23226/diff/
>
>
> Testing
> -------
>
> Built the management server, ran the simulator and deployed dc.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/#review47181
-----------------------------------------------------------
Good stuff, but there seem to be a number of places where the PreparedStatement should be inside the try clause to actually guarantee closing the resource i think. Can you have a look?
engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java
<https://reviews.apache.org/r/23226/#comment82802>
Shouldn't this be inside the try-with-resources bit of the try keyword?
engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java
<https://reviews.apache.org/r/23226/#comment82803>
same here?
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java
<https://reviews.apache.org/r/23226/#comment82804>
To make it more clear for the readers of the code why not write 'if (params == null) { throw blabla }'
- Hugo Trippaers
On July 2, 2014, 8:44 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23226/
> -----------------------------------------------------------
>
> (Updated July 2, 2014, 8:44 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>
>
> Bugs: coverity
> https://issues.apache.org/jira/browse/coverity
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
> engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
> engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
> engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
> engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
> engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
> engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
> engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
> engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
> engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
> engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
> engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
> engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
> framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
> framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
> plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
> server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
> server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
> server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
>
> Diff: https://reviews.apache.org/r/23226/diff/
>
>
> Testing
> -------
>
> Built the management server, ran the simulator and deployed dc.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Santhosh Edukulla <sa...@citrix.com>.
> On July 4, 2014, 11:17 a.m., Santhosh Edukulla wrote:
> > Ship It!
a600d8408ea86782318139c17cf346c8497943d0. Pushed to Master
- Santhosh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/#review47340
-----------------------------------------------------------
On July 4, 2014, 9:07 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23226/
> -----------------------------------------------------------
>
> (Updated July 4, 2014, 9:07 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>
>
> Bugs: coverity
> https://issues.apache.org/jira/browse/coverity
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
> engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
> engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
> engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
> engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
> engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
> engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
> engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
> engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
> engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
> engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
> engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
> engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
> framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
> framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
> plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
> server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
> server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
> server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
>
> Diff: https://reviews.apache.org/r/23226/diff/
>
>
> Testing
> -------
>
> Built the management server, ran the simulator and deployed dc.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/#review47340
-----------------------------------------------------------
Ship it!
Ship It!
- Santhosh Edukulla
On July 4, 2014, 9:07 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23226/
> -----------------------------------------------------------
>
> (Updated July 4, 2014, 9:07 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>
>
> Bugs: coverity
> https://issues.apache.org/jira/browse/coverity
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
> engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
> engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
> engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
> engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
> engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
> engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
> engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
> engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
> engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
> engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
> engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
> engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
> framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
> framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
> plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
> server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
> server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
> server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
>
> Diff: https://reviews.apache.org/r/23226/diff/
>
>
> Testing
> -------
>
> Built the management server, ran the simulator and deployed dc.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Hugo Trippaers <ht...@schubergphilis.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/#review47337
-----------------------------------------------------------
Ship it!
LGTM Thanks for making the effort to incorporate the feedback.
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java
<https://reviews.apache.org/r/23226/#comment83090>
Just a style thing, but i would remove the else. It is already implicit as the function would not continue after the throw in the if.
- Hugo Trippaers
On July 4, 2014, 9:07 a.m., Santhosh Edukulla wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23226/
> -----------------------------------------------------------
>
> (Updated July 4, 2014, 9:07 a.m.)
>
>
> Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
>
>
> Bugs: coverity
> https://issues.apache.org/jira/browse/coverity
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
>
>
> Diffs
> -----
>
> engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
> engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
> engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
> engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
> engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
> engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
> engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
> engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
> engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
> engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
> engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
> engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
> engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
> engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
> framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
> framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
> framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
> plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
> server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
> server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
> server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
>
> Diff: https://reviews.apache.org/r/23226/diff/
>
>
> Testing
> -------
>
> Built the management server, ran the simulator and deployed dc.
>
>
> Thanks,
>
> Santhosh Edukulla
>
>
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/
-----------------------------------------------------------
(Updated July 4, 2014, 9:07 a.m.)
Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
Changes
-------
Added try-with-resource and other misc fixes for coverity.
Bugs: coverity
https://issues.apache.org/jira/browse/coverity
Repository: cloudstack-git
Description
-------
Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
Diffs (updated)
-----
engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
Diff: https://reviews.apache.org/r/23226/diff/
Testing
-------
Built the management server, ran the simulator and deployed dc.
Thanks,
Santhosh Edukulla
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance issues and bugs reported by coverity
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/
-----------------------------------------------------------
(Updated July 2, 2014, 8:44 a.m.)
Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
Summary (updated)
-----------------
Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity
Bugs: coverity
https://issues.apache.org/jira/browse/coverity
Repository: cloudstack-git
Description (updated)
-------
Fixed resource leaks, null dererence issues, performance issues and bugs reported by coverity. Simple bug in generic dao was found during these changes, fixed that as well.
Diffs
-----
engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
Diff: https://reviews.apache.org/r/23226/diff/
Testing
-------
Built the management server, ran the simulator and deployed dc.
Thanks,
Santhosh Edukulla
Re: Review Request 23226: Fixed resource leaks, null dererence issues,
performance and bugs reported by coverity
Posted by Santhosh Edukulla <sa...@citrix.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23226/
-----------------------------------------------------------
(Updated July 2, 2014, 8:42 a.m.)
Review request for cloudstack, Abhinandan Prateek and daan Hoogland.
Bugs: coverity
https://issues.apache.org/jira/browse/coverity
Repository: cloudstack-git
Description
-------
Fixed resource leaks, null dererence issues, performance and bugs reported by coverity
Diffs
-----
engine/schema/src/com/cloud/dc/dao/VlanDaoImpl.java a38a96e
engine/schema/src/com/cloud/storage/dao/StoragePoolWorkDaoImpl.java c6e1b74
engine/schema/src/com/cloud/upgrade/DatabaseCreator.java b04607d
engine/schema/src/com/cloud/usage/dao/UsageIPAddressDaoImpl.java 17d5dc8
engine/schema/src/com/cloud/usage/dao/UsageLoadBalancerPolicyDaoImpl.java c868ac0
engine/schema/src/com/cloud/usage/dao/UsageNetworkOfferingDaoImpl.java f27fd60
engine/schema/src/com/cloud/usage/dao/UsagePortForwardingRuleDaoImpl.java 803288a
engine/schema/src/com/cloud/usage/dao/UsageSecurityGroupDaoImpl.java 0fc2ce0
engine/schema/src/com/cloud/usage/dao/UsageStorageDaoImpl.java 77fc56f
engine/schema/src/com/cloud/usage/dao/UsageVPNUserDaoImpl.java 64d3ecd
engine/schema/src/com/cloud/usage/dao/UsageVolumeDaoImpl.java 7bf058c
engine/schema/src/org/apache/cloudstack/storage/datastore/db/PrimaryDataStoreDaoImpl.java 92793f1
engine/storage/cache/src/org/apache/cloudstack/storage/cache/allocator/StorageCacheRandomAllocator.java b4ef595
engine/storage/src/org/apache/cloudstack/storage/volume/datastore/PrimaryDataStoreHelper.java 6b12975
framework/cluster/src/com/cloud/cluster/dao/ManagementServerHostDaoImpl.java 3d0c3f5
framework/db/src/com/cloud/utils/db/DbUtil.java 792573c
framework/db/src/com/cloud/utils/db/GenericDaoBase.java 2052aad
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java e684b8d
plugins/storage/volume/sample/src/org/apache/cloudstack/storage/datastore/lifecycle/SamplePrimaryDataStoreLifeCycleImpl.java 579cc24
server/src/com/cloud/storage/VolumeApiServiceImpl.java a557c0e
server/src/com/cloud/tags/TaggedResourceManagerImpl.java b77c55e
server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java e085b8d
Diff: https://reviews.apache.org/r/23226/diff/
Testing
-------
Built the management server, ran the simulator and deployed dc.
Thanks,
Santhosh Edukulla