Currently if I create my public range with 'untagged', I get:

    "Vlan id is required when add ip range to the public network"

So at the very least it seems the patch is not working. It may be
better to edit CreateVlanIpRangeCmd and change the parameter at the
source if 'untagged' is passed in, like:
     public String getVlan() {
+        if (vlan.equalsIgnoreCase("untagged")) {
+            vlan = "";
+        }
         return vlan;
     }


If I create my public range with "", I get a failure in bringing up
the first system vm:

    Networks.java line 323, "Unable to convert to isolation URI:"

I'm running tests against the version with your patch reverted, but I
think the root is that untagged (and perhaps empty string) was
previously interpreted as Vlan.UNTAGGED in 4.2, and is not in 4.3,
nothing to do with your change.


There is a lot of code that is looking for untagged, or looking for
null, and either setting null to 'untagged' or skipping code entirely
if it is null, e.g. NetworkOrchestrator:

basic networking seems to get around it:

            if (vlanId == null) {
                vlanId = Vlan.UNTAGGED;
            } else {
                if (!vlanId.equalsIgnoreCase(Vlan.UNTAGGED)) {
                    throw new InvalidParameterValueException("Only
vlan " + Vlan.UNTAGGED + " can be created in " + "the zone of type " +
NetworkType.Basic);
                }
            }

but advanced networking skips stuff altogether if vlanId is null
(basic networking set it to UNTAGGED and saved the day per previous
code):

                if (vlanIdFinal != null) {
                        if (isolatedPvlan == null) {
                        URI uri = BroadcastDomainType.fromString(vlanIdFinal);
                        userNetwork.setBroadcastUri(uri);
                                if
(!vlanIdFinal.equalsIgnoreCase(Vlan.UNTAGGED)) {

userNetwork.setBroadcastDomainType(BroadcastDomainType.Vlan);
                                } else {

userNetwork.setBroadcastDomainType(BroadcastDomainType.Native);
                                }
                        } else {
                                if
(vlanIdFinal.equalsIgnoreCase(Vlan.UNTAGGED)) {
                                        throw new
InvalidParameterValueException("Cannot support pvlan with untagged
primary vlan!");
                                }

userNetwork.setBroadcastUri(NetUtils.generateUriForPvlan(vlanIdFinal,
isolatedPvlan));

userNetwork.setBroadcastDomainType(BroadcastDomainType.Pvlan);
                        }
                }


On Thu, Jan 9, 2014 at 10:10 AM, Marcus Sorensen <shadow...@gmail.com> wrote:
> Let me see if I can dig up the specifics. If I remember right, it just
> seemed backward. No vlan id should be treated as untagged, not
> untagged treated as null. There's a lot of code that is skipped if
> vlanid is null, but does useful things if UNTAGGED. Ill be back in an
> hour or two with the results.
>
> On Thu, Jan 9, 2014 at 1:25 AM, Daan Hoogland <daan.hoogl...@gmail.com> wrote:
>> H Marcus,
>>
>> concerning https://issues.apache.org/jira/browse/CLOUDSTACK-5502
>>
>> Is this still a problem for you? Can I have some more info? I was
>> aware of the setting of vlan back to untagged when I wrote it. That's
>> why I thought it wouldn't be a problem. In a prior mail of you you
>> downwrote the issue as specific to your use-case. I really want to
>> understand as much as possible of these problems with
>> BroadcastDomainType as little varieties appear all over the place once
>> in a while.
>>
>> thanks,
>> Daan
>>
>>
>> ---------- Forwarded message ----------
>> From: Daan Hoogland <daan.hoogl...@gmail.com>
>> Date: Tue, Jan 7, 2014 at 4:30 PM
>> Subject: Re: CLOUDSTACK-5502 [Automation] createVlanIpRange API
>> failing, if you pass VLAN
>> To: dev <dev@cloudstack.apache.org>, Marcus Sorensen <shadow...@gmail.com>
>>
>>
>> Sachin,
>>
>> I will look at it and propose some. @Marcus can you send me some
>> description/stacktrace/log on how your testcase fails.
>>
>> regards
>>
>>
>> On Tue, Jan 7, 2014 at 4:13 PM, Sachchidanand Vaidya
>> <vaidy...@juniper.net> wrote:
>>> Hi,
>>>     Bug CS-5502 was fixed (12/20/13) and then reopened (12/26/13) again 
>>> since for
>>> Vlan=UNTAGGED  case,  createVlanIpRange() API fails with "Vlan id is 
>>> required when add ip
>>> range to the public network".  Vlan ="untagged" can be a valid case for 
>>> public networks
>>> (in advanced networking) as is the case in  juniper-contrail solution.
>>> Is there a fix planned for this bug?
>>>
>>> Thanks & Regards,
>>> Sachin

Reply via email to