-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12500/#review23200
-----------------------------------------------------------
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.
- Ryan Lei
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
>
>
