Koushik,

please review https://reviews.apache.org/r/12849/ It should solve the
problem.

regards,
Daan


On Mon, Jul 22, 2013 at 8:40 PM, Daan Hoogland <daan.hoogl...@gmail.com>wrote:

> Ignore my remark about master. I have been looking
> At an old log.
> I looked some more into this and it seems the even simpler
>                 if (scheme == Vlan.scheme) {
>
>                     return new URI(scheme + "://" + value);
>                 }
> would do the trick. I have not been able to reproduce the problem reliably
> yet.
>
> Daan,
>
> After manually updating the database entries to 'vlan://', the system VMs
> started working. So I don't think using a database before the commit is a
> problem. One piece of code that seems broken is
> CitrixResourceBase.getNetwork().
>
> Refer to the line of code
>
> long vlan = Long.parseLong(broadcastUri.getHost());
>
> This fails with a NumberFormatException for vlan:<vlanid>.
>
> -Koushik
>
> On 22-Jul-2013, at 9:38 PM, Daan Hoogland <daan.hoogl...@gmail.com> wrote:
>
> > Koushik,
> >
> > thanks for reporting this. You are probably using a database form before
> > this commit. This should work but I suspect it doesn't.
> >        public <T> URI toUri(T value) {
> >            try {
> >                // do we need to check that value does not contain a
> scheme
> >                // part?
> >                try {
> >                    Long.parseLong(value.toString());
> >                    return new URI(Vlan.scheme + "://" + value);
> >                } catch (NumberFormatException e) {
> >                    if (value.toString().contains(":"))
> >                        return new URI(value.toString());
> >                    else
> >                        return new URI(scheme, value.toString(), null);
> >                }
> >            } catch (URISyntaxException e) {
> >                throw new CloudRuntimeException(
> >                        "Unable to convert to broadcast URI: " + value);
> >            }
> >        }
> >
> > would be the correct code for the function that is your culprit, but
> master
> > doesn't work due to database mismatches between the code and the databse
> > schema. I will test this as soon as possible.
> >
> > regards,
> > Daan
> >
> >
> > On Mon, Jul 22, 2013 at 5:18 PM, Koushik Das <koushik....@citrix.com>
> wrote:
> >
> >> Standard system VMs with systemvm.iso from latest master. But that
> >> shouldn't matter as the issue is in the deploy VM code.
> >>
> >> ________________________________
> >> From: Donal Lafferty
> >> Sent: 22/07/2013 6:37 PM
> >> To: 'dev@cloudstack.apache.org'; cloudstack-...@incubator.apache.org
> >> Subject: RE: System VMs not coming up due to
> >> https://reviews.apache.org/r/12685/
> >>
> >> Where are you getting your system VM from?
> >>
> >> DL
> >>
> >>> -----Original Message-----
> >>> From: Koushik Das [mailto:koushik....@citrix.com]
> >>> Sent: 22 July 2013 11:09 AM
> >>> To: cloudstack-...@incubator.apache.org
> >>> Subject: System VMs not coming up due to
> >>> https://reviews.apache.org/r/12685/
> >>>
> >>> Commit id: 2d4464d2badc9aff842fd180bafc4c384a83a91d
> >>>
> >>> -                return new URI(scheme + "://" + value);
> >>> +                // do we need to check that value does not contain a
> >> scheme
> >>> +                // part?
> >>> +                if (value.toString().contains(":"))
> >>> +                    return new URI(value.toString());
> >>> +                else
> >>> +                    return new URI(scheme, value.toString(), null);
> >>>
> >>>
> >>> As part of this commit system VMs are not working in latest master. I
> >> see as
> >>> part of this change the broadcast/isolation URI format has changed from
> >>> vlan://<vlanid> to vlan:<vlanid>. The code in many places still depends
> >> on
> >>> the URI.getHosts() to extract the vlan id which is broken due to the
> new
> >>> format.
> >>> Can this be reverted?
> >>>
> >>>
> >>>
> >>
> >>
>
>

Reply via email to