[PATCH 0/4] Fix bug #450: Allow creating guest with non-ASCII characters in name

*** BLURB HERE *** Aline Manera (4): Remove manual <metadata> manupulation Allow setting multiple metadata nodes at once Create helper method to remove metadata node Fix bug #450: Allow creating guest with non-ASCII characters in name src/kimchi/i18n.py | 1 - src/kimchi/model/config.py | 2 - src/kimchi/model/featuretests.py | 24 -------- src/kimchi/model/utils.py | 118 ++++++++++++++----------------------- src/kimchi/model/vmifaces.py | 2 +- src/kimchi/model/vms.py | 122 ++++++++++++++++++++++++--------------- src/kimchi/model/vmstorages.py | 14 ++--- 7 files changed, 127 insertions(+), 156 deletions(-) -- 2.1.0

All the latest version of the major Linux distributions are now using the libvirt which has support to <metadata> manupulation through methods virDomain.setMetadata() and virDomain.metadata() So remove the manual manupulation to make the code maintenance easier. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 - src/kimchi/model/config.py | 2 - src/kimchi/model/featuretests.py | 24 ----------- src/kimchi/model/utils.py | 93 ++++++++-------------------------------- src/kimchi/model/vmifaces.py | 2 +- src/kimchi/model/vms.py | 19 ++++---- src/kimchi/model/vmstorages.py | 7 ++- 7 files changed, 30 insertions(+), 118 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 862df7f..116e520 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -101,7 +101,6 @@ messages = { "KCHVM0027E": _("User(s) '%(users)s' do not exist"), "KCHVM0028E": _("Group(s) '%(groups)s' do not exist"), "KCHVM0029E": _("Unable to shutdown virtual machine %(name)s. Details: %(err)s"), - "KCHVM0030E": _("Unable to get access metadata of virtual machine %(name)s. Details: %(err)s"), "KCHVM0031E": _("The guest console password must be a string."), "KCHVM0032E": _("The life time for the guest console password must be a number."), "KCHVM0033E": _("Virtual machine '%(name)s' must be stopped before cloning it."), diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py index d894b21..fe2a529 100644 --- a/src/kimchi/model/config.py +++ b/src/kimchi/model/config.py @@ -53,7 +53,6 @@ class CapabilitiesModel(object): self.qemu_stream = False self.libvirt_stream_protocols = [] self.fc_host_support = False - self.metadata_support = False self.kernel_vfio = False self.mem_hotplug_support = False @@ -90,7 +89,6 @@ class CapabilitiesModel(object): self.qemu_stream = FeatureTests.qemu_supports_iso_stream() self.nfs_target_probe = FeatureTests.libvirt_support_nfs_probe(conn) self.fc_host_support = FeatureTests.libvirt_support_fc_host(conn) - self.metadata_support = FeatureTests.has_metadata_support(conn) self.kernel_vfio = FeatureTests.kernel_support_vfio() self.mem_hotplug_support = FeatureTests.has_mem_hotplug_support(conn) diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 047108f..4075d7c 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -186,30 +186,6 @@ class FeatureTests(object): return True @staticmethod - def has_metadata_support(conn): - KIMCHI_META_URL = "https://github.com/kimchi-project/kimchi/" - KIMCHI_NAMESPACE = "kimchi" - with RollbackContext() as rollback: - FeatureTests.disable_libvirt_error_logging() - rollback.prependDefer(FeatureTests.enable_libvirt_error_logging) - conn_type = conn.getType().lower() - domain_type = 'test' if conn_type == 'test' else 'kvm' - arch = 'i686' if conn_type == 'test' else platform.machine() - arch = 'ppc64' if arch == 'ppc64le' else arch - dom = conn.defineXML(SIMPLE_VM_XML % {'name': FEATURETEST_VM_NAME, - 'domain': domain_type, - 'arch': arch}) - rollback.prependDefer(dom.undefine) - try: - dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, - "<metatest/>", KIMCHI_NAMESPACE, - KIMCHI_META_URL, - flags=libvirt.VIR_DOMAIN_AFFECT_CURRENT) - return True - except libvirt.libvirtError: - return False - - @staticmethod def kernel_support_vfio(): out, err, rc = run_command(['modprobe', 'vfio-pci']) if rc != 0: diff --git a/src/kimchi/model/utils.py b/src/kimchi/model/utils.py index 7f27edd..c053807 100644 --- a/src/kimchi/model/utils.py +++ b/src/kimchi/model/utils.py @@ -18,8 +18,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import libvirt -from lxml import etree, objectify -from lxml.builder import E, ElementMaker +from lxml import etree +from lxml.builder import E from kimchi.exception import OperationFailed @@ -59,30 +59,7 @@ def update_node(root, node): return root -def _kimchi_set_metadata_node(dom, node): - # some other tools will not let libvirt create a persistent - # configuration, raise exception. - if not dom.isPersistent(): - msg = 'The VM has not a persistent configuration' - raise OperationFailed("KCHVM0030E", - {'name': dom.name(), "err": msg}) - xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) - root = etree.fromstring(xml) - kimchi = root.find("metadata/{%s}kimchi" % KIMCHI_META_URL) - - EM = ElementMaker(namespace=KIMCHI_META_URL, - nsmap={KIMCHI_NAMESPACE: KIMCHI_META_URL}) - kimchi = EM("kimchi") if kimchi is None else kimchi - - update_node(kimchi, node) - metadata = root.find("metadata") - metadata = E.metadata() if metadata is None else metadata - update_node(metadata, kimchi) - update_node(root, metadata) - dom.connect().defineXML(etree.tostring(root)) - - -def libvirt_get_kimchi_metadata_node(dom, mode="current"): +def get_kimchi_metadata_node(dom, mode="current"): if not metadata_exists(dom): return None try: @@ -94,56 +71,22 @@ def libvirt_get_kimchi_metadata_node(dom, mode="current"): return None -def set_metadata_node(dom, node, metadata_support, mode="all"): - if metadata_support: - kimchi = libvirt_get_kimchi_metadata_node(dom, mode) - kimchi = E.metadata(E.kimchi()) if kimchi is None else kimchi - - update_node(kimchi, node) - 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 - # element present). Do not support remove the old metadata. - dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, kimchi_xml, - KIMCHI_NAMESPACE, KIMCHI_META_URL, - flags=get_vm_config_flag(dom, mode)) - else: - # FIXME remove this code when all distro libvirt supports metadata - # element - _kimchi_set_metadata_node(dom, node) - - -def _kimchi_get_metadata_node(dom, tag): - # some other tools will not let libvirt create a persistent - # configuration, just return empty - if not dom.isPersistent(): - return None - xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) - root = etree.fromstring(xml) - kimchi = root.find("metadata/{%s}kimchi" % KIMCHI_META_URL) - # remove the "kimchi" prefix of xml - if kimchi is not None: - for elem in kimchi.getiterator(): - if not hasattr(elem.tag, 'find'): - continue - i = elem.tag.find('}') - if i >= 0: - elem.tag = elem.tag[i+1:] - - objectify.deannotate(kimchi) - etree.cleanup_namespaces(kimchi) - return kimchi - return None - - -def get_metadata_node(dom, tag, metadata_support, mode="current"): - if metadata_support: - kimchi = libvirt_get_kimchi_metadata_node(dom, mode) - else: - # FIXME remove this code when all distro libvirt supports metadata - # element - kimchi = _kimchi_get_metadata_node(dom, tag) +def set_metadata_node(dom, node, mode="all"): + kimchi = get_kimchi_metadata_node(dom, mode) + kimchi = E.metadata() if kimchi is None else kimchi + + update_node(kimchi, node) + 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 + # element present). Do not support remove the old metadata. + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, kimchi_xml, + KIMCHI_NAMESPACE, KIMCHI_META_URL, + flags=get_vm_config_flag(dom, mode)) + +def get_metadata_node(dom, tag, mode="current"): + kimchi = get_kimchi_metadata_node(dom, mode) if kimchi is not None: node = kimchi.find(tag) if node is not None: diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py index 93a769b..8c97786 100644 --- a/src/kimchi/model/vmifaces.py +++ b/src/kimchi/model/vmifaces.py @@ -74,7 +74,7 @@ class VMIfacesModel(object): dom = VMModel.get_vm(vm, self.conn) - os_data = VMModel.vm_get_os_metadata(dom, self.caps.metadata_support) + os_data = VMModel.vm_get_os_metadata(dom) os_version, os_distro = os_data xml = get_iface_xml(params, conn.getInfo()[0], os_distro, os_version) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 106e9bc..a1f1798 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -161,8 +161,7 @@ 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, - self.caps.metadata_support) + VMModel.vm_update_os_metadata(VMModel.get_vm(name, self.conn), t.info) cb('OK', True) def get_list(self): @@ -485,8 +484,7 @@ class VMModel(object): def _build_access_elem(self, dom, users, groups): auth = config.get("authentication", "method") - access_xml = get_metadata_node(dom, "access", - self.caps.metadata_support) + access_xml = get_metadata_node(dom, "access") auth_elem = None @@ -545,12 +543,11 @@ class VMModel(object): return node = self._build_access_elem(dom, users, groups) - set_metadata_node(dom, node, self.caps.metadata_support) + set_metadata_node(dom, node) def _get_access_info(self, dom): users = groups = list() - access_xml = (get_metadata_node(dom, "access", - self.caps.metadata_support) or + access_xml = (get_metadata_node(dom, "access") or """<access></access>""") access_info = dictize(access_xml) auth = config.get("authentication", "method") @@ -568,20 +565,20 @@ class VMModel(object): return users, groups @staticmethod - def vm_get_os_metadata(dom, metadata_support): - os_xml = (get_metadata_node(dom, "os", metadata_support) or + def vm_get_os_metadata(dom): + os_xml = (get_metadata_node(dom, "os") or """<os></os>""") 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, metadata_support): + 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, metadata_support) + set_metadata_node(dom, os_elem) def _update_graphics(self, dom, xml, params): root = objectify.fromstring(xml) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 142b177..5b90c18 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -36,9 +36,9 @@ from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks HOTPLUG_TYPE = ['scsi', 'virtio'] -def _get_device_bus(dev_type, dom, metadata_support): +def _get_device_bus(dev_type, dom): try: - version, distro = VMModel.vm_get_os_metadata(dom, metadata_support) + version, distro = VMModel.vm_get_os_metadata(dom) except: version, distro = ('unknown', 'unknown') return lookup(distro, version)[dev_type+'_bus'] @@ -84,8 +84,7 @@ class VMStoragesModel(object): raise InvalidParameter("KCHVMSTOR0017E") dom = VMModel.get_vm(vm_name, self.conn) - params['bus'] = _get_device_bus(params['type'], dom, - self.caps.metadata_support) + params['bus'] = _get_device_bus(params['type'], dom) params['format'] = 'raw' dev_list = [dev for dev, bus in get_vm_disks(dom).iteritems() -- 2.1.0

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

It will be useful when updating guest metadata configuration. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/utils.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/kimchi/model/utils.py b/src/kimchi/model/utils.py index feb67d9..9fc9ea0 100644 --- a/src/kimchi/model/utils.py +++ b/src/kimchi/model/utils.py @@ -71,6 +71,16 @@ def get_kimchi_metadata_node(dom, mode="current"): return None +def set_kimchi_metadata_node(dom, metadata, mode="all"): + metadata_xml = etree.tostring(metadata) + # From libvirt doc, Passing None for @metadata says to remove that + # element from the domain XML (passing the empty string leaves the + # element present). Do not support remove the old metadata. + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, metadata_xml, + KIMCHI_NAMESPACE, KIMCHI_META_URL, + flags=get_vm_config_flag(dom, mode)) + + def set_metadata_node(dom, nodes, mode="all"): kimchi = get_kimchi_metadata_node(dom, mode) kimchi = E.metadata() if kimchi is None else kimchi @@ -78,13 +88,16 @@ def set_metadata_node(dom, nodes, mode="all"): 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 - # element present). Do not support remove the old metadata. - dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, kimchi_xml, - KIMCHI_NAMESPACE, KIMCHI_META_URL, - flags=get_vm_config_flag(dom, mode)) + set_kimchi_metadata_node(dom, kimchi, mode) + + +def remove_metadata_node(dom, tag, mode="all"): + kimchi = get_kimchi_metadata_node(dom, mode) + if kimchi is not None: + node = kimchi.find(tag) + if node is not None: + kimchi.remove(node) + set_kimchi_metadata_node(dom, kimchi, mode) def get_metadata_node(dom, tag, mode="current"): -- 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@linux.vnet.ibm.com> --- 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

Tested on Fedora 22 Tested-By: Ramon Medeiros <ramonn@br.ibm.com> Reviewed-By: Ramon Medeiros <ramonn@br.ibm.com> On 09/01/2015 07:49 PM, Aline Manera wrote:
*** BLURB HERE *** Aline Manera (4): Remove manual <metadata> manupulation Allow setting multiple metadata nodes at once Create helper method to remove metadata node Fix bug #450: Allow creating guest with non-ASCII characters in name
src/kimchi/i18n.py | 1 - src/kimchi/model/config.py | 2 - src/kimchi/model/featuretests.py | 24 -------- src/kimchi/model/utils.py | 118 ++++++++++++++----------------------- src/kimchi/model/vmifaces.py | 2 +- src/kimchi/model/vms.py | 122 ++++++++++++++++++++++++--------------- src/kimchi/model/vmstorages.py | 14 ++--- 7 files changed, 127 insertions(+), 156 deletions(-)
-- Ramon Nunes Medeiros Kimchi Developer Linux Technology Center Brazil IBM Systems & Technology Group Phone : +55 19 2132 7878 ramonn@br.ibm.com
participants (2)
-
Aline Manera
-
Ramon Medeiros