Sometimes it is useful to update multiple metadata nodes so allow it to be done in one single virDomain.setMetadata() call. Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; --- src/kimchi/model/utils.py | 6 ++++-- src/kimchi/model/vms.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/kimchi/model/utils.py b/src/kimchi/model/utils.py index c053807..feb67d9 100644 --- a/src/kimchi/model/utils.py +++ b/src/kimchi/model/utils.py @@ -71,11 +71,13 @@ def get_kimchi_metadata_node(dom, mode="current"): return None -def set_metadata_node(dom, node, mode="all"): +def set_metadata_node(dom, nodes, mode="all"): kimchi = get_kimchi_metadata_node(dom, mode) kimchi = E.metadata() if kimchi is None else kimchi - update_node(kimchi, node) + for n in nodes: + update_node(kimchi, n) + kimchi_xml = etree.tostring(kimchi) # From libvirt doc, Passing None for @metadata says to remove that # element from the domain XML (passing the empty string leaves the diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index a1f1798..fc2c92d 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -543,7 +543,7 @@ class VMModel(object): return node = self._build_access_elem(dom, users, groups) - set_metadata_node(dom, node) + set_metadata_node(dom, [node]) def _get_access_info(self, dom): users = groups = list() @@ -578,7 +578,7 @@ class VMModel(object): if distro is None: return os_elem = E.os({"distro": distro, "version": version}) - set_metadata_node(dom, os_elem) + set_metadata_node(dom, [os_elem]) def _update_graphics(self, dom, xml, params): root = objectify.fromstring(xml) -- 2.1.0

As discussed in the github (https://github.com/kimchi-project/kimchi/issues/450), the idea to solve this problem is always save the guest name with ASCII characters and use the <title> tag to handle the non-ASCII name. As the <title> tag is wide used for libvirt guests and Kimchi already has its own <metadata> section in the guest XML, I decided to include the non-ASCII name in the <metadata> as well, to avoid any conflicts with the <title> tag. When the guest name has non-ASCII characters, it is encoded to base64 and the unicode name is stored in the <metadata> as <name>. The backend is responsible to hanble those values, which means the UI keeps working with the non-ASCII name without problems. Another known issue is related on how Javascript works with base64 and non-ASCII characters which is preventing Kimchi to properly display the guest console. The problem is because atob() and btoa() do not work well with non-ASCII characters which turns into a wrong base64 result. For example, "café" would be "Y2Fmw6k" but btod() turns it into "Y2Fm6Q" This is another issue and related to UI. Another patch to fix it will be send soon. Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; --- src/kimchi/model/utils.py | 12 +++++ src/kimchi/model/vms.py | 109 +++++++++++++++++++++++++++-------------- src/kimchi/model/vmstorages.py | 7 +-- 3 files changed, 86 insertions(+), 42 deletions(-) diff --git a/src/kimchi/model/utils.py b/src/kimchi/model/utils.py index 9fc9ea0..5f47e6a 100644 --- a/src/kimchi/model/utils.py +++ b/src/kimchi/model/utils.py @@ -17,7 +17,9 @@ # 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 base64 import libvirt + from lxml import etree from lxml.builder import E @@ -38,6 +40,16 @@ def get_vm_name(vm_name, t_name, name_list): raise OperationFailed("KCHUTILS0003E") +def get_ascii_nonascii_name(name): + nonascii_name = None + if name.encode('ascii', 'ignore') != name: + nonascii_name = name + name = base64.urlsafe_b64encode(name.encode('utf-8')).rstrip('=') + name = unicode(name) + + return (name, nonascii_name) + + def get_vm_config_flag(dom, mode="persistent"): # libvirt.VIR_DOMAIN_AFFECT_CURRENT is 0 # VIR_DOMAIN_AFFECT_LIVE is 1, VIR_DOMAIN_AFFECT_CONFIG is 2 diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index fc2c92d..bd64702 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -17,14 +17,16 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -from lxml.builder import E +import copy import lxml.etree as ET -from lxml import etree, objectify import os import random import string import time import uuid + +from lxml import etree, objectify +from lxml.builder import E from xml.etree import ElementTree import libvirt @@ -35,10 +37,11 @@ from kimchi.exception import InvalidOperation, InvalidParameter from kimchi.exception import NotFoundError, OperationFailed from kimchi.kvmusertests import UserTests from kimchi.model.config import CapabilitiesModel +from kimchi.model.featuretests import FeatureTests from kimchi.model.tasks import TaskModel from kimchi.model.templates import TemplateModel -from kimchi.model.utils import get_vm_name -from kimchi.model.utils import get_metadata_node +from kimchi.model.utils import get_ascii_nonascii_name, get_vm_name +from kimchi.model.utils import get_metadata_node, remove_metadata_node from kimchi.model.utils import set_metadata_node from kimchi.rollbackcontext import RollbackContext from kimchi.screenshot import VMScreenshot @@ -119,7 +122,7 @@ class VMsModel(object): """ vm_uuid = str(uuid.uuid4()) t = params['template'] - name = params['name'] + name, nonascii_name = get_ascii_nonascii_name(params['name']) conn = self.conn.get() cb('Storing VM icon') @@ -161,7 +164,17 @@ class VMsModel(object): 'err': e.get_error_message()}) cb('Updating VM metadata') - VMModel.vm_update_os_metadata(VMModel.get_vm(name, self.conn), t.info) + meta_elements = [] + + distro = t.info.get("os_distro") + version = t.info.get("os_version") + if distro is not None: + meta_elements.append(E.os({"distro": distro, "version": version})) + + if nonascii_name is not None: + meta_elements.append(E.name(nonascii_name)) + + set_metadata_node(VMModel.get_vm(name, self.conn), meta_elements) cb('OK', True) def get_list(self): @@ -170,7 +183,14 @@ class VMsModel(object): @staticmethod def get_vms(conn): conn_ = conn.get() - names = [dom.name().decode('utf-8') for dom in conn_.listAllDomains(0)] + names = [] + for dom in conn_.listAllDomains(0): + nonascii_xml = get_metadata_node(dom, 'name') + if nonascii_xml: + nonascii_node = ET.fromstring(nonascii_xml) + names.append(nonascii_node.text) + else: + names.append(dom.name().decode('utf-8')) names = sorted(names, key=unicode.lower) return names @@ -196,9 +216,9 @@ class VMModel(object): def update(self, name, params): dom = self.get_vm(name, self.conn) - dom = self._static_vm_update(dom, params) + vm_name, dom = self._static_vm_update(name, dom, params) self._live_vm_update(dom, params) - return dom.name().decode('utf-8') + return vm_name def clone(self, name): """Clone a virtual machine based on an existing one. @@ -225,8 +245,6 @@ class VMModel(object): Return: A Task running the clone operation. """ - name = name.decode('utf-8') - # VM must be shutoff in order to clone it info = self.lookup(name) if info['state'] != u'shutoff': @@ -266,12 +284,11 @@ class VMModel(object): """ name = params['name'] new_name = params['new_name'] - vir_conn = self.conn.get() # fetch base XML cb('reading source VM XML') try: - vir_dom = vir_conn.lookupByName(name) + vir_dom = self.get_vm(name, self.conn) flags = libvirt.VIR_DOMAIN_XML_SECURE xml = vir_dom.XMLDesc(flags).decode('utf-8') except libvirt.libvirtError, e: @@ -299,12 +316,15 @@ class VMModel(object): # update name cb('updating VM name') + new_name, nonascii_name = get_ascii_nonascii_name(new_name) xml = xml_item_update(xml, './name', new_name) # create new guest cb('defining new VM') try: - vir_conn.defineXML(xml) + vir_conn = self.conn.get() + dom = vir_conn.defineXML(xml) + self._update_metadata_name(dom, nonascii_name) except libvirt.libvirtError, e: raise OperationFailed('KCHVM0035E', {'name': name, 'err': e.message}) @@ -571,15 +591,6 @@ class VMModel(object): os_elem = ET.fromstring(os_xml) return (os_elem.attrib.get("version"), os_elem.attrib.get("distro")) - @staticmethod - def vm_update_os_metadata(dom, params): - distro = params.get("os_distro") - version = params.get("os_version") - if distro is None: - return - os_elem = E.os({"distro": distro, "version": version}) - set_metadata_node(dom, [os_elem]) - def _update_graphics(self, dom, xml, params): root = objectify.fromstring(xml) graphics = root.devices.find("graphics") @@ -647,9 +658,26 @@ class VMModel(object): dom.snapshotCreateXML(info['xml'], flags) - def _static_vm_update(self, dom, params): + def _update_metadata_name(self, dom, nonascii_name): + if nonascii_name is not None: + set_metadata_node(dom, [E.name(nonascii_name)]) + else: + remove_metadata_node(dom, 'name') + + def _static_vm_update(self, vm_name, dom, params): old_xml = new_xml = dom.XMLDesc(0) + params = copy.deepcopy(params) + name = params.get('name') + nonascii_name = None + if name is not None: + state = DOM_STATE_MAP[dom.info()[0]] + if state != 'shutoff': + msg_args = {'name': vm_name, 'new_name': params['name']} + raise InvalidParameter("KCHVM0003E", msg_args) + + params['name'], nonascii_name = get_ascii_nonascii_name(name) + for key, val in params.items(): if key in VM_STATIC_UPDATE_PARAMS: if type(val) == int: @@ -674,15 +702,9 @@ class VMModel(object): new_xml = self._update_graphics(dom, new_xml, params) snapshots_info = [] - vm_name = dom.name() conn = self.conn.get() try: if 'name' in params: - state = DOM_STATE_MAP[dom.info()[0]] - if state != 'shutoff': - msg_args = {'name': vm_name, 'new_name': params['name']} - raise InvalidParameter("KCHVM0003E", msg_args) - lflags = libvirt.VIR_DOMAIN_SNAPSHOT_LIST_ROOTS dflags = (libvirt.VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN | libvirt.VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) @@ -698,6 +720,7 @@ class VMModel(object): dom.undefine() dom = conn.defineXML(new_xml) + self._update_metadata_name(dom, nonascii_name) if 'name' in params: self._redefine_snapshots(dom, snapshots_info) except libvirt.libvirtError as e: @@ -707,7 +730,7 @@ class VMModel(object): raise OperationFailed("KCHVM0008E", {'name': vm_name, 'err': e.get_error_message()}) - return dom + return (nonascii_name if nonascii_name is not None else vm_name, dom) def _update_memory_config(self, xml, params): # Checks if NUMA memory is already configured, if not, checks if CPU @@ -1005,16 +1028,28 @@ class VMModel(object): @staticmethod def get_vm(name, conn): + def raise_exception(error_code): + if error_code == libvirt.VIR_ERR_NO_DOMAIN: + raise NotFoundError("KCHVM0002E", {'name': name}) + else: + raise OperationFailed("KCHVM0009E", {'name': name, + 'err': e.message}) conn = conn.get() + FeatureTests.disable_libvirt_error_logging() try: # outgoing text to libvirt, encode('utf-8') return conn.lookupByName(name.encode("utf-8")) except libvirt.libvirtError as e: - if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: - raise NotFoundError("KCHVM0002E", {'name': name}) - else: - raise OperationFailed("KCHVM0009E", {'name': name, - 'err': e.message}) + name, nonascii_name = get_ascii_nonascii_name(name) + if nonascii_name is None: + raise_exception(e.get_error_code()) + + try: + return conn.lookupByName(name) + except libvirt.libvirtError as e: + raise_exception(e.get_error_code()) + + FeatureTests.enable_libvirt_error_logging() def delete(self, name): conn = self.conn.get() @@ -1164,7 +1199,7 @@ class VMModel(object): # (type, listen, port, passwd, passwdValidTo) graphics_port = self._vm_get_graphics(name)[2] if graphics_port is not None: - vnc.add_proxy_token(name, graphics_port) + vnc.add_proxy_token(name.encode('utf-8'), graphics_port) else: raise OperationFailed("KCHVM0010E", {'name': name}) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 5b90c18..0beaaf4 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -134,8 +134,7 @@ class VMStoragesModel(object): # Add device to VM dev, xml = get_disk_xml(params) try: - conn = self.conn.get() - dom = conn.lookupByName(vm_name) + dom = VMModel.get_vm(vm_name, self.conn) dom.attachDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0008E", {'error': e.message}) @@ -167,11 +166,9 @@ class VMStorageModel(object): return get_vm_disk_info(dom, dev_name) def delete(self, vm_name, dev_name): - conn = self.conn.get() - try: bus_type = self.lookup(vm_name, dev_name)['bus'] - dom = conn.lookupByName(vm_name) + dom = VMModel.get_vm(vm_name, self.conn) except NotFoundError: raise -- 2.1.0