> On Jan. 10, 2014, 10:30 a.m., daan Hoogland wrote: > > I am fine with this with the exception of one worry: 'vlan://untagged' may > > be compared to 'untagged' at some points in the system. I would like to > > include a change in NetUtils.java for this: > > if (VLAN_UNTAGGED.equalsIgnoreCase(one) && > > VLAN_UNTAGGED.equalsIgnoreCase(other)) > > becomes > > if (one.contains(VLAN_UNTAGGED) && other.contains(VLAN_UNTAGGED)) > > in isSameIsolationId(String,String)
Ok, thanks. I'll test that and apply the fix if it doesn't change any of the cases. - Marcus ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16758/#review31514 ----------------------------------------------------------- On Jan. 9, 2014, 6:52 p.m., Marcus Sorensen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16758/ > ----------------------------------------------------------- > > (Updated Jan. 9, 2014, 6:52 p.m.) > > > Review request for cloudstack and daan Hoogland. > > > Bugs: CLOUDSTACK-5502 > https://issues.apache.org/jira/browse/CLOUDSTACK-5502 > > > Repository: cloudstack-git > > > Description > ------- > > See conversation on mailing list and CLOUDSTACK-5502: > > To recap, in 4.2 > the UI would interpret no vlan entered as 'untagged' and pass that as > a parameter. Undocumented or not, it was being used by the API's > biggest customer. > > with 4.2, passing 'untagged' resulted in: > > Saving vlan range > Vlan[untagged|12.12.12.1|255.255.255.0|null|null|12.12.12.10-12.12.12.20||null1004] > > with 4.3 AND 4.2, passing '' fails with 'Vlan id required', > because untagged was changed to null, while an empty string fell > through the null check and resulted in > Saving vlan range > Vlan[|12.12.12.1|255.255.255.0|null|null|12.12.12.10-12.12.12.20||null1004] > which breaks later: Networks.java line 323, "Unable to convert to > isolation URI:" > > with 4.3, minus patch aaf3979cf92518d3dc5587ea0192f4b3ce1e7866, > passing vlan=untagged results in: > Saving vlan range > Vlan[vlan://untagged|12.12.12.1|255.255.255.0|null|null|12.12.12.10-12.12.12.20||null1004] > > Which is exactly the behavior we want to see for both "" and > "untagged", if we want to maintain compatibility with previous public > network deployments. > > > Diffs > ----- > > > api/src/org/apache/cloudstack/api/command/admin/vlan/CreateVlanIpRangeCmd.java > 541da1e > server/src/com/cloud/configuration/ConfigurationManagerImpl.java f9d282f > > Diff: https://reviews.apache.org/r/16758/diff/ > > > Testing > ------- > > Tested advanced zone deployments, passing both empty string for 'vlan' and > 'untagged' > > > Thanks, > > Marcus Sorensen > >
