[PATCH 0/4] Use one weksockify instance as all vms' vnc proxy.

[Mark Wu]

It resolves two problems:
A fixed port for firewall rule.
Race between client and server. It happens on every first connection after
the host running kimchi reboot.

Mark Wu (4):
Move configuration parsing to config.py
Add a configuration for vnc websocket proxy
Use one weksockify instance as all vms' vnc proxy.
Remove vnc related code in mockmodel

docs/API.md | 1 +
src/kimchi.conf.in | 3 +++
src/kimchi/config.py.in | 24 +++++++++++++++++++++---
src/kimchi/controller.py | 5 ++++-
src/kimchi/mockmodel.py | 19 -------------------
src/kimchi/model.py | 18 +++++-------------
src/kimchi/server.py | 5 +++++
src/kimchi/vnc.py | 42 +++++++++++++++++++++++-------------------
src/kimchid.in | 20 +-------------------
ui/js/src/kimchi.api.js | 14 +++++---------
10 files changed, 68 insertions(+), 83 deletions(-)

--
1.8.3.1

[Mark Wu]

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 <wu...@linux.vnet.ibm.com>
---

docs/API.md | 1 +
src/kimchi.conf.in | 3 +++

src/kimchi/config.py.in | 1 +
src/kimchi/controller.py | 5 ++++-
4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/API.md b/docs/API.md
index 74bc1b5..b0bcefb 100644
--- a/docs/API.md
+++ b/docs/API.md
@@ -413,6 +413,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..4ce80ca 100644
--- a/src/kimchi.conf.in
+++ b/src/kimchi.conf.in
@@ -23,6 +23,9 @@
# Running environment of the server
#environment = development

+# Port for vnc's websocket proxy to listen on
+#vnc_proxy_port = 64667
+
[logging]
# Log directory
#log_dir = @localstatedir@/log/kimchi
diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
index 49d42db..e2b8423 100644
--- a/src/kimchi/config.py.in
+++ b/src/kimchi/config.py.in
@@ -174,6 +174,7 @@ config.set("server", "port", "8000")
config.set("server", "ssl_port", "8001")
config.set("server", "ssl_cert", "")
config.set("server", "ssl_key", "")
+config.set("server", "vnc_proxy_port", "64667")
config.set("server", "environment", "development")
config.add_section("logging")
config.set("logging", "log_dir", get_default_log_dir())
diff --git a/src/kimchi/controller.py b/src/kimchi/controller.py
index 3b27c27..695d58a 100644
--- a/src/kimchi/controller.py
+++ b/src/kimchi/controller.py
@@ -31,6 +31,7 @@ from jsonschema import Draft3Validator, ValidationError

import kimchi.template
from kimchi import auth
+from kimchi.config import config
from kimchi.exception import InvalidOperation, InvalidParameter, MissingParameter
from kimchi.exception import NotFoundError, OperationFailed
from kimchi.model import ISO_POOL_NAME
@@ -651,7 +652,9 @@ 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('server', 'vnc_proxy_port')}
+

class Capabilities(Resource):
def __init__(self, model, id=None):
--
1.8.3.1

[Mark Wu]

The configruation 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 <wu...@linux.vnet.ibm.com>
---

src/kimchi/config.py.in | 23 ++++++++++++++++++++---
src/kimchid.in | 20 +-------------------
2 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
index f3c408a..49d42db 100644
--- a/src/kimchi/config.py.in
+++ b/src/kimchi/config.py.in
@@ -25,11 +25,9 @@
import libvirt
import os
import platform
-
-
+from ConfigParser import SafeConfigParser
from glob import iglob

-
from kimchi.xmlutils import xpath_get_text


@@ -166,5 +164,24 @@ def get_plugin_tab_xml(name):
return os.path.join(_get_plugin_ui_dir(name), 'config/tab-ext.xml')


+CONFIG_FILE = "%s/kimchi.conf" % get_config_dir()
+DEFAULT_LOG_LEVEL = "debug"
+
+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)
+
+
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")
--
1.8.3.1

[Mark Wu]

We don't need to start a vnc server in mock model because vm's
connect action is removed from REST api.

Signed-off-by: Mark Wu <wu...@linux.vnet.ibm.com>
---

src/kimchi/mockmodel.py | 19 -------------------
1 file changed, 19 deletions(-)

diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
index 4839d4c..4f14024 100644
--- a/src/kimchi/mockmodel.py
+++ b/src/kimchi/mockmodel.py
@@ -44,7 +44,6 @@ except ImportError:
import kimchi.model
from kimchi import config
from kimchi import network as knetwork
-from kimchi import vnc
from kimchi.asynctask import AsyncTask
from kimchi.distroloader import DistroLoader
from kimchi.exception import InvalidOperation, InvalidParameter
@@ -59,20 +58,8 @@ class MockModel(object):
def __init__(self, objstore_loc=None):
self.reset()
self.objstore = ObjectStore(objstore_loc)
- self.vnc_port = 5999
self.distros = self._get_distros()

- # open vnc port
- # make it here to make sure it will be available on server startup
- cmd = config.find_qemu_binary()
- args = [cmd, "-vnc", ":99"]
-
- cmd = "ps aux | grep '%s' | grep -v grep" % " ".join(args)
- proc = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE)
-
- if len(proc.stdout.readlines()) == 0:
- p = subprocess.Popen(args, close_fds=True)
-
def get_capabilities(self):
return {'libvirt_stream_protocols': ['http', 'https', 'ftp', 'ftps', 'tftp'],
'qemu_stream': True,
@@ -85,7 +72,6 @@ class MockModel(object):
self._mock_templates = {}
self._mock_storagepools = {'default': MockStoragePool('default')}
self._mock_networks = {'default': MockNetwork('default')}
- self._mock_graphics_ports = {}
self._mock_interfaces = self.dummy_interfaces()
self.next_taskid = 1
self.storagepool_activate('default')
@@ -121,7 +107,6 @@ class MockModel(object):
vm.info['screenshot'] = self.vmscreenshot_lookup(name)
else:
vm.info['screenshot'] = None
- vm.info['graphics']['port'] = self._mock_graphics_ports.get(name, None)
return vm.info

def vm_delete(self, name):
@@ -139,10 +124,6 @@ class MockModel(object):
def vm_stop(self, name):
self._get_vm(name).info['state'] = 'shutoff'

- def vm_connect(self, name):
- vnc_port = kimchi.vnc.new_ws_proxy(self.vnc_port)
- self._mock_graphics_ports[name] = vnc_port
-
def vms_create(self, params):
t_name = kimchi.model.template_name_from_uri(params['template'])
name = kimchi.model.get_vm_name(params.get('name'), t_name,
--
1.8.3.1

[Mark Wu]

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. Basically, it gets the target
host/port by searching the token passed by the client on the connect request.
It adds token for vm when it's started and the token is kept until the vm is
stopped. After this change, we don't need do anything in kimchi because all things
are set up before connecting. So the vm's connect action is removed too.

Signed-off-by: Mark Wu <wu...@linux.vnet.ibm.com>
---

src/kimchi/model.py | 18 +++++-------------
src/kimchi/server.py | 5 +++++
src/kimchi/vnc.py | 42 +++++++++++++++++++++++-------------------

ui/js/src/kimchi.api.js | 14 +++++---------
4 files changed, 38 insertions(+), 41 deletions(-)

diff --git a/src/kimchi/model.py b/src/kimchi/model.py
index 73c18ac..952a216 100644
--- a/src/kimchi/model.py
+++ b/src/kimchi/model.py
@@ -125,7 +125,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)
@@ -500,10 +499,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,11 +562,15 @@ class Model(object):
def vm_start(self, name):
dom = self._get_vm(name)
dom.create()
+ graphics, port = self._vm_get_graphics(name)
+ if graphics == "vnc" and port != None:
+ vnc.add_proxy_token(name, port)

def vm_stop(self, name):
if self._vm_exists(name):
dom = self._get_vm(name)
dom.destroy()
+ vnc.remove_proxy_token(name)

def _vm_get_graphics(self, name):
dom = self._get_vm(name)
@@ -589,14 +589,6 @@ class Model(object):
graphics_type = None if graphics_type != "vnc" else graphics_type
return graphics_type, port

- 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
- else:
- raise OperationFailed("Unable to find VNC port in %s" % name)
-
def vms_create(self, params):
conn = self.conn.get()
t_name = template_name_from_uri(params['template'])
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..83fb82e 100644
--- a/ui/js/src/kimchi.api.js
+++ b/ui/js/src/kimchi.api.js
@@ -291,19 +291,15 @@ var kimchi = {
dataType : 'json'
}).done(function(data, textStatus, xhr) {
http_port = data['http_port'];
- kimchi.requestJSON({
- url : "/vms/" + encodeURIComponent(vm) + "/connect",
- type : "POST",
- dataType : "json"
- }).done(function(data, textStatus, xhr) {
+ proxy_port = data['vnc_proxy_port'];
/**
* 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;
- window.open(url);
- });
+ url = 'http://' + location.hostname + ':' + http_port;
+ url += "/vnc_auto.html?port=" + proxy_port;
+ url += "&path=?token=" + encodeURIComponent(vm);
+ window.open(url);
}).error(function() {
kimchi.message.error(i18n['msg.fail.get.config']);
});
--
1.8.3.1

[Mark Wu]

Aline,

I made this series of patches when I investigated the vnc connection failure.  But it turns out it only happened in a very minor change.  So I think we don't need include the fix in this release.  We could consider it after the window of next release is open.

Mark.

[Eli Qiao]

于 2013年12月18日 16:51, Mark Wu 写道:
> +# Port for vnc's websocket proxy to listen on
> +#vnc_proxy_port = 64667
> +
> [logging]

we need to add a iptable rule to open this port in spec file post section.

--
Thanks Eli (Li Yong) Qiao (qia...@cn.ibm.com)
CSTL-KVM Frobisher/RHEV-H

[Sheldon]

       

On 12/18/2013 05:00 PM, Mark Wu wrote:

Aline,

I made this series of patches when I investigated the vnc connection failure.  But it turns out it only happened in a very minor change.  So I think we don't need include the fix in this release.  We could consider it after the window of next release is open.

Mark.

sure, we do need this  patch. 
It is also helpful for Firewall configure.
Firewall configure is important for a product environment. 

And we has list this as a task of Firewall configure for a long time.
https://github.com/kimchi-project/kimchi/wiki/Firewall-configure

Also Eli has post a patch for Firewall configure.

On 12/18/2013 04:51 PM, Mark Wu wrote:

It resolves two problems:
   A fixed port for firewall rule.
   Race between client and server. It happens on every first connection after
   the host running kimchi reboot.

Mark Wu (4):
  Move configuration parsing to config.py
  Add a configuration for vnc websocket proxy
  Use one weksockify instance as all vms' vnc proxy.
  Remove vnc related code in mockmodel

 docs/API.md              |  1 +
 src/kimchi.conf.in       |  3 +++
 src/kimchi/config.py.in  | 24 +++++++++++++++++++++---
 src/kimchi/controller.py |  5 ++++-
 src/kimchi/mockmodel.py  | 19 -------------------
 src/kimchi/model.py      | 18 +++++-------------
 src/kimchi/server.py     |  5 +++++
 src/kimchi/vnc.py        | 42 +++++++++++++++++++++++-------------------
 src/kimchid.in           | 20 +-------------------
 ui/js/src/kimchi.api.js  | 14 +++++---------
 10 files changed, 68 insertions(+), 83 deletions(-)

--
project-kimchi mailing list <project...@googlegroups.com>
https://groups.google.com/forum/#!forum/project-kimchi
---
You received this message because you are subscribed to the Google Groups "project-kimchi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-kimch...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Sheldon Feng(冯少合)
IBM Linux Technology Center

[Royce Lv]

I like explicit load rather than put these in top level and generate a
global variable.
Think about two processes(let's say in websockify.py) both use this
module, top level code executed for a second time, the later process may
get a different config from the first.
But developer may fail to notice.
This will cause some trouble which is difficult to diagnose.

[Royce Lv]

On 2013年12月18日 16:51, Mark Wu wrote:

Update kimchid.in to allow this param to be assigned in cmd line as well.

[Mark Wu]

It's not option for kimchi server. So I don't think we need pass
around the option from command line. Actually, I am considering to
run the proxy service as a standalone service in future.

[Mark Wu]

Nope, I don't think it will cause any race. First of all, no running
config happens outside that module.
Second, that module will be imported only one time because they're
running in the same python process.
So do you still think it can cause any problem?

[Mark Wu]

On 12/18/2013 05:01 PM, Eli Qiao wrote:
>
> 于 2013年12月18日 16:51, Mark Wu 写道:
>> +# Port for vnc's websocket proxy to listen on
>> +#vnc_proxy_port = 64667
>> +
>> [logging]
>
> we need to add a iptable rule to open this port in spec file post
> section.
>

It will be resolved in a separate patch after we get agreement on the
approach of firewall configuration.

[Royce Lv]

1. This global variable need to be updated by OptionParser() to gurentee
the config is latest. global variable config in config.py in not
updated, if used elsewhere, it is the original value.
2. We can't gurantee we won't have process started by kimchi and use
config in the future. If others access this config else where, it is the
un-updated value.