We are going to use one proxy instance to forward all vm's vnc traffics. The proxy instance listens on a fixed port instead of a random one. This patch makes the port configurable and exported to client by adding it to the response of '/config'. Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt; --- docs/API.md | 1 + src/kimchi.conf.in | 4 ++++ src/kimchi/config.py.in | 2 ++ src/kimchi/control/config.py | 4 +++- 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/API.md b/docs/API.md index 0013e86..4d7bdff 100644 --- a/docs/API.md +++ b/docs/API.md @@ -432,6 +432,7 @@ Contains information about the application environment and configuration. * **GET**: Retrieve configuration information * http_port: The port number on which the server is listening + * vnc_proxy_port: Port for vnc's websocket proxy to listen on * **POST**: *See Configuration Actions* **Actions (POST):** diff --git a/src/kimchi.conf.in b/src/kimchi.conf.in index bf26c26..ac5a1f5 100644 --- a/src/kimchi.conf.in +++ b/src/kimchi.conf.in @@ -29,3 +29,7 @@ # Logging level: debug, info, warning, error or critical #log_level = debug + +[novnc] +# Port for vnc's websocket proxy to listen on +#vnc_proxy_port = 64667 diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index 72308ff..e1319e3 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -179,6 +179,8 @@ def _get_config(): config.add_section("logging") config.set("logging", "log_dir", get_default_log_dir()) config.set("logging", "log_level", DEFAULT_LOG_LEVEL) + config.add_section("novnc") + config.set("novnc", "vnc_proxy_port", "64667") if os.path.exists(CONFIG_FILE): config.read(CONFIG_FILE) diff --git a/src/kimchi/control/config.py b/src/kimchi/control/config.py index 3630172..5186ddd 100644 --- a/src/kimchi/control/config.py +++ b/src/kimchi/control/config.py @@ -25,6 +25,7 @@ import cherrypy +from kimchi.config import config from kimchi.control.base import Collection, Resource @@ -38,7 +39,8 @@ class Config(Resource): @property def data(self): - return {'http_port': cherrypy.server.socket_port} + return {'http_port': cherrypy.server.socket_port, + 'vnc_proxy_port': config.get('novnc', 'vnc_proxy_port')} class Capabilities(Resource):

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; --- src/kimchi/model.py | 14 ++++++-------- src/kimchi/server.py | 5 +++++ src/kimchi/vnc.py | 41 +++++++++++++++++++++++------------------ ui/js/src/kimchi.api.js | 6 ++++-- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/src/kimchi/model.py b/src/kimchi/model.py index a21fcf7..10e6c3f 100644 --- a/src/kimchi/model.py +++ b/src/kimchi/model.py @@ -129,7 +129,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) @@ -504,10 +503,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) @@ -567,6 +563,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() @@ -596,10 +594,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 1467c87..b820263 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 get_enabled_plugins, import_class @@ -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..dc70b46 100644 --- a/src/kimchi/vnc.py +++ b/src/kimchi/vnc.py @@ -21,33 +21,38 @@ # 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 contextlib import closing +from kimchi.config import config -from kimchi.exception import OperationFailed +WS_TOKENS_DIR = '/var/lib/kimchi/vnc-tokens' -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") +def new_ws_proxy(): + try: + os.makedirs(WS_TOKENS_DIR, mode=0755) + except OSError as e: + if e.errno == errno.EEXIST: + pass - 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('novnc', '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() {

The configuration is also needed for other code except starting kimchi server. So it should be moved to a separate module, config.py. Then the configuration can be accessed directly by importing config module. Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt; --- src/kimchi/config.py.in | 24 ++++++++++++++++++++++++ src/kimchid.in | 20 +------------------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index f3c408a..72308ff 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -27,6 +27,7 @@ import os import platform +from ConfigParser import SafeConfigParser from glob import iglob @@ -166,5 +167,28 @@ def get_plugin_tab_xml(name): return os.path.join(_get_plugin_ui_dir(name), 'config/tab-ext.xml') +def _get_config(): + config = SafeConfigParser() + config.add_section("server") + config.set("server", "host", "0.0.0.0") + config.set("server", "port", "8000") + config.set("server", "ssl_port", "8001") + config.set("server", "ssl_cert", "") + config.set("server", "ssl_key", "") + config.set("server", "environment", "development") + config.add_section("logging") + config.set("logging", "log_dir", get_default_log_dir()) + config.set("logging", "log_level", DEFAULT_LOG_LEVEL) + + if os.path.exists(CONFIG_FILE): + config.read(CONFIG_FILE) + return config +

+ +CONFIG_FILE = "%s/kimchi.conf" % get_config_dir() +DEFAULT_LOG_LEVEL = "debug" +config = _get_config() +

+ if __name__ == '__main__': print get_prefix() diff --git a/src/kimchid.in b/src/kimchid.in index 7865713..548fa52 100644 --- a/src/kimchid.in +++ b/src/kimchid.in @@ -33,31 +33,13 @@ import kimchi.config if kimchi.config.without_installation(): sys.path.append(kimchi.config.get_prefix()) -from ConfigParser import SafeConfigParser +from kimchi.config import config from optparse import OptionParser ACCESS_LOG = "kimchi-access.log" ERROR_LOG = "kimchi-error.log" -CONFIG_FILE = "%s/kimchi.conf" % kimchi.config.get_config_dir() -DEFAULT_LOG_DIR = kimchi.config.get_default_log_dir() -DEFAULT_LOG_LEVEL = "debug" def main(options): - config = SafeConfigParser() - config.add_section("server") - config.set("server", "host", "0.0.0.0") - config.set("server", "port", "8000") - config.set("server", "ssl_port", "8001") - config.set("server", "ssl_cert", "") - config.set("server", "ssl_key", "") - config.set("server", "environment", "development") - config.add_section("logging") - config.set("logging", "log_dir", DEFAULT_LOG_DIR) - config.set("logging", "log_level", DEFAULT_LOG_LEVEL) - - if os.path.exists(CONFIG_FILE): - config.read(CONFIG_FILE) - host = config.get("server", "host") port = config.get("server", "port") ssl_port = config.get("server", "ssl_port")

The configuration is also needed for other code except starting kimchi server. So it should be moved to a separate module, config.py. Then the configuration can be accessed directly by importing config module. Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt; --- src/kimchi/config.py.in | 24 ++++++++++++++++++++++++ src/kimchid.in | 20 +------------------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in index f3c408a..72308ff 100644 --- a/src/kimchi/config.py.in +++ b/src/kimchi/config.py.in @@ -27,6 +27,7 @@ import os import platform +from ConfigParser import SafeConfigParser from glob import iglob @@ -166,5 +167,28 @@ def get_plugin_tab_xml(name): return os.path.join(_get_plugin_ui_dir(name), 'config/tab-ext.xml') +def _get_config(): + config = SafeConfigParser() + config.add_section("server") + config.set("server", "host", "0.0.0.0") + config.set("server", "port", "8000") + config.set("server", "ssl_port", "8001") + config.set("server", "ssl_cert", "") + config.set("server", "ssl_key", "") + config.set("server", "environment", "development") + config.add_section("logging") + config.set("logging", "log_dir", get_default_log_dir()) + config.set("logging", "log_level", DEFAULT_LOG_LEVEL) + + if os.path.exists(CONFIG_FILE): + config.read(CONFIG_FILE) + return config + + +CONFIG_FILE = "%s/kimchi.conf" % get_config_dir() +DEFAULT_LOG_LEVEL = "debug" +config = _get_config() + + if __name__ == '__main__': print get_prefix() diff --git a/src/kimchid.in b/src/kimchid.in index 7865713..548fa52 100644 --- a/src/kimchid.in +++ b/src/kimchid.in @@ -33,31 +33,13 @@ import kimchi.config if kimchi.config.without_installation(): sys.path.append(kimchi.config.get_prefix()) -from ConfigParser import SafeConfigParser +from kimchi.config import config from optparse import OptionParser ACCESS_LOG = "kimchi-access.log" ERROR_LOG = "kimchi-error.log" -CONFIG_FILE = "%s/kimchi.conf" % kimchi.config.get_config_dir() -DEFAULT_LOG_DIR = kimchi.config.get_default_log_dir() -DEFAULT_LOG_LEVEL = "debug" def main(options): - config = SafeConfigParser() - config.add_section("server") - config.set("server", "host", "0.0.0.0") - config.set("server", "port", "8000") - config.set("server", "ssl_port", "8001") - config.set("server", "ssl_cert", "") - config.set("server", "ssl_key", "") - config.set("server", "environment", "development") - config.add_section("logging") - config.set("logging", "log_dir", DEFAULT_LOG_DIR) - config.set("logging", "log_level", DEFAULT_LOG_LEVEL) - - if os.path.exists(CONFIG_FILE): - config.read(CONFIG_FILE) -

host = config.get("server", "host") port = config.get("server", "port") ssl_port = config.get("server", "ssl_port")

On 2014年01月08日 10:55, Royce Lv wrote: > On 2014年01月07日 16:56, Mark Wu wrote: >> The configuration is also needed for other code except starting kimchi >> server. So it should be moved to a separate module, config.py. Then the >> configuration can be accessed directly by importing config module. >> >> Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt; >> --- >> src/kimchi/config.py.in | 24 ++++++++++++++++++++++++ >> src/kimchid.in | 20 +------------------- >> 2 files changed, 25 insertions(+), 19 deletions(-) >> >> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in >> index f3c408a..72308ff 100644 >> --- a/src/kimchi/config.py.in >> +++ b/src/kimchi/config.py.in >> @@ -27,6 +27,7 @@ import os >> import platform >> >> >> +from ConfigParser import SafeConfigParser >> from glob import iglob >> >> >> @@ -166,5 +167,28 @@ def get_plugin_tab_xml(name): >> return os.path.join(_get_plugin_ui_dir(name), >> 'config/tab-ext.xml') >> >> >> +def _get_config(): >> + config = SafeConfigParser() >> + config.add_section("server") >> + config.set("server", "host", "0.0.0.0") >> + config.set("server", "port", "8000") >> + config.set("server", "ssl_port", "8001") >> + config.set("server", "ssl_cert", "") >> + config.set("server", "ssl_key", "") >> + config.set("server", "environment", "development") >> + config.add_section("logging") >> + config.set("logging", "log_dir", get_default_log_dir()) >> + config.set("logging", "log_level", DEFAULT_LOG_LEVEL) >> + >> + if os.path.exists(CONFIG_FILE): >> + config.read(CONFIG_FILE) >> + return config >> + >> + >> +CONFIG_FILE = "%s/kimchi.conf" % get_config_dir() >> +DEFAULT_LOG_LEVEL = "debug" >> +config = _get_config() >> + >> + >> if __name__ == '__main__': >> print get_prefix() >> diff --git a/src/kimchid.in b/src/kimchid.in >> index 7865713..548fa52 100644 >> --- a/src/kimchid.in >> +++ b/src/kimchid.in >> @@ -33,31 +33,13 @@ import kimchi.config >> if kimchi.config.without_installation(): >> sys.path.append(kimchi.config.get_prefix()) >> >> -from ConfigParser import SafeConfigParser >> +from kimchi.config import config >> from optparse import OptionParser >> >> ACCESS_LOG = "kimchi-access.log" >> ERROR_LOG = "kimchi-error.log" >> -CONFIG_FILE = "%s/kimchi.conf" % kimchi.config.get_config_dir() >> -DEFAULT_LOG_DIR = kimchi.config.get_default_log_dir() >> -DEFAULT_LOG_LEVEL = "debug" >> >> def main(options): >> - config = SafeConfigParser() >> - config.add_section("server") >> - config.set("server", "host", "0.0.0.0") >> - config.set("server", "port", "8000") >> - config.set("server", "ssl_port", "8001") >> - config.set("server", "ssl_cert", "") >> - config.set("server", "ssl_key", "") >> - config.set("server", "environment", "development") >> - config.add_section("logging") >> - config.set("logging", "log_dir", DEFAULT_LOG_DIR) >> - config.set("logging", "log_level", DEFAULT_LOG_LEVEL) >> - >> - if os.path.exists(CONFIG_FILE): >> - config.read(CONFIG_FILE) >> - > Starting here: >> host = config.get("server", "host") >> port = config.get("server", "port") >> ssl_port = config.get("server", "ssl_port") And don't forget to update config object if you want to use it else where, because the right configuration is processed by cmd args.In your patch it won't cause problem because you don't accepted cmd args of vnc port, but if in the future if we want to use config object, that'll cause problem.

> This may be a further refactor , but seems we can use set_defaults of > optionparser to set config dict as default value of option parser, so > that the annoying dirty lines of reading every item of config can be > removed. > REF: > http://docs.python.org/3.3/library/optparse.html#optparse.OptionParser.se... > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 2014年01月07日 16:56, Mark Wu wrote: > The configuration is also needed for other code except starting kimchi > server. So it should be moved to a separate module, config.py. Then the > configuration can be accessed directly by importing config module. > > Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt; > --- > src/kimchi/config.py.in | 24 ++++++++++++++++++++++++ > src/kimchid.in | 20 +------------------- > 2 files changed, 25 insertions(+), 19 deletions(-) > > diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in > index f3c408a..72308ff 100644 > --- a/src/kimchi/config.py.in > +++ b/src/kimchi/config.py.in > @@ -27,6 +27,7 @@ import os > import platform > > > +from ConfigParser import SafeConfigParser > from glob import iglob > > > @@ -166,5 +167,28 @@ def get_plugin_tab_xml(name): > return os.path.join(_get_plugin_ui_dir(name), > 'config/tab-ext.xml') > > > +def _get_config(): > + config = SafeConfigParser() > + config.add_section("server") > + config.set("server", "host", "0.0.0.0") > + config.set("server", "port", "8000") > + config.set("server", "ssl_port", "8001") > + config.set("server", "ssl_cert", "") > + config.set("server", "ssl_key", "") > + config.set("server", "environment", "development") > + config.add_section("logging") > + config.set("logging", "log_dir", get_default_log_dir()) > + config.set("logging", "log_level", DEFAULT_LOG_LEVEL) > + > + if os.path.exists(CONFIG_FILE): > + config.read(CONFIG_FILE) > + return config > + > + > +CONFIG_FILE = "%s/kimchi.conf" % get_config_dir() > +DEFAULT_LOG_LEVEL = "debug" > +config = _get_config() > + > + > if __name__ == '__main__': > print get_prefix() > diff --git a/src/kimchid.in b/src/kimchid.in > index 7865713..548fa52 100644 > --- a/src/kimchid.in > +++ b/src/kimchid.in > @@ -33,31 +33,13 @@ import kimchi.config > if kimchi.config.without_installation(): > sys.path.append(kimchi.config.get_prefix()) > > -from ConfigParser import SafeConfigParser > +from kimchi.config import config > from optparse import OptionParser > > ACCESS_LOG = "kimchi-access.log" > ERROR_LOG = "kimchi-error.log" > -CONFIG_FILE = "%s/kimchi.conf" % kimchi.config.get_config_dir() > -DEFAULT_LOG_DIR = kimchi.config.get_default_log_dir() > -DEFAULT_LOG_LEVEL = "debug" > > def main(options): > - config = SafeConfigParser() > - config.add_section("server") > - config.set("server", "host", "0.0.0.0") > - config.set("server", "port", "8000") > - config.set("server", "ssl_port", "8001") > - config.set("server", "ssl_cert", "") > - config.set("server", "ssl_key", "") > - config.set("server", "environment", "development") > - config.add_section("logging") > - config.set("logging", "log_dir", DEFAULT_LOG_DIR) > - config.set("logging", "log_level", DEFAULT_LOG_LEVEL) > - > - if os.path.exists(CONFIG_FILE): > - config.read(CONFIG_FILE) > - Starting here: > host = config.get("server", "host") > port = config.get("server", "port") > ssl_port = config.get("server", "ssl_port") This may be a further refactor , but seems we can use set_defaults of optionparser to set config dict as default value of option parser, so that the annoying dirty lines of reading every item of config can be removed. REF: http://docs.python.org/3.3/library/optparse.html#optparse.OptionParser.se...