[Patch v2 1/4] Move configuration parsing to config.py

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

VM on power machine requires different parameters from X86 machine. So kimchi should provide different template according to the architecture. IDE bus is not available on power platform. So change the device bus of cdrom to scsi instead of ide for VM on power. The same problem exists for old nic model, and change it to rtl8139 instead. Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index b141d9e..62c1bbf 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -25,6 +25,9 @@ import os from distutils.version import LooseVersion +SUPPORTED_ARCHS = {'x86': ('i386', 'x86_64'), 'power': ('ppc', 'ppc64')} + + common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', 'cdrom_index': 2} @@ -33,7 +36,15 @@ common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio') -old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') +template_specs = {'x86': {'old': dict(common_spec, disk_bus='ide', + nic_model='e1000'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio')}, + 'power': {'old': dict(common_spec, disk_bus='scsi', + nic_model='rtl8139', cdrom_bus='scsi'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio', + cdrom_bus='scsi')}} modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', @@ -61,6 +72,13 @@ defaults = {'network': 'default', 'storagepool': '/storagepools/default', 'domain': 'kvm', 'arch': os.uname()[4] } + +def _get_arch(): + for arch, sub_archs in SUPPORTED_ARCHS.iteritems(): + if os.uname()[4] in sub_archs: + return arch + + def lookup(distro, version): """ Lookup all parameters needed to run a VM of a known or unknown operating @@ -72,18 +90,19 @@ def lookup(distro, version): params['os_distro'] = distro params['os_version'] = version params['cdrom'] = isolinks.get(distro, {}).get(version, '') + arch = _get_arch() for name, base_version in modern_version_bases.iteritems(): if name == distro: params['icon'] = 'images/icon-%s.png' % distro if LooseVersion(version) >= LooseVersion(base_version): - params.update(modern_spec) + params.update(template_specs[arch]['modern']) else: - params.update(old_spec) + params.update(template_specs[arch]['old']) break else: params['icon'] = 'images/icon-vm.png' params['os_distro'] = params['os_version'] = "unknown" - params.update(old_spec) + params.update(template_specs[arch]['old']) return params -- 1.8.3.1

On 12/24/2013 12:41 AM, Mark Wu wrote:
VM on power machine requires different parameters from X86 machine. So kimchi should provide different template according to the architecture. IDE bus is not available on power platform. So change the device bus of cdrom to scsi instead of ide for VM on power. The same problem exists for old nic model, and change it to rtl8139 instead.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index b141d9e..62c1bbf 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -25,6 +25,9 @@ import os from distutils.version import LooseVersion
It seems this patch wasn't made based on master branch. Or I am missing something.
+SUPPORTED_ARCHS = {'x86': ('i386', 'x86_64'), 'power': ('ppc', 'ppc64')} + + common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', 'cdrom_index': 2} @@ -33,7 +36,15 @@ common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio')
-old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') +template_specs = {'x86': {'old': dict(common_spec, disk_bus='ide', + nic_model='e1000'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio')}, + 'power': {'old': dict(common_spec, disk_bus='scsi', + nic_model='rtl8139', cdrom_bus='scsi'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio', + cdrom_bus='scsi')}}
modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', @@ -61,6 +72,13 @@ defaults = {'network': 'default', 'storagepool': '/storagepools/default', 'domain': 'kvm', 'arch': os.uname()[4] }
+ +def _get_arch(): + for arch, sub_archs in SUPPORTED_ARCHS.iteritems(): + if os.uname()[4] in sub_archs: + return arch + +
You can use platform.machine() to get arch info
import platform platform.machine() 'x86_64'
def lookup(distro, version): """ Lookup all parameters needed to run a VM of a known or unknown operating @@ -72,18 +90,19 @@ def lookup(distro, version): params['os_distro'] = distro params['os_version'] = version params['cdrom'] = isolinks.get(distro, {}).get(version, '') + arch = _get_arch()
for name, base_version in modern_version_bases.iteritems(): if name == distro: params['icon'] = 'images/icon-%s.png' % distro if LooseVersion(version) >= LooseVersion(base_version): - params.update(modern_spec) + params.update(template_specs[arch]['modern']) else: - params.update(old_spec) + params.update(template_specs[arch]['old']) break else: params['icon'] = 'images/icon-vm.png' params['os_distro'] = params['os_version'] = "unknown" - params.update(old_spec) + params.update(template_specs[arch]['old'])
return params

On 12/27/2013 12:01 AM, Aline Manera wrote:
On 12/24/2013 12:41 AM, Mark Wu wrote:
VM on power machine requires different parameters from X86 machine. So kimchi should provide different template according to the architecture. IDE bus is not available on power platform. So change the device bus of cdrom to scsi instead of ide for VM on power. The same problem exists for old nic model, and change it to rtl8139 instead.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/osinfo.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/kimchi/osinfo.py b/src/kimchi/osinfo.py index b141d9e..62c1bbf 100644 --- a/src/kimchi/osinfo.py +++ b/src/kimchi/osinfo.py @@ -25,6 +25,9 @@ import os from distutils.version import LooseVersion
It seems this patch wasn't made based on master branch. Or I am missing something. I am not sure if it's valid for current master code. I will try to rebase it if necessary.
+SUPPORTED_ARCHS = {'x86': ('i386', 'x86_64'), 'power': ('ppc', 'ppc64')} + + common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, 'disks': [{'index': 0, 'size': 10}], 'cdrom_bus': 'ide', 'cdrom_index': 2} @@ -33,7 +36,15 @@ common_spec = {'cpus': 1, 'cpu_cores': 1, 'cpu_threads': 1, 'memory': 1024, modern_spec = dict(common_spec, disk_bus='virtio', nic_model='virtio')
-old_spec = dict(common_spec, disk_bus='ide', nic_model='e1000') +template_specs = {'x86': {'old': dict(common_spec, disk_bus='ide', + nic_model='e1000'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio')}, + 'power': {'old': dict(common_spec, disk_bus='scsi', + nic_model='rtl8139', cdrom_bus='scsi'), + 'modern': dict(common_spec, disk_bus='virtio', + nic_model='virtio', + cdrom_bus='scsi')}}
modern_version_bases = {'debian': '6.0', 'ubuntu': '7.10', 'opensuse': '10.3', @@ -61,6 +72,13 @@ defaults = {'network': 'default', 'storagepool': '/storagepools/default', 'domain': 'kvm', 'arch': os.uname()[4] }
+ +def _get_arch(): + for arch, sub_archs in SUPPORTED_ARCHS.iteritems(): + if os.uname()[4] in sub_archs: + return arch + +
You can use platform.machine() to get arch info
import platform platform.machine() 'x86_64'
Yes, but platform.machine just use uname. We can avoid an unnecessary import by using uname.
def lookup(distro, version): """ Lookup all parameters needed to run a VM of a known or unknown operating @@ -72,18 +90,19 @@ def lookup(distro, version): params['os_distro'] = distro params['os_version'] = version params['cdrom'] = isolinks.get(distro, {}).get(version, '') + arch = _get_arch()
for name, base_version in modern_version_bases.iteritems(): if name == distro: params['icon'] = 'images/icon-%s.png' % distro if LooseVersion(version) >= LooseVersion(base_version): - params.update(modern_spec) + params.update(template_specs[arch]['modern']) else: - params.update(old_spec) + params.update(template_specs[arch]['old']) break else: params['icon'] = 'images/icon-vm.png' params['os_distro'] = params['os_version'] = "unknown" - params.update(old_spec) + params.update(template_specs[arch]['old'])
return params

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

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@linux.vnet.ibm.com> --- 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
+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() {

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@linux.vnet.ibm.com> --- 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() {

On 12/27/2013 09:52 AM, Mark Wu wrote:
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@linux.vnet.ibm.com> --- 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?
We are using the following rule: import ... import ... import ... <2 lines> from ... import ... from ... import ... <2 lines> import kimchi... from kimchi import ... <2 lines> All those blocks must be in alphabetic order
+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() {

On Tue, 2013-12-24 at 10:41 +0800, 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.
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
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- 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() {

Mockmodel is used to test restapi's request validation and response. We don't need to start a real vnc server because we don't run functional test with mock model. Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 348127a..50e89a5 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): @@ -140,8 +125,7 @@ class MockModel(object): 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 + pass def vms_create(self, params): t_name = kimchi.model.template_name_from_uri(params['template']) -- 1.8.3.1

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 12/24/2013 12:41 AM, Mark Wu wrote:
Mockmodel is used to test restapi's request validation and response. We don't need to start a real vnc server because we don't run functional test with mock model.
Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com> --- src/kimchi/mockmodel.py | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-)
diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py index 348127a..50e89a5 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): @@ -140,8 +125,7 @@ class MockModel(object): 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 + pass
def vms_create(self, params): t_name = kimchi.model.template_name_from_uri(params['template'])

于 2013年12月24日 10:41, Mark Wu 写道:
The configruation is also needed for other code except starting kimchi
except -> apart from I think '除了' in our Chinese has two meanings, in English there are two words for different meaning. In this case I guess you want to use 'apart from'.
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@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")
-- Thanks and best regards! Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397

On 12/24/2013 01:09 PM, Zhou Zheng Sheng wrote:
于 2013年12月24日 10:41, Mark Wu 写道:
The configruation is also needed for other code except starting kimchi except -> apart from
I think '除了' in our Chinese has two meanings, in English there are two words for different meaning. In this case I guess you want to use 'apart from'. Yes, you're correct. I felt weird when I wrote the commit message. Thanks for the good comment.
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@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")

On 12/24/2013 12:41 AM, Mark Wu wrote:
The configruation is also needed for other code except starting kimchi
typo: configruation
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@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 - -
Keep 2 lines here.
+from ConfigParser import SafeConfigParser from glob import iglob
-
And here
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) + +
Let's do it in a function: get_server_config() or something like it
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 12/26/2013 11:53 PM, Aline Manera wrote:
On 12/24/2013 12:41 AM, Mark Wu wrote:
The configruation is also needed for other code except starting kimchi
typo: configruation
Ack.
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@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 - -
Keep 2 lines here.
+from ConfigParser import SafeConfigParser from glob import iglob
-
And here
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) + +
Let's do it in a function: get_server_config() or something like it I will resolve it in next in v3. thanks!
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 12/26/2013 11:53 PM, Aline Manera wrote:
On 12/24/2013 12:41 AM, Mark Wu wrote:
The configruation is also needed for other code except starting kimchi
typo: configruation
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@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 - -
Keep 2 lines here.
I think PEP8 is enough for coding style guide. I don't think we need other rules for code style.
+from ConfigParser import SafeConfigParser from glob import iglob
-
And here
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) + +
Let's do it in a function: get_server_config() or something like it
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")
participants (4)
-
Aline Manera
-
Christy Perez
-
Mark Wu
-
Zhou Zheng Sheng