On 12/24/2013 12:41 AM, Mark Wu wrote: It seems this patch wasn't made based on master branch. Or I am missing something. You can use platform.machine() to get arch info 'x86_64'

Currently, we start a new weksockify proxy on every vm's connect action. It causes trouble to allow the vnc traffic in firewall because we choose a random port for each vm. And it's also obversed that vnc connection fails because of race between client and proxy server. This patch changes to use websockify's target_cfg option instead of target_port. Then kimchi can dynamically add proxy configuration for a vm. It adds a map of target host/port and token pair to websockify's cfg on the connect request. With this change, all vnc traffic can be forwared in one tcp port, and therefore it's possible to add firewall rule for kimchi's vnc remote access. Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt; --- Changes: change to add token on connect rather than vm's start to work with vms started by external tools, like virt-manager (Per Royce) src/kimchi/model.py | 14 ++++++-------- src/kimchi/server.py | 5 +++++ src/kimchi/vnc.py | 42 +++++++++++++++++++++++------------------- ui/js/src/kimchi.api.js | 6 ++++-- 4 files changed, 38 insertions(+), 29 deletions(-) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index 1a547ec..ba7e53c 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -128,7 +128,6 @@ class Model(object): self.libvirt_uri = libvirt_uri or 'qemu:///system' self.conn = LibvirtConnection(self.libvirt_uri) self.objstore = ObjectStore(objstore_loc) - self.graphics_ports = {} self.next_taskid = 1 self.stats = {} self.host_stats = defaultdict(int) @@ -503,10 +502,7 @@ class Model(object): info = dom.info() state = Model.dom_state_map[info[0]] screenshot = None - graphics_type, _ = self._vm_get_graphics(name) - # 'port' must remain None until a connect call is issued - graphics_port = (self.graphics_ports.get(name, None) if state == 'running' - else None) + graphics_type, graphics_port = self._vm_get_graphics(name) try: if state == 'running': screenshot = self.vmscreenshot_lookup(name) @@ -566,6 +562,8 @@ class Model(object): with self.objstore as session: session.delete('vm', dom.UUIDString(), ignore_missing=True) + vnc.remove_proxy_token(name) + def vm_start(self, name): dom = self._get_vm(name) dom.create() @@ -595,10 +593,10 @@ class Model(object): def vm_connect(self, name): graphics, port = self._vm_get_graphics(name) if graphics == "vnc" and port != None: - port = vnc.new_ws_proxy(port) - self.graphics_ports[name] = port + vnc.add_proxy_token(name, port) else: - raise OperationFailed("Unable to find VNC port in %s" % name) + raise OperationFailed("Only able to connect to running vm's vnc " + "graphics.") def vms_create(self, params): conn = self.conn.get() diff --git a/src/kimchi/server.py b/src/kimchi/server.py index 6ff6fa0..9afe2ec 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -32,6 +32,7 @@ from kimchi import auth from kimchi import config from kimchi import model from kimchi import mockmodel +from kimchi import vnc from kimchi.root import Root from kimchi.utils import import_class, get_enabled_plugins @@ -188,6 +189,10 @@ class Server(object): else: model_instance = model.Model() + if isinstance(model_instance, model.Model): + vnc_ws_proxy = vnc.new_ws_proxy() + cherrypy.engine.subscribe('exit', vnc_ws_proxy.kill) + self.app = cherrypy.tree.mount(Root(model_instance, dev_env), config=self.configObj) self._load_plugins() diff --git a/src/kimchi/vnc.py b/src/kimchi/vnc.py index 089d64a..e476002 100644 --- a/src/kimchi/vnc.py +++ b/src/kimchi/vnc.py @@ -21,33 +21,37 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import errno import os -import socket import subprocess -import sys +from kimchi.config import config -from contextlib import closing +WS_TOKENS_DIR = '/var/lib/kimchi/vnc-tokens' -from kimchi.exception import OperationFailed +def new_ws_proxy(): + try: + os.makedirs(WS_TOKENS_DIR, mode=0755) + except OSError as e: + if e.errno == errno.EEXIST: + pass -def _getFreePort(): - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - with closing(sock): - try: - sock.bind(("0.0.0.0", 0)) - except: - raise OperationFailed("Could not find a free port") - - return sock.getsockname()[1] - -def new_ws_proxy(target_port): - src_port = _getFreePort() cmd = os.path.join(os.path.dirname(__file__), 'websockify.py') - args = ['python', cmd, str(src_port), '--timeout', '10', - '--idle-timeout', '10', 'localhost:%s' % target_port] + args = ['python', cmd, config.get('server', 'vnc_proxy_port'), + '--target-config', WS_TOKENS_DIR] p = subprocess.Popen(args, close_fds=True) + return p + + +def add_proxy_token(name, port): + with open(os.path.join(WS_TOKENS_DIR, name), 'w') as f: + f.write('%s: localhost:%s' % (name.encode('utf-8'), port)) + - return src_port +def remove_proxy_token(name): + try: + os.unlink(os.path.join(WS_TOKENS_DIR, name)) + except OSError: + pass diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js index fbcf4a2..a4608ff 100644 --- a/ui/js/src/kimchi.api.js +++ b/ui/js/src/kimchi.api.js @@ -291,17 +291,19 @@ var kimchi = { dataType : 'json' }).done(function(data, textStatus, xhr) { http_port = data['http_port']; + proxy_port = data['vnc_proxy_port']; kimchi.requestJSON({ url : "/vms/" + encodeURIComponent(vm) + "/connect", type : "POST", dataType : "json" - }).done(function(data, textStatus, xhr) { + }).done(function() { /** * Due to problems with web sockets and self-signed * certificates, for now we will always redirect to http */ url = 'http://' + location.hostname + ':' + http_port; - url += "/vnc_auto.html?port=" + data.graphics.port; + url += "/vnc_auto.html?port=" + proxy_port; + url += "&path=?token=" + encodeURIComponent(vm); window.open(url); }); }).error(function() { -- 1.8.3.1

On 12/27/2013 12:11 AM, Aline Manera wrote: I don't understand why we need two blank lines for it. What's the rule are you following here?

On Tue, 2013-12-24 at 10:41 +0800, Mark Wu wrote: Sorry if I'm missing something obvious here, or someone has already pointed this out, but the subject and the commit message both say 'weksockify," which I think should be 'websockify.' Regards, Christy

Reviewed-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; On 12/24/2013 12:41 AM, Mark Wu wrote:

On 12/24/2013 01:09 PM, Zhou Zheng Sheng wrote: Yes, you're correct. I felt weird when I wrote the commit message. Thanks for the good comment.

On 12/26/2013 11:53 PM, Aline Manera wrote: Ack. I will resolve it in next in v3. thanks!