You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Hongtu Zang <ho...@tcloudcomputing.com> on 2013/07/12 11:58:59 UTC

Review Request 12500: fix CLOUDSTACK-3495 xenserver 6.1 and 6.2 can not open vnc console

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12500/
-----------------------------------------------------------

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


Re: Review Request 12500: fix CLOUDSTACK-3495 xenserver 6.1 and 6.2 can not open vnc console

Posted by Ryan Lei <ry...@gmail.com>.

> 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
> 
>


Re: Review Request 12500: fix CLOUDSTACK-3495 xenserver 6.1 and 6.2 can not open vnc console

Posted by Ryan Lei <ry...@gmail.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 12500: fix CLOUDSTACK-3495 xenserver 6.1 and 6.2 can not open vnc console

Posted by Abhinandan Prateek <ap...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12500/#review23077
-----------------------------------------------------------

Ship it!


Ship It!

- Abhinandan Prateek


On July 12, 2013, 9:58 a.m., Hongtu Zang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12500/
> -----------------------------------------------------------
> 
> (Updated July 12, 2013, 9:58 a.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
> 
>


Re: Review Request 12500: fix CLOUDSTACK-3495 xenserver 6.1 and 6.2 can not open vnc console

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12500/#review24958
-----------------------------------------------------------


Commit 4333209af36ca001ca88935ddf2b4d981cc0150e in branch refs/heads/master from anthony
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=4333209 ]

CLOUDSTACK-3495
        CS used to access vnc server in xenserver dom0 to get VM console, now CS moves to use XenServer console API. getvncport plugin is not needed any more.
        remove the code related to getvncport in XenServer


- ASF Subversion and Git Services


On July 12, 2013, 9:58 a.m., Hongtu Zang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12500/
> -----------------------------------------------------------
> 
> (Updated July 12, 2013, 9:58 a.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
> 
>


Re: Review Request 12500: fix CLOUDSTACK-3495 xenserver 6.1 and 6.2 can not open vnc console

Posted by ASF Subversion and Git Services <as...@urd.zones.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12500/#review24957
-----------------------------------------------------------


Commit f39937223310cffde9308b16f26157f2e883d018 in branch refs/heads/4.2 from anthony
[ https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;h=f399372 ]

CLOUDSTACK-3495
        CS used to access vnc server in xenserver dom0 to get VM console, now CS moves to use XenServer console API. getvncport plugin is not needed any more.
        remove the code related to getvncport in XenServer


- ASF Subversion and Git Services


On July 12, 2013, 9:58 a.m., Hongtu Zang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12500/
> -----------------------------------------------------------
> 
> (Updated July 12, 2013, 9:58 a.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
> 
>