Re: [Kimchi-devel] [Patch v2 3/4] Use one weksockify instance as all vms' vnc proxy.

On 12/27/2013 12:11 AM, Aline Manera wrote: > > Just a small comment below > > On 12/24/2013 12:41 AM, Mark Wu wrote: >> 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 > > 2 lines I don't understand why we need two blank lines for it. What's the rule are you following here?

> >> +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() { >