Yeah, but I wasn't sure of the coder's intend and if your replacement code
meet their expectations, so I didn't change it. I was hoping someone would
claim the code and chime in. :)
On Wed, Jan 1, 2014 at 10:16 PM, Marcus Sorensen <shadow...@gmail.com>wrote:
> Yeah, it would be clearer if they were checked separately:
>
> if (one == null || one.isEmpty()) {
> return true;
> } else if ( other == null || other.isEmpty()) [
> return true;
> }
>
> or something like that.
>
> On Wed, Jan 1, 2014 at 10:00 PM, Mike Tutkowski
> <mike.tutkow...@solidfire.com> wrote:
> > I should say this check doesn't have to catch it...it might, but it
> doesn't
> > have to (depends on the value of one).
> >
> >
> > On Wed, Jan 1, 2014 at 9:59 PM, Mike Tutkowski <
> mike.tutkow...@solidfire.com
> >> wrote:
> >
> >> Yeah, in my case I'm just setting up a basic zone with a XenServer host.
> >>
> >> The code in NetUtils checks for null or "" on the variable in question
> >> that's passed in. However, in a certain case, null for that variable can
> >> slip by and lead to a NPE.
> >>
> >> if ((one == null || one.equals(""))
> >>
> >> &&
> >>
> >> (other == null || other.equals("")))
> >>
> >> {
> >>
> >> return true;
> >>
> >> }
> >>
> >> if other == null, this will not catch it and it can throw a NPE later.
> >>
> >>
> >> On Wed, Jan 1, 2014 at 9:51 PM, Marcus Sorensen <shadow...@gmail.com
> >wrote:
> >>
> >>> You can do "git blame (file)" and it will show you each line and the
> >>> commit. You can also do a git log on the file. The issue may not be as
> >>> obvious as that, though, there may be something totally unrelated
> causing
> >>> that object to end up null in this code. Or it may be specific to your
> >>> setup, some obscure bug nobody else is hitting.
> >>> On Jan 1, 2014 4:22 PM, "Mike Tutkowski" <mike.tutkow...@solidfire.com
> >
> >>> wrote:
> >>>
> >>> > This is in 4.3.
> >>> >
> >>> > I know the file is NetUtils, but I'm not sure in Git how to look at
> the
> >>> > history of a particular file like I could do in SVN.
> >>> >
> >>> >
> >>> > On Wed, Jan 1, 2014 at 3:55 PM, Marcus Sorensen <shadow...@gmail.com
> >
> >>> > wrote:
> >>> >
> >>> > > Which branch? I see these in master, you can check out the commit
> just
> >>> > > before these and see if it helps:
> >>> > >
> >>> > > commit b477e4e830597100f0c0171dd8e56f4033bd07aa
> >>> > > Author: Daan Hoogland <dhoogl...@schubergphilis.com>
> >>> > > Date: Tue Dec 31 12:52:51 2013 +0100
> >>> > >
> >>> > > some xtra cases
> >>> > >
> >>> > > commit 2cf356e047e26977c1d294fafc57e986c04fc5f4
> >>> > > Author: Daan Hoogland <dhoogl...@schubergphilis.com>
> >>> > > Date: Tue Dec 31 12:25:17 2013 +0100
> >>> > >
> >>> > > isSameIsolationId
> >>> > >
> >>> > > commit 04570eefed9a0ee1eca1fd700ed5732ba67150ce
> >>> > > Author: Daan Hoogland <d...@onecht.net>
> >>> > > Date: Fri Dec 20 16:47:58 2013 +0100
> >>> > >
> >>> > > check vlans and other isolation types
> >>> > >
> >>> > > commit d50517e931e68daef6735bd18273499fee0d4649
> >>> > > Author: Sateesh Chodapuneedi <sate...@apache.org>
> >>> > > Date: Tue Dec 31 07:16:35 2013 +0530
> >>> > >
> >>> > > I also have a commit just after these, but it was pretty minor and
> >>> > > only to KVM agent code.
> >>> > >
> >>> > > On Wed, Jan 1, 2014 at 3:27 PM, Mike Tutkowski
> >>> > > <mike.tutkow...@solidfire.com> wrote:
> >>> > > > Hey guys,
> >>> > > >
> >>> > > > The NPE I saw last night was related to "isolation id." Is it
> >>> possible
> >>> > > this
> >>> > > > NPE is related to something new that was put that you are talking
> >>> about
> >>> > > > here?
> >>> > > >
> >>> > > > Thank!
> >>> > > >
> >>> > > > ERROR [c.c.a.ApiServer] (1583467451@qtp-185135566-2:ctx-ae5d80b2
> >>> > > > ctx-5c12c4d9) unhandled exception executing api command:
> >>> > > createVlanIpRange
> >>> > > > java.lang.NullPointerException
> >>> > > > at
> >>> > com.cloud.utils.net.NetUtils.isSameIsolationId(NetUtils.java:1419)
> >>> > > > at com.cloud.configuration.ConfigurationManagerImpl.
> >>> > > > createVlanAndPublicIpRange(ConfigurationManagerImpl.java:2474)
> >>> > > > at sun.reflect.NativeMethodAccessorImpl.invoke0(Native
> Method)
> >>> > > > at sun.reflect.NativeMethodAccessorImpl.invoke(
> >>> > > > NativeMethodAccessorImpl.java:57)
> >>> > > > at sun.reflect.DelegatingMethodAccessorImpl.invoke(
> >>> > > > DelegatingMethodAccessorImpl.java:43)
> >>> > > > at java.lang.reflect.Method.invoke(Method.java:616)
> >>> > > > at org.springframework.aop.support.AopUtils.
> >>> > > > invokeJoinpointUsingReflection(AopUtils.java:317)
> >>> > > > at
> org.springframework.aop.framework.ReflectiveMethodInvocation.
> >>> > > > invokeJoinpoint(ReflectiveMethodInvocation.java:183)
> >>> > > > at
> >>> > >
> org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(
> >>> > > > ReflectiveMethodInvocation.java:150)
> >>> > > > at com.cloud.event.ActionEventInterceptor.invoke(
> >>> > > > ActionEventInterceptor.java:50)
> >>> > > > at
> >>> > >
> org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(
> >>> > > > ReflectiveMethodInvocation.java:161)
> >>> > > > at
> >>> org.springframework.aop.interceptor.ExposeInvocationInterceptor.
> >>> > > > invoke(ExposeInvocationInterceptor.java:91)
> >>> > > > at
> >>> > >
> org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(
> >>> > > > ReflectiveMethodInvocation.java:172)
> >>> > > > at org.springframework.aop.framework.JdkDynamicAopProxy.
> >>> > > > invoke(JdkDynamicAopProxy.java:204)
> >>> > > > at sun.proxy.$Proxy96.createVlanAndPublicIpRange(Unknown
> Source)
> >>> > > > at org.apache.cloudstack.api.command.admin.vlan.
> >>> > > > CreateVlanIpRangeCmd.execute(CreateVlanIpRangeCmd.java:211)
> >>> > > > at
> com.cloud.api.ApiDispatcher.dispatch(ApiDispatcher.java:161)
> >>> > > > at com.cloud.api.ApiServer.queueCommand(ApiServer.java:530)
> >>> > > > at com.cloud.api.ApiServer.handleRequest(ApiServer.java:373)
> >>> > > > at
> >>> > >
> com.cloud.api.ApiServlet.processRequestInContext(ApiServlet.java:322)
> >>> > > > at com.cloud.api.ApiServlet.access$000(ApiServlet.java:52)
> >>> > > > at com.cloud.api.ApiServlet$1.run(ApiServlet.java:114)
> >>> > > > at org.apache.cloudstack.managed.context.impl.
> >>> > > > DefaultManagedContext$1.call(DefaultManagedContext.java:56)
> >>> > > > at
> >>> > org.apache.cloudstack.managed.context.impl.DefaultManagedContext.
> >>> > > > callWithContext(DefaultManagedContext.java:103)
> >>> > > > at
> >>> > org.apache.cloudstack.managed.context.impl.DefaultManagedContext.
> >>> > > > runWithContext(DefaultManagedContext.java:53)
> >>> > > > at
> com.cloud.api.ApiServlet.processRequest(ApiServlet.java:111)
> >>> > > >
> >>> > > >
> >>> > > > On Wed, Jan 1, 2014 at 2:33 PM, Marcus Sorensen <
> >>> shadow...@gmail.com>
> >>> > > wrote:
> >>> > > >
> >>> > > >> That's just it. The isolation type *is* provided when creating
> >>> > > >> physical network. If I create a physical network with isolation
> >>> type
> >>> > > >> 'VXLAN', and then add traffic type of 'Public', it doesn't obey
> it.
> >>> > > >> There's physical_networks and networks, when the zone is
> created,
> >>> an
> >>> > > >> entry goes in network that is Public/Vlan, hardcoded. The Public
> >>> > > >> traffic type uses this, regardless of what the physical_network
> its
> >>> > > >> being added to says. So if we updated the the public network
> table
> >>> row
> >>> > > >> with the correct isolation method for that physical network we
> are
> >>> > > >> adding traffic type to when we add the public traffic type, that
> >>> would
> >>> > > >> work. It's worth noting that a zone can only have one physical
> >>> network
> >>> > > >> with traffic type of public.
> >>> > > >>
> >>> > > >> On Wed, Jan 1, 2014 at 12:37 PM, Daan Hoogland <
> >>> > daan.hoogl...@gmail.com
> >>> > > >
> >>> > > >> wrote:
> >>> > > >> >> While I've got your attention, what's the deal with isolation
> >>> > method
> >>> > > vs
> >>> > > >> broadcast method? These are always set to the same thing as far
> as
> >>> > I've
> >>> > > >> seen.
> >>> > > >> >
> >>> > > >> > I've been asking this but haven't found the answer yet. There
> is
> >>> an
> >>> > > >> > overlap but both have some extra values the other hasn't.
> >>> > > >> >
> >>> > > >> > I don't like either of your solutions but haven't got a good
> >>> > > >> > alternative. Best would be to be able to set the isolation
> type
> >>> on
> >>> > > >> > each physical network on creation. The wizard and zone
> creation
> >>> api
> >>> > > >> > command would have to be extended and allow for vlan as
> default.
> >>> > > >> >
> >>> > > >> > regards,
> >>> > > >> >
> >>> > > >> > On Wed, Jan 1, 2014 at 8:53 AM, Marcus Sorensen <
> >>> > shadow...@gmail.com>
> >>> > > >> wrote:
> >>> > > >> >> I suppose the answer might be to update the network with the
> >>> proper
> >>> > > >> >> isolation method when the traffic type is added. Look up the
> >>> > physical
> >>> > > >> >> network's isolation method, grab network object for the
> public
> >>> > > network,
> >>> > > >> and
> >>> > > >> >> set the right isolation.
> >>> > > >> >> On Jan 1, 2014 12:46 AM, "Marcus Sorensen" <
> shadow...@gmail.com
> >>> >
> >>> > > wrote:
> >>> > > >> >>
> >>> > > >> >>> I ran into an issue today that I'm still trying to wrap my
> >>> head
> >>> > > >> >>> around, and I wanted to bounce this off of you guys. I have
> a
> >>> > > physical
> >>> > > >> >>> network whose isolation method is set to 'VXLAN' (v4.3+). I
> >>> add my
> >>> > > >> >>> Public traffic type to it. I'd assume that nics generated
> for
> >>> > public
> >>> > > >> >>> traffic would have the standard vxlan:// URI for isolation
> >>> URI
> >>> > and
> >>> > > >> >>> broadcast URI, but they just have a vlan://. Digging into
> it,
> >>> it
> >>> > > seems
> >>> > > >> >>> that public traffic is hard-coded to
> BroadcastDomainType.Vlan.
> >>> I
> >>> > > fixed
> >>> > > >> >>> this fairly easily for my testing, there were only a few
> >>> places to
> >>> > > >> >>> fix, by pulling the BroadcastDomainType from the network
> object
> >>> > > rather
> >>> > > >> >>> than hardcoding it, but that found another problem. This
> only
> >>> > works
> >>> > > if
> >>> > > >> >>> I change the broadcast type in the 'networks' mysql table by
> >>> hand,
> >>> > > as
> >>> > > >> >>> during zone deployment the public network creation is also
> >>> > > hard-coded
> >>> > > >> >>> to vlan.
> >>> > > >> >>>
> >>> > > >> >>> I'm not sure how to go about fixing this, since the
> Public,
> >>> > > Control,
> >>> > > >> >>> Management networks are created upon zone deployment, (see
> >>> > > >> >>> createDefaultSystemNetworks). The immediate thing that
> jumped
> >>> out
> >>> > > was
> >>> > > >> >>> a config variable for public isolation method, set prior to
> >>> zone
> >>> > > >> >>> deployment, or perhaps even one that overrides what's in the
> >>> > table.
> >>> > > >> >>>
> >>> > > >> >>> While I've got your attention, what's the deal with
> isolation
> >>> > > method
> >>> > > >> >>> vs broadcast method? These are always set to the same thing
> as
> >>> far
> >>> > > as
> >>> > > >> >>> I've seen.
> >>> > > >> >>>
> >>> > > >>
> >>> > > >
> >>> > > >
> >>> > > >
> >>> > > > --
> >>> > > > *Mike Tutkowski*
> >>> > > > *Senior CloudStack Developer, SolidFire Inc.*
> >>> > > > e: mike.tutkow...@solidfire.com
> >>> > > > o: 303.746.7302
> >>> > > > Advancing the way the world uses the
> >>> > > > cloud<http://solidfire.com/solution/overview/?video=play>
> >>> > > > *™*
> >>> > >
> >>> >
> >>> >
> >>> >
> >>> > --
> >>> > *Mike Tutkowski*
> >>> > *Senior CloudStack Developer, SolidFire Inc.*
> >>> > e: mike.tutkow...@solidfire.com
> >>> > o: 303.746.7302
> >>> > Advancing the way the world uses the
> >>> > cloud<http://solidfire.com/solution/overview/?video=play>
> >>> > *™*
> >>> >
> >>>
> >>
> >>
> >>
> >> --
> >> *Mike Tutkowski*
> >> *Senior CloudStack Developer, SolidFire Inc.*
> >> e: mike.tutkow...@solidfire.com
> >> o: 303.746.7302
> >> Advancing the way the world uses the cloud<
> http://solidfire.com/solution/overview/?video=play>
> >> *™*
> >>
> >
> >
> >
> > --
> > *Mike Tutkowski*
> > *Senior CloudStack Developer, SolidFire Inc.*
> > e: mike.tutkow...@solidfire.com
> > o: 303.746.7302
> > Advancing the way the world uses the
> > cloud<http://solidfire.com/solution/overview/?video=play>
> > *™*
>
--
*Mike Tutkowski*
*Senior CloudStack Developer, SolidFire Inc.*
e: mike.tutkow...@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*™*
