> On July 16, 2013, 2:06 p.m., Ryan Lei wrote: > > Hi, my first comment at the review board. :) > > Looking at the diff, I think it's better to change the if else statements > > so that 6.x is the default behavior. Like this: > > > > if version[:3] == '5.6': > > path1 = "/local/domain/" + domid + "/serial/0/vncterm-pid" > > path2 = "/local/domain/" + domid + "/serial/0/vnc-port" > > else: > > path1 = "/local/domain/" + domid + "/vncterm-pid" > > path2 = "/local/domain/" + domid + "/console/vnc-port" > > > > If the future XenServer versions STILL have the same VNC paths, we wouldn't > > have to add "or version[:3] == '6.3'" whenever a new version comes out. > > > > My two concerns though: > > 1. Not sure if we can simply use version[:3] to handle the string in > > XenServer 5.x. Maybe there was a string that didn't start with "5.6"? > > 2. Not sure if this change will break the compatibility with XCP. I didn't > > investigate how XCP versions are handled. > >
BTW, in CloudStack 4.1.0 + XenServer 6.1, VM console in web UI works fine. But the code in vmops only says "if version[:3] == '6.0'" Then why does the problem occur only in CloudStack 4.2? - Ryan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12500/#review23200 ----------------------------------------------------------- On July 12, 2013, 5:58 p.m., Hongtu Zang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12500/ > ----------------------------------------------------------- > > (Updated July 12, 2013, 5:58 p.m.) > > > Review request for cloudstack, Abhinandan Prateek and Devdeep Singh. > > > Bugs: CLOUDSTACK-3495 > > > Repository: cloudstack-git > > > Description > ------- > > change vmops getvncport function > xenserver 6.1 and 6.2 will find the vnc-port in the same path with 6.0 > > > Diffs > ----- > > scripts/vm/hypervisor/xenserver/vmops 650e955 > > Diff: https://reviews.apache.org/r/12500/diff/ > > > Testing > ------- > > can open vnc console in web UI > > > Thanks, > > Hongtu Zang > >
