[PATCH 0/9] Issue #450 - "test_delete_running_vm" fails on Fedora 20
Libvirt does not support special characters on name tag. This patch make Kimchi writes the special name at title tag and use an ascii version in name tag. Ramon Medeiros (9): Create title tag Change the way that get_vm() and vm_list() search for VMs Use get_vm() instead of conn.lookupByName Encode vm_name to generate VNC token Reflect the unicode conversion on token creation Redefine title tag on vm update Fix functions that uses connection as parameter Return name with special characters when taking snapshot Template_test: check assert that verifies the vm name src/kimchi/model/vms.py | 82 +++++++++++++++++++++++++++++++++++------ src/kimchi/model/vmsnapshots.py | 7 ++++ src/kimchi/model/vmstorages.py | 7 +--- src/kimchi/vmtemplate.py | 4 +- tests/test_vmtemplate.py | 8 +++- ui/js/src/kimchi.api.js | 4 +- 6 files changed, 92 insertions(+), 20 deletions(-) -- 2.1.0
Title tag will store name with special characters, while name will store with ascii convertion, to keep compatibility with libvirt (that does not accept unicode string on name tag). For for details of this bug: https://bugzilla.redhat.com/show_bug.cgi?id=1062943 Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/vmtemplate.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py index e047228..52e3333 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -284,7 +284,8 @@ class VMTemplate(object): def to_vm_xml(self, vm_name, vm_uuid, **kwargs): params = dict(self.info) - params['name'] = vm_name + params['name'] = unicode(filter(str.isalnum, vm_name.encode('ascii', 'xmlcharrefreplace'))) + params['title'] = vm_name params['uuid'] = vm_uuid params['networks'] = self._get_networks_xml() params['input_output'] = self._get_input_output_xml() @@ -312,6 +313,7 @@ class VMTemplate(object): <domain type='%(domain)s'> %(qemu-stream-cmdline)s <name>%(name)s</name> + <title>%(title)s</title> <uuid>%(uuid)s</uuid> <memory unit='MiB'>%(memory)s</memory> <vcpu>%(cpus)s</vcpu> -- 2.1.0
Instead of searching VMs by name, get_vm() and vm_list() will use as criteria the title tag to search vms. This is necessary to avoid unicode conflicts with libvirt. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 55 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index f0182b9..2ae58ee 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -169,7 +169,21 @@ class VMsModel(object): @staticmethod def get_vms(conn): conn_ = conn.get() - names = [dom.name().decode('utf-8') for dom in conn_.listAllDomains(0)] + + # retrieve title instead of name + names = [] + for dom in conn_.listAllDomains(0): + + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) + title = xpath_get_text(xml, '/domain/title') + + # machine does not have title: use name + if title == []: + names.append(unicode(dom.name())) + continue + + # has title: use it + names.append(title[0]) names = sorted(names, key=unicode.lower) return names @@ -882,15 +896,40 @@ class VMModel(object): @staticmethod def get_vm(name, conn): conn = conn.get() + + # try to reach vm directly try: - # outgoing text to libvirt, encode('utf-8') - return conn.lookupByName(name.encode("utf-8")) + dom = conn.lookupByName(unicode(filter(str.isalnum, + name.encode('ascii', 'xmlcharrefreplace')))) + return dom + # error: skip it and wait for the next search 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}) + pass + + try: + + for dom in conn.listAllDomains(0): + + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) + title = xpath_get_text(xml, '/domain/title') + + # machine does not have title: use name + if title == []: + vm_name = unicode(dom.name()) + # has title: use it + else: + vm_name = title[0] + + # name match: return dom + if vm_name == name: + return dom + + except libvirt.libvirtError as e: + raise OperationFailed("KCHVM0009E", {'name': name, 'err': e.message}) + + # not found + raise NotFoundError("KCHVM0002E", {'name': name}) + def delete(self, name): conn = self.conn.get() -- 2.1.0
Remove duplicated code of conn.lookupByName. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 2 +- src/kimchi/model/vmstorages.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 2ae58ee..3445def 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -284,7 +284,7 @@ class VMModel(object): # fetch base XML cb('reading source VM XML') try: - vir_dom = vir_conn.lookupByName(name) + vir_dom = self.get_vm(name) flags = libvirt.VIR_DOMAIN_XML_SECURE xml = vir_dom.XMLDesc(flags).decode('utf-8') except libvirt.libvirtError, e: diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 87c6b3d..95af7ac 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -136,7 +136,7 @@ class VMStoragesModel(object): dev, xml = get_disk_xml(params) try: conn = self.conn.get() - dom = conn.lookupByName(vm_name) + dom = VMModel.get_vm(vm_name, conn) dom.attachDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0008E", {'error': e.message}) @@ -171,7 +171,7 @@ class VMStorageModel(object): try: bus_type = self.lookup(vm_name, dev_name)['bus'] - dom = conn.lookupByName(vm_name) + dom = VMModel.get_vm(vm_name, conn) except NotFoundError: raise -- 2.1.0
Library base64 does not support unicode format. So, the conversion to utf-8 will be forced before. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 3445def..97c011d 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -1077,7 +1077,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}) -- 2.1.0
In the backend, the token is created after a convertion of unicode, so, the token passed by the ui will be forcely converted to unicode. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- ui/js/src/kimchi.api.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js index 5c36418..0f24a47 100644 --- a/ui/js/src/kimchi.api.js +++ b/ui/js/src/kimchi.api.js @@ -362,7 +362,7 @@ var kimchi = { * contain = which is not safe in a URL query component. * So remove it when needed as base64 can work well without it. * */ - url += "&path=?token=" + kimchi.urlSafeB64Encode(vm).replace(/=*$/g, ""); + url += "&path=?token=" + kimchi.urlSafeB64Encode(unescape(encodeURIComponent(vm))).replace(/=*$/g, ""); url += "&kimchi=" + location.port; url += '&encrypt=1'; window.open(url); @@ -394,7 +394,7 @@ var kimchi = { * contain = which is not safe in a URL query component. * So remove it when needed as base64 can work well without it. * */ - url += "&token=" + kimchi.urlSafeB64Encode(vm).replace(/=*$/g, ""); + url += "&token=" + kimchi.urlSafeB64Encode(unescape(encodeURIComponent(vm))).replace(/=*$/g, ""); url += "&kimchi=" + location.port; url += '&encrypt=1'; window.open(url); -- 2.1.0
To keep compatibility, redefine the title. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 97c011d..79967cc 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -59,6 +59,7 @@ DOM_STATE_MAP = {0: 'nostate', 7: 'pmsuspended'} VM_STATIC_UPDATE_PARAMS = {'name': './name', + 'title': './title', 'cpus': './vcpu', 'memory': './memory'} VM_LIVE_UPDATE_PARAMS = {} @@ -183,7 +184,8 @@ class VMsModel(object): continue # has title: use it - names.append(title[0]) + names.append(unicode(title[0])) + names = sorted(names, key=unicode.lower) return names @@ -211,7 +213,11 @@ class VMModel(object): dom = self.get_vm(name, self.conn) dom = self._static_vm_update(dom, params) self._live_vm_update(dom, params) - return dom.name().decode('utf-8') + + if params.get("name") != None: + return params["name"] + + return name def clone(self, name): """Clone a virtual machine based on an existing one. @@ -671,6 +677,21 @@ class VMModel(object): val = val * 1024 if type(val) == int: val = str(val) + + # name is passed: update name and title + if key == "name": + new_name = unicode(filter(str.isalnum, + params["name"].encode('ascii', 'xmlcharrefreplace'))) + + # update name + xpath = VM_STATIC_UPDATE_PARAMS[key] + new_xml = xml_item_update(new_xml, xpath, new_name) + + # update title + xpath = VM_STATIC_UPDATE_PARAMS["title"] + new_xml = xml_item_update(new_xml, xpath, val) + continue + xpath = VM_STATIC_UPDATE_PARAMS[key] new_xml = xml_item_update(new_xml, xpath, val) -- 2.1.0
Some calls to get_vm() were passing the connection instance, but the function waits for the connection object. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 2 +- src/kimchi/model/vmstorages.py | 7 ++----- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 79967cc..5f9dbaa 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -290,7 +290,7 @@ class VMModel(object): # fetch base XML cb('reading source VM XML') try: - vir_dom = self.get_vm(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: diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 95af7ac..c2aa7be 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -135,8 +135,7 @@ class VMStoragesModel(object): # Add device to VM dev, xml = get_disk_xml(params) try: - conn = self.conn.get() - dom = VMModel.get_vm(vm_name, conn) + 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 = VMModel.get_vm(vm_name, conn) + dom = VMModel.get_vm(vm_name, self.conn) except NotFoundError: raise -- 2.1.0
Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- src/kimchi/model/vmsnapshots.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/kimchi/model/vmsnapshots.py b/src/kimchi/model/vmsnapshots.py index 3a92cdc..af084a9 100644 --- a/src/kimchi/model/vmsnapshots.py +++ b/src/kimchi/model/vmsnapshots.py @@ -160,6 +160,13 @@ class VMSnapshotModel(object): # get vm name recorded in the snapshot and return new uri params vm_new_name = xpath_get_text(vir_snap.getXMLDesc(0), + 'domain/title') + + # title exists: return title + if len(vm_new_name) > 0: + return [vm_new_name[0], name] + + vm_new_name = xpath_get_text(vir_snap.getXMLDesc(0), 'domain/name')[0] return [vm_new_name, name] except libvirt.libvirtError, e: -- 2.1.0
The test was checking the name directly, which is wrong, due special characters imcompatibility. So, if title is present, use it. Signed-off-by: Ramon Medeiros <ramonn@linux.vnet.ibm.com> --- tests/test_vmtemplate.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py index b504fbc..4df1c7f 100644 --- a/tests/test_vmtemplate.py +++ b/tests/test_vmtemplate.py @@ -86,7 +86,13 @@ class VMTemplateTests(unittest.TestCase): t = VMTemplate({'name': 'test-template', 'cdrom': self.iso}) xml = t.to_vm_xml('test-vm', vm_uuid, graphics=graphics) self.assertEquals(vm_uuid, xpath_get_text(xml, "/domain/uuid")[0]) - self.assertEquals('test-vm', xpath_get_text(xml, "/domain/name")[0]) + + name = xpath_get_text(xml, "/domain/name")[0] + title = xpath_get_text(xml, "/domain/title") + if len(title) > 0: + name = title[0] + + self.assertEquals('test-vm', name) expr = "/domain/devices/graphics/@type" self.assertEquals(graphics['type'], xpath_get_text(xml, expr)[0]) expr = "/domain/devices/graphics/@listen" -- 2.1.0
"make check-local" returns a few errors: ./src/kimchi/vmtemplate.py:287:80: E501 line too long (99 > 79 characters) ./src/kimchi/model/vms.py:217:31: E711 comparison to None should be 'if cond is not None:' ./src/kimchi/model/vms.py:686:25: E128 continuation line under-indented for visual indent ./src/kimchi/model/vms.py:926:80: E501 line too long (80 > 79 characters) ./src/kimchi/model/vms.py:951:80: E501 line too long (81 > 79 characters) ./src/kimchi/model/vms.py:957:5: E303 too many blank lines (2) And "sudo make check" also reports a new test failure: ====================================================================== FAIL: test_vm_lifecycle (test_rest.RestTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "test_rest.py", line 307, in test_vm_lifecycle self.assertEquals(200, resp.status) AssertionError: 200 != 404 ---------------------------------------------------------------------- Please run "make check-local" and "sudo make check" to make sure new patches don't break the application. On 07-05-2015 14:40, Ramon Medeiros wrote:
Libvirt does not support special characters on name tag. This patch make Kimchi writes the special name at title tag and use an ascii version in name tag.
Ramon Medeiros (9): Create title tag Change the way that get_vm() and vm_list() search for VMs Use get_vm() instead of conn.lookupByName Encode vm_name to generate VNC token Reflect the unicode conversion on token creation Redefine title tag on vm update Fix functions that uses connection as parameter Return name with special characters when taking snapshot Template_test: check assert that verifies the vm name
src/kimchi/model/vms.py | 82 +++++++++++++++++++++++++++++++++++------ src/kimchi/model/vmsnapshots.py | 7 ++++ src/kimchi/model/vmstorages.py | 7 +--- src/kimchi/vmtemplate.py | 4 +- tests/test_vmtemplate.py | 8 +++- ui/js/src/kimchi.api.js | 4 +- 6 files changed, 92 insertions(+), 20 deletions(-)
participants (2)
-
CrÃstian Deives -
Ramon Medeiros