Hi Daan, I think your patch is completely wrong.
The BroadcastDomainType.getValue() would accept format of URI, not the number. I don't think your change would work, either in code or by semantic. I think your commit would break all elements your code touched. The current assumption of vlanId and secondaryVlanId are both numbers, not URI. Please revert it. --Sheng On Thu, Nov 7, 2013 at 4:07 PM, Min Chen <min.c...@citrix.com> wrote: > Then we need to clearly define the semantic of parameter "vlanId" of > HypervisorHostHelper.prepareNetwork is indeed the vlan Id string, not > other format. The invoker method should pass the correct one to this > utility to make it work. > > Thanks > -min > > On 11/7/13 3:43 PM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote: > > >Doesn't sound like a guarantee, If the stack is build otherwise there > >might come in a different (non-integer-representing) string. > > > >On Thu, Nov 7, 2013 at 6:08 PM, Min Chen <min.c...@citrix.com> wrote: > >> Yes. From callstack, > >> BroadcastDomainType.getValue(BroadcastDomainType.fromString(vlanId) is > >> already called before going to that routine. > >> > >> Thanks > >> -min > >> > >> On 11/7/13 1:51 AM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote: > >> > >>>H Min, > >>> > >>>Your fix will work if you can guarantee that the String passed is a > >>>integer. if it has the chance of being in the form of for instance > >>>vlan://<some_number> , you should use: > >>>vid = > >>>Integer.parseInt(BroadcastDomainType.getValue(BroadcastDomainType.fromSt > >>>ri > >>>ng(vlanId))); > >>> > >>>regards, > >>>Daan > >>> > >>> > >>>On Wed, Nov 6, 2013 at 3:25 AM, Min Chen <min.c...@citrix.com> wrote: > >>>> Hi Daan, > >>>> > >>>> Your commit > >>>> > >>>> > https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;h=25c > >>>>8c > >>>>ee01a450ee924fe108cafe54b046485ab2b > >>>> broke Vmware advanced zone setup on Master, where system vm starts > >>>>failed > >>>> with "NumberFormatException", see details in > >>>> https://issues.apache.org/jira/browse/CLOUDSTACK-5046. I fixed this > >>>>blocker > >>>> bug (commit 94f9b31c9a4c7ae67feabbe16d2ea753e3183289) by reverting > >>>>part > >>>>of > >>>> your change on HypervisorHostHelper method, but not sure if there is > >>>>any > >>>> side effect on your another functionality fixed in your commit. Can > >>>>you > >>>> please review to make sure that it is ok? > >>>> > >>>> Thanks > >>>> -min > >> > >
