You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by daan Hoogland <da...@gmail.com> on 2013/08/23 10:48:45 UTC
Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/
-----------------------------------------------------------
Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, and Sheng Yang.
Bugs: CLOUDSTACK-4346
Repository: cloudstack-git
Description
-------
After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
Diffs
-----
plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
server/src/com/cloud/api/ApiResponseHelper.java c771431
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 57dc0b3
server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
utils/src/com/cloud/utils/net/NetUtils.java 05b485b
Diff: https://reviews.apache.org/r/13771/diff/
Testing
-------
tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
Thanks,
daan Hoogland
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/#review25816
-----------------------------------------------------------
Commit d9c06e538be96c347acc9055c267efda0b7bb559 in branch refs/heads/master from Daan Hoogland
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=d9c06e5 ]
CLOUDSTACK-4346 replace URI getHost() and create(String) calls with BroadcastDomainType methods
- ASF Subversion and Git Services
On Aug. 31, 2013, 7:57 p.m., daan Hoogland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> -----------------------------------------------------------
>
> (Updated Aug. 31, 2013, 7:57 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, Wido den Hollander, and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-4346
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>
>
> Diffs
> -----
>
> plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
> plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
> plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
> plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
> plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
> plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
> plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
> plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
> server/src/com/cloud/api/ApiResponseHelper.java c771431
> server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
> server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
> server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
> server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
> server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
> server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
> server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
> server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
> server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
> utils/src/com/cloud/utils/net/NetUtils.java 05b485b
>
> Diff: https://reviews.apache.org/r/13771/diff/
>
>
> Testing
> -------
>
> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>
>
> Thanks,
>
> daan Hoogland
>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/#review26002
-----------------------------------------------------------
Commit 25c8cee01a450ee924fe108cafe54b046485ab2b in branch refs/heads/master from Daan Hoogland
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=25c8cee ]
CLOUDSTACK-4346 uses of parseInt and parseLong secured
- ASF Subversion and Git Services
On Aug. 31, 2013, 7:57 p.m., daan Hoogland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> -----------------------------------------------------------
>
> (Updated Aug. 31, 2013, 7:57 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, Wido den Hollander, and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-4346
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>
>
> Diffs
> -----
>
> plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
> plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
> plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
> plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
> plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
> plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
> plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
> plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
> server/src/com/cloud/api/ApiResponseHelper.java c771431
> server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
> server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
> server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
> server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
> server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
> server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
> server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
> server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
> server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
> utils/src/com/cloud/utils/net/NetUtils.java 05b485b
>
> Diff: https://reviews.apache.org/r/13771/diff/
>
>
> Testing
> -------
>
> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>
>
> Thanks,
>
> daan Hoogland
>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Daan Hoogland <da...@gmail.com>.
Wido, I just added you for your kvm interesse. Do you mind having a look
before I commit this?
regards,
Daan
On Sat, Aug 31, 2013 at 9:57 PM, daan Hoogland <da...@gmail.com>wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo
> Trippaers, Wido den Hollander, and Sheng Yang.
> By daan Hoogland.
>
> *Updated Aug. 31, 2013, 7:57 p.m.*
> Changes
>
> adding wido for his interest in kvm
>
> *Bugs: * CLOUDSTACK-4346
> *Repository: * cloudstack-git
> Description
>
> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>
> Testing
>
> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>
> Diffs
>
> - plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java
> (07ee12d)
> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
> (195cf40)
> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
> (a156ae6)
> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
> (7038d7e)
> - plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java
> (59ba001)
> - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
> (5ab2216)
> - plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
> (ecdec1e)
> - plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java
> (54623e9)
> - plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java
> (3ae6a08)
> - plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java
> (1733712)
> - plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java
> (3d3d797)
> - plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java
> (c7d0884)
> - plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java
> (ff238ed)
> - plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java
> (36a807f)
> - server/src/com/cloud/api/ApiResponseHelper.java (c771431)
> - server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java
> (e91dcfa)
> - server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java
> (a934024)
> - server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java
> (c14d5c7)
> - server/src/com/cloud/network/NetworkManagerImpl.java (00103e3)
> - server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java
> (5b87d54)
> - server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java
> (00598dd)
> - server/src/com/cloud/network/guru/GuestNetworkGuru.java (b0da42f)
> - server/src/com/cloud/network/guru/PrivateNetworkGuru.java (6521cf4)
> - server/src/com/cloud/network/guru/PublicNetworkGuru.java (d109468)
> - server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
> (ee0d058)
> - utils/src/com/cloud/utils/net/NetUtils.java (05b485b)
>
> View Diff <https://reviews.apache.org/r/13771/diff/>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/
-----------------------------------------------------------
(Updated Aug. 31, 2013, 7:57 p.m.)
Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, Wido den Hollander, and Sheng Yang.
Changes
-------
adding wido for his interest in kvm
Bugs: CLOUDSTACK-4346
Repository: cloudstack-git
Description
-------
After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
Diffs
-----
plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
server/src/com/cloud/api/ApiResponseHelper.java c771431
server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
utils/src/com/cloud/utils/net/NetUtils.java 05b485b
Diff: https://reviews.apache.org/r/13771/diff/
Testing
-------
tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
Thanks,
daan Hoogland
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Dave Cahill <dc...@midokura.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/#review25724
-----------------------------------------------------------
Ship it!
LGTM.
- Dave Cahill
On Aug. 28, 2013, 2:37 p.m., daan Hoogland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2013, 2:37 p.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-4346
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>
>
> Diffs
> -----
>
> plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
> plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
> plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
> plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
> plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
> plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
> plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
> plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
> server/src/com/cloud/api/ApiResponseHelper.java c771431
> server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
> server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
> server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
> server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
> server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
> server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
> server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
> server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
> server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
> utils/src/com/cloud/utils/net/NetUtils.java 05b485b
>
> Diff: https://reviews.apache.org/r/13771/diff/
>
>
> Testing
> -------
>
> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>
>
> Thanks,
>
> daan Hoogland
>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/
-----------------------------------------------------------
(Updated Aug. 28, 2013, 2:37 p.m.)
Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, and Sheng Yang.
Changes
-------
Dave's comments addressed
Bugs: CLOUDSTACK-4346
Repository: cloudstack-git
Description
-------
After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
Diffs (updated)
-----
plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
server/src/com/cloud/api/ApiResponseHelper.java c771431
server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
utils/src/com/cloud/utils/net/NetUtils.java 05b485b
Diff: https://reviews.apache.org/r/13771/diff/
Testing
-------
tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
Thanks,
daan Hoogland
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by daan Hoogland <da...@gmail.com>.
> On Aug. 28, 2013, 2:05 a.m., Dave Cahill wrote:
> > plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java, line 100
> > <https://reviews.apache.org/r/13771/diff/2/?file=345161#file345161line100>
> >
> > Not an issue, just to jog my memory - we're using static methods on BroadcastDomainType instead of member method on the URI because we don't know if the URI will have a getValue method, right?
not quite, the static method allows for getting the value when the enumeration value is not known (yet)
> On Aug. 28, 2013, 2:05 a.m., Dave Cahill wrote:
> > plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java, line 248
> > <https://reviews.apache.org/r/13771/diff/2/?file=345173#file345173line248>
> >
> > This comment implies there's an issue here, but doesn't suggest a fix - do you have a fix for it?
I am adding to the existing comment here. It implies everything is alright whilst it may not. My fix here may/should resolve the issue. I will change the '(not)' into 'now'. Hope that removes your issue.
> On Aug. 28, 2013, 2:05 a.m., Dave Cahill wrote:
> > server/src/com/cloud/configuration/ConfigurationManagerImpl.java, line 2013
> > <https://reviews.apache.org/r/13771/diff/2/?file=345175#file345175line2013>
> >
> > Do you mean "why not have"? This comment seems to add confusion - if it should be changed, maybe propose a change? What would we gain from adding a None BroadcastDomainType? Is null handling working correctly already?
You are right. I will drop this comment. It was expressing my own confusion.
> On Aug. 28, 2013, 2:05 a.m., Dave Cahill wrote:
> > server/src/com/cloud/network/NetworkManagerImpl.java, line 1904
> > <https://reviews.apache.org/r/13771/diff/2/?file=345179#file345179line1904>
> >
> > Looks like this changes behavior - if isolatePvlan is null, userNetwork now gets its broadcast URI set anyway.
> >
> > Could you explain why we're making that change? I'm sure it's for a reason, I just can't tell why from a quick read.
thanks for pointing this out. It is not an issue as it get overwritten in the else {} clause, but it is superfluous at this location.
> On Aug. 28, 2013, 2:05 a.m., Dave Cahill wrote:
> > server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java, line 796
> > <https://reviews.apache.org/r/13771/diff/2/?file=345185#file345185line796>
> >
> > Not sure what this comment means - is it meant to be left in?
this is a question I can not answer. I will remove the TODO part
> On Aug. 28, 2013, 2:05 a.m., Dave Cahill wrote:
> > utils/src/com/cloud/utils/net/NetUtils.java, line 1381
> > <https://reviews.apache.org/r/13771/diff/2/?file=345186#file345186line1381>
> >
> > "these pvlan functions should take into account code in Networks.BroadcastDomainType"
> >
> > In what way? As it is, I think this comment is not so helpful.
I am warning future maintainers for the relation with the mentioned code. I will try to rephrase to make it more clear.
- daan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/#review25650
-----------------------------------------------------------
On Aug. 27, 2013, 11:36 a.m., daan Hoogland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 11:36 a.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-4346
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>
>
> Diffs
> -----
>
> plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
> plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
> plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
> plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
> plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
> plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
> plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
> plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
> server/src/com/cloud/api/ApiResponseHelper.java c771431
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 57dc0b3
> server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
> server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
> server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
> server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
> server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
> server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
> server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
> server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
> server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
> utils/src/com/cloud/utils/net/NetUtils.java 05b485b
>
> Diff: https://reviews.apache.org/r/13771/diff/
>
>
> Testing
> -------
>
> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>
>
> Thanks,
>
> daan Hoogland
>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Dave Cahill <dc...@midokura.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/#review25650
-----------------------------------------------------------
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
<https://reviews.apache.org/r/13771/#comment50147>
Not an issue, just to jog my memory - we're using static methods on BroadcastDomainType instead of member method on the URI because we don't know if the URI will have a getValue method, right?
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
<https://reviews.apache.org/r/13771/#comment50142>
Nice change here - the old substring / calculate length stuff looks quite hacky!
plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java
<https://reviews.apache.org/r/13771/#comment50143>
This comment implies there's an issue here, but doesn't suggest a fix - do you have a fix for it?
server/src/com/cloud/configuration/ConfigurationManagerImpl.java
<https://reviews.apache.org/r/13771/#comment50144>
Do you mean "why not have"? This comment seems to add confusion - if it should be changed, maybe propose a change? What would we gain from adding a None BroadcastDomainType? Is null handling working correctly already?
server/src/com/cloud/network/NetworkManagerImpl.java
<https://reviews.apache.org/r/13771/#comment50149>
Looks like this changes behavior - if isolatePvlan is null, userNetwork now gets its broadcast URI set anyway.
Could you explain why we're making that change? I'm sure it's for a reason, I just can't tell why from a quick read.
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
<https://reviews.apache.org/r/13771/#comment50150>
Not sure what this comment means - is it meant to be left in?
utils/src/com/cloud/utils/net/NetUtils.java
<https://reviews.apache.org/r/13771/#comment50146>
"these pvlan functions should take into account code in Networks.BroadcastDomainType"
In what way? As it is, I think this comment is not so helpful.
This mostly looks good, but I had a few questions (inline), mostly around comments which seemed confusing.
This 200 line patch was significantly easier to review than the previous 10k line version. :)
- Dave Cahill
On Aug. 27, 2013, 11:36 a.m., daan Hoogland wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 11:36 a.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, and Sheng Yang.
>
>
> Bugs: CLOUDSTACK-4346
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>
>
> Diffs
> -----
>
> plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
> plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
> plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
> plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
> plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
> plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
> plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
> plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
> plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
> server/src/com/cloud/api/ApiResponseHelper.java c771431
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java 57dc0b3
> server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
> server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
> server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
> server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
> server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
> server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
> server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
> server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
> server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
> utils/src/com/cloud/utils/net/NetUtils.java 05b485b
>
> Diff: https://reviews.apache.org/r/13771/diff/
>
>
> Testing
> -------
>
> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>
>
> Thanks,
>
> daan Hoogland
>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Daan Hoogland <da...@gmail.com>.
Donal,
These were tested creating and deleting various types of networks with clients.
to be precise:
a standard network
a bridged network
a nicira based network
a standard vpc with two tiert networks.
kind regards,
On Tue, Aug 27, 2013 at 3:27 PM, Donal Lafferty
<do...@citrix.com> wrote:
> How were the changes tested?
...
>> Testing
>> -------
>>
>> tested with old style uris in regular networks and vpc based networks as well
>> as in nicira based networks test build in nonoss but not all code has probably
>> been touched yet. or at least I am unsure of that.
>>
>>
>> Thanks,
>>
>> daan Hoogland
>
RE: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Donal Lafferty <do...@citrix.com>.
How were the changes tested?
> -----Original Message-----
> From: daan Hoogland [mailto:noreply@reviews.apache.org] On Behalf Of
> daan Hoogland
> Sent: 27 August 2013 12:37
> To: Sheng Yang; Dave Cahill; Chiradeep Vittal; Hugo Trippaers
> Cc: daan Hoogland; cloudstack
> Subject: Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost()
> and create(String) calls
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> -----------------------------------------------------------
>
> (Updated Aug. 27, 2013, 11:36 a.m.)
>
>
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers,
> and Sheng Yang.
>
>
> Changes
> -------
>
> re-applied the changes without formatting coerrections upon request by
> reviewer
>
>
> Bugs: CLOUDSTACK-4346
>
>
> Repository: cloudstack-git
>
>
> Description
> -------
>
> After global search and replace all calls to retrieve ids for networks from URIs
> using getHost() should be gone. Creating URI should now all use appropriate
> calls as well so maitaining the way uris are built can now be done centrally.
>
>
> Diffs (updated)
> -----
>
>
> plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/B
> aremetaNetworkGuru.java 07ee12d
>
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVi
> fDriver.java 195cf40
>
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtCo
> mputingResource.java a156ae6
>
> plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDr
> iver.java 7038d7e
>
> plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBas
> e.java 59ba001
>
> plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/
> VmwareResource.java 5ab2216
>
> plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixReso
> urceBase.java ecdec1e
> plugins/network-elements/bigswitch-
> vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
> plugins/network-elements/cisco-
> vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
> plugins/network-
> elements/f5/src/com/cloud/network/resource/F5BigIpResource.java
> 1733712
> plugins/network-elements/juniper-
> srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
> plugins/network-elements/nicira-
> nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
> plugins/network-elements/nicira-
> nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java
> ff238ed
> plugins/network-
> elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java
> 36a807f
> server/src/com/cloud/api/ApiResponseHelper.java c771431
> server/src/com/cloud/configuration/ConfigurationManagerImpl.java
> 57dc0b3
> server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java
> e91dcfa
> server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java
> a934024
>
> server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.ja
> va c14d5c7
> server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
> server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java
> 5b87d54
> server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java
> 00598dd
> server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
> server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
> server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
>
> server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManage
> rImpl.java ee0d058
> utils/src/com/cloud/utils/net/NetUtils.java 05b485b
>
> Diff: https://reviews.apache.org/r/13771/diff/
>
>
> Testing
> -------
>
> tested with old style uris in regular networks and vpc based networks as well
> as in nicira based networks test build in nonoss but not all code has probably
> been touched yet. or at least I am unsure of that.
>
>
> Thanks,
>
> daan Hoogland
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by daan Hoogland <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13771/
-----------------------------------------------------------
(Updated Aug. 27, 2013, 11:36 a.m.)
Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo Trippaers, and Sheng Yang.
Changes
-------
re-applied the changes without formatting coerrections upon request by reviewer
Bugs: CLOUDSTACK-4346
Repository: cloudstack-git
Description
-------
After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
Diffs (updated)
-----
plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java 07ee12d
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java 195cf40
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java a156ae6
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java 7038d7e
plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java 59ba001
plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java 5ab2216
plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java ecdec1e
plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java 54623e9
plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java 3ae6a08
plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java 1733712
plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java 3d3d797
plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java c7d0884
plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java ff238ed
plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java 36a807f
server/src/com/cloud/api/ApiResponseHelper.java c771431
server/src/com/cloud/configuration/ConfigurationManagerImpl.java 57dc0b3
server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java e91dcfa
server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java a934024
server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java c14d5c7
server/src/com/cloud/network/NetworkManagerImpl.java 00103e3
server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java 5b87d54
server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java 00598dd
server/src/com/cloud/network/guru/GuestNetworkGuru.java b0da42f
server/src/com/cloud/network/guru/PrivateNetworkGuru.java 6521cf4
server/src/com/cloud/network/guru/PublicNetworkGuru.java d109468
server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java ee0d058
utils/src/com/cloud/utils/net/NetUtils.java 05b485b
Diff: https://reviews.apache.org/r/13771/diff/
Testing
-------
tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
Thanks,
daan Hoogland
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Dave Cahill <dc...@midokura.com>.
Get well soon!
On Mon, Aug 26, 2013 at 3:48 PM, Daan Hoogland <da...@gmail.com>wrote:
> You are right, I incorporated Alex' auto format for eclipse. Sorry for
> that. Being sick at home today, I will try to submit an update without it
> tomorrow.
>
> regards,
>
>
>
> On Mon, Aug 26, 2013 at 6:04 AM, Dave Cahill <dc...@midokura.com> wrote:
>
>> Hi Daan,
>>
>> I started to take a look - the diff is ~10k lines long, most of which
>> appears to be whitespace changes.
>>
>> Are the whitespace changes important? Without them, the patch might be a
>> lot easier to review.
>>
>> Thanks,
>> Dave.
>>
>>
>>
>>
>> On Sun, Aug 25, 2013 at 7:37 PM, Daan Hoogland <da...@gmail.com>wrote:
>>
>>> Sheng, Dave, Chiradeep and Hugo,
>>>
>>> Can you please review this? In my experience the time that this patch
>>> will expire is rather short, I have been resolving conflicts on this a lot.
>>> If now, given the 4.2 release is inconvenient, I would like to set a window
>>> for submitting this in which I will rebase it a couple of times so as to
>>> make sure it works in the end.
>>>
>>> thanks,
>>> Daan
>>>
>>>
>>> On Fri, Aug 23, 2013 at 10:48 AM, daan Hoogland <daan.hoogland@gmail.com
>>> > wrote:
>>>
>>>> This is an automatically generated e-mail. To reply, visit:
>>>> https://reviews.apache.org/r/13771/
>>>> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo
>>>> Trippaers, and Sheng Yang.
>>>> By daan Hoogland.
>>>> *Bugs: * CLOUDSTACK-4346
>>>> *Repository: * cloudstack-git
>>>> Description
>>>>
>>>> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>>>>
>>>> Testing
>>>>
>>>> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
>>>> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>>>>
>>>> Diffs
>>>>
>>>> - plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java
>>>> (07ee12d)
>>>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
>>>> (195cf40)
>>>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
>>>> (a156ae6)
>>>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
>>>> (7038d7e)
>>>> - plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java
>>>> (59ba001)
>>>> - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
>>>> (5ab2216)
>>>> - plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
>>>> (ecdec1e)
>>>> - plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java
>>>> (54623e9)
>>>> - plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java
>>>> (3ae6a08)
>>>> - plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java
>>>> (1733712)
>>>> - plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java
>>>> (3d3d797)
>>>> - plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java
>>>> (c7d0884)
>>>> - plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java
>>>> (ff238ed)
>>>> - plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java
>>>> (36a807f)
>>>> - server/src/com/cloud/api/ApiResponseHelper.java (c771431)
>>>> - server/src/com/cloud/configuration/ConfigurationManagerImpl.java
>>>> (57dc0b3)
>>>> - server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java
>>>> (e91dcfa)
>>>> - server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java
>>>> (a934024)
>>>> - server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java
>>>> (c14d5c7)
>>>> - server/src/com/cloud/network/NetworkManagerImpl.java (00103e3)
>>>> - server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java
>>>> (5b87d54)
>>>> - server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java
>>>> (00598dd)
>>>> - server/src/com/cloud/network/guru/GuestNetworkGuru.java (b0da42f)
>>>> - server/src/com/cloud/network/guru/PrivateNetworkGuru.java
>>>> (6521cf4)
>>>> - server/src/com/cloud/network/guru/PublicNetworkGuru.java (d109468)
>>>> - server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
>>>> (ee0d058)
>>>> - utils/src/com/cloud/utils/net/NetUtils.java (05b485b)
>>>>
>>>> View Diff <https://reviews.apache.org/r/13771/diff/>
>>>>
>>>
>>>
>>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Daan Hoogland <da...@gmail.com>.
You are right, I incorporated Alex' auto format for eclipse. Sorry for
that. Being sick at home today, I will try to submit an update without it
tomorrow.
regards,
On Mon, Aug 26, 2013 at 6:04 AM, Dave Cahill <dc...@midokura.com> wrote:
> Hi Daan,
>
> I started to take a look - the diff is ~10k lines long, most of which
> appears to be whitespace changes.
>
> Are the whitespace changes important? Without them, the patch might be a
> lot easier to review.
>
> Thanks,
> Dave.
>
>
>
>
> On Sun, Aug 25, 2013 at 7:37 PM, Daan Hoogland <da...@gmail.com>wrote:
>
>> Sheng, Dave, Chiradeep and Hugo,
>>
>> Can you please review this? In my experience the time that this patch
>> will expire is rather short, I have been resolving conflicts on this a lot.
>> If now, given the 4.2 release is inconvenient, I would like to set a window
>> for submitting this in which I will rebase it a couple of times so as to
>> make sure it works in the end.
>>
>> thanks,
>> Daan
>>
>>
>> On Fri, Aug 23, 2013 at 10:48 AM, daan Hoogland <da...@gmail.com>wrote:
>>
>>> This is an automatically generated e-mail. To reply, visit:
>>> https://reviews.apache.org/r/13771/
>>> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo
>>> Trippaers, and Sheng Yang.
>>> By daan Hoogland.
>>> *Bugs: * CLOUDSTACK-4346
>>> *Repository: * cloudstack-git
>>> Description
>>>
>>> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>>>
>>> Testing
>>>
>>> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
>>> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>>>
>>> Diffs
>>>
>>> - plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java
>>> (07ee12d)
>>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
>>> (195cf40)
>>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
>>> (a156ae6)
>>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
>>> (7038d7e)
>>> - plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java
>>> (59ba001)
>>> - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
>>> (5ab2216)
>>> - plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
>>> (ecdec1e)
>>> - plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java
>>> (54623e9)
>>> - plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java
>>> (3ae6a08)
>>> - plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java
>>> (1733712)
>>> - plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java
>>> (3d3d797)
>>> - plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java
>>> (c7d0884)
>>> - plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java
>>> (ff238ed)
>>> - plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java
>>> (36a807f)
>>> - server/src/com/cloud/api/ApiResponseHelper.java (c771431)
>>> - server/src/com/cloud/configuration/ConfigurationManagerImpl.java
>>> (57dc0b3)
>>> - server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java
>>> (e91dcfa)
>>> - server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java
>>> (a934024)
>>> - server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java
>>> (c14d5c7)
>>> - server/src/com/cloud/network/NetworkManagerImpl.java (00103e3)
>>> - server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java
>>> (5b87d54)
>>> - server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java
>>> (00598dd)
>>> - server/src/com/cloud/network/guru/GuestNetworkGuru.java (b0da42f)
>>> - server/src/com/cloud/network/guru/PrivateNetworkGuru.java (6521cf4)
>>> - server/src/com/cloud/network/guru/PublicNetworkGuru.java (d109468)
>>> - server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
>>> (ee0d058)
>>> - utils/src/com/cloud/utils/net/NetUtils.java (05b485b)
>>>
>>> View Diff <https://reviews.apache.org/r/13771/diff/>
>>>
>>
>>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Dave Cahill <dc...@midokura.com>.
Hi Daan,
I started to take a look - the diff is ~10k lines long, most of which
appears to be whitespace changes.
Are the whitespace changes important? Without them, the patch might be a
lot easier to review.
Thanks,
Dave.
On Sun, Aug 25, 2013 at 7:37 PM, Daan Hoogland <da...@gmail.com>wrote:
> Sheng, Dave, Chiradeep and Hugo,
>
> Can you please review this? In my experience the time that this patch will
> expire is rather short, I have been resolving conflicts on this a lot. If
> now, given the 4.2 release is inconvenient, I would like to set a window
> for submitting this in which I will rebase it a couple of times so as to
> make sure it works in the end.
>
> thanks,
> Daan
>
>
> On Fri, Aug 23, 2013 at 10:48 AM, daan Hoogland <da...@gmail.com>wrote:
>
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/13771/
>> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo
>> Trippaers, and Sheng Yang.
>> By daan Hoogland.
>> *Bugs: * CLOUDSTACK-4346
>> *Repository: * cloudstack-git
>> Description
>>
>> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>>
>> Testing
>>
>> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
>> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>>
>> Diffs
>>
>> - plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java
>> (07ee12d)
>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
>> (195cf40)
>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
>> (a156ae6)
>> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
>> (7038d7e)
>> - plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java
>> (59ba001)
>> - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
>> (5ab2216)
>> - plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
>> (ecdec1e)
>> - plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java
>> (54623e9)
>> - plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java
>> (3ae6a08)
>> - plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java
>> (1733712)
>> - plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java
>> (3d3d797)
>> - plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java
>> (c7d0884)
>> - plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java
>> (ff238ed)
>> - plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java
>> (36a807f)
>> - server/src/com/cloud/api/ApiResponseHelper.java (c771431)
>> - server/src/com/cloud/configuration/ConfigurationManagerImpl.java
>> (57dc0b3)
>> - server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java
>> (e91dcfa)
>> - server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java
>> (a934024)
>> - server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java
>> (c14d5c7)
>> - server/src/com/cloud/network/NetworkManagerImpl.java (00103e3)
>> - server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java
>> (5b87d54)
>> - server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java
>> (00598dd)
>> - server/src/com/cloud/network/guru/GuestNetworkGuru.java (b0da42f)
>> - server/src/com/cloud/network/guru/PrivateNetworkGuru.java (6521cf4)
>> - server/src/com/cloud/network/guru/PublicNetworkGuru.java (d109468)
>> - server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
>> (ee0d058)
>> - utils/src/com/cloud/utils/net/NetUtils.java (05b485b)
>>
>> View Diff <https://reviews.apache.org/r/13771/diff/>
>>
>
>
Re: Review Request 13771: CLOUDSTACK-4346 replace URI getHost() and
create(String) calls
Posted by Daan Hoogland <da...@gmail.com>.
Sheng, Dave, Chiradeep and Hugo,
Can you please review this? In my experience the time that this patch will
expire is rather short, I have been resolving conflicts on this a lot. If
now, given the 4.2 release is inconvenient, I would like to set a window
for submitting this in which I will rebase it a couple of times so as to
make sure it works in the end.
thanks,
Daan
On Fri, Aug 23, 2013 at 10:48 AM, daan Hoogland <da...@gmail.com>wrote:
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13771/
> Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Hugo
> Trippaers, and Sheng Yang.
> By daan Hoogland.
> *Bugs: * CLOUDSTACK-4346
> *Repository: * cloudstack-git
> Description
>
> After global search and replace all calls to retrieve ids for networks from URIs using getHost() should be gone. Creating URI should now all use appropriate calls as well so maitaining the way uris are built can now be done centrally.
>
> Testing
>
> tested with old style uris in regular networks and vpc based networks as well as in nicira based networks
> test build in nonoss but not all code has probably been touched yet. or at least I am unsure of that.
>
> Diffs
>
> - plugins/hypervisors/baremetal/src/com/cloud/baremetal/networkservice/BaremetaNetworkGuru.java
> (07ee12d)
> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java
> (195cf40)
> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
> (a156ae6)
> - plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/OvsVifDriver.java
> (7038d7e)
> - plugins/hypervisors/ovm/src/com/cloud/ovm/hypervisor/OvmResourceBase.java
> (59ba001)
> - plugins/hypervisors/vmware/src/com/cloud/hypervisor/vmware/resource/VmwareResource.java
> (5ab2216)
> - plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixResourceBase.java
> (ecdec1e)
> - plugins/network-elements/bigswitch-vns/src/com/cloud/network/element/BigSwitchVnsElement.java
> (54623e9)
> - plugins/network-elements/cisco-vnmc/src/com/cloud/network/element/CiscoVnmcElement.java
> (3ae6a08)
> - plugins/network-elements/f5/src/com/cloud/network/resource/F5BigIpResource.java
> (1733712)
> - plugins/network-elements/juniper-srx/src/com/cloud/network/resource/JuniperSrxResource.java
> (3d3d797)
> - plugins/network-elements/nicira-nvp/src/com/cloud/network/element/NiciraNvpElement.java
> (c7d0884)
> - plugins/network-elements/nicira-nvp/src/com/cloud/network/guru/NiciraNvpGuestNetworkGuru.java
> (ff238ed)
> - plugins/network-elements/ovs/src/com/cloud/network/ovs/OvsTunnelManagerImpl.java
> (36a807f)
> - server/src/com/cloud/api/ApiResponseHelper.java (c771431)
> - server/src/com/cloud/configuration/ConfigurationManagerImpl.java
> (57dc0b3)
> - server/src/com/cloud/network/ExternalDeviceUsageManagerImpl.java
> (e91dcfa)
> - server/src/com/cloud/network/ExternalFirewallDeviceManagerImpl.java
> (a934024)
> - server/src/com/cloud/network/ExternalLoadBalancerDeviceManagerImpl.java
> (c14d5c7)
> - server/src/com/cloud/network/NetworkManagerImpl.java (00103e3)
> - server/src/com/cloud/network/guru/DirectPodBasedNetworkGuru.java
> (5b87d54)
> - server/src/com/cloud/network/guru/ExternalGuestNetworkGuru.java
> (00598dd)
> - server/src/com/cloud/network/guru/GuestNetworkGuru.java (b0da42f)
> - server/src/com/cloud/network/guru/PrivateNetworkGuru.java (6521cf4)
> - server/src/com/cloud/network/guru/PublicNetworkGuru.java (d109468)
> - server/src/com/cloud/network/router/VpcVirtualNetworkApplianceManagerImpl.java
> (ee0d058)
> - utils/src/com/cloud/utils/net/NetUtils.java (05b485b)
>
> View Diff <https://reviews.apache.org/r/13771/diff/>
>