Donal, The mix of vlan://<number> and <number> is one of the issues i had to deal with doing this. The vlan not being handled by my first patch correctly is why this second one was made. for vlans it reverts what I did before. What is it you are seeing? And how does it relate to the BroadcastDomainType? The code you refer to expects something like '<scheme>://<number>'. The reason mido needs the string concatination is because it can contain 'mido://<somestring>:<someotherstring>'.
Daan On Wed, Aug 7, 2013 at 7:12 PM, Donal Lafferty <donal.laffe...@citrix.com>wrote: > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12849/ > > On August 6th, 2013, 11:40 p.m. UTC, *Sheng Yang* wrote: > > > api/src/com/cloud/network/Networks.java<https://reviews.apache.org/r/12849/diff/5/?file=337785#file337785line79> > (Diff > revision 5) > > public class Networks { > > 79 > > return new URI("vlan", value.toString(), null, null); > > I don't quite understand this part. Why "//" is missing in this case? Who > would require "//" and who wouldn't? > > On August 7th, 2013, 8:42 a.m. UTC, *daan Hoogland* wrote: > > Only the mido code needs the explicit // the rest can deal with URI > constructors > > Are you sure? I'm seeing problems with > CitrixResourceBase.getNetwork(CitrixResourceBase.java:1043): > > long vlan = Long.parseLong(broadcastUri.getHost()); > > > > - Donal > > On August 7th, 2013, 9:40 a.m. UTC, daan Hoogland wrote: > Review request for cloudstack, Chiradeep Vittal, Dave Cahill, Koushik > Das, and Sheng Yang. > By daan Hoogland. > > *Updated Aug. 7, 2013, 9:40 a.m.* > *Repository: * cloudstack-git > Description > > Both BroadcastDomainType and IsolationType needed some extra code for > backwards compatibility > > Diffs > > - api/src/com/cloud/network/Networks.java (c76c3d4) > - api/test/com/cloud/network/NetworksTest.java (31114e8) > > View Diff <https://reviews.apache.org/r/12849/diff/> >
