[PATCH 0/5] Support vm name with special characters
Ramon Medeiros (5): 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 src/kimchi/model/vms.py | 50 +++++++++++++++++++++++++++++++++--------- src/kimchi/model/vmstorages.py | 4 ++-- src/kimchi/vmtemplate.py | 4 +++- ui/js/src/kimchi.api.js | 4 ++-- 4 files changed, 47 insertions(+), 15 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). 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 ef97d83..972745f 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -290,7 +290,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(vm_name.encode('utf-8'), errors='ignore') + params['title'] = vm_name params['uuid'] = vm_uuid params['networks'] = self._get_networks_xml() params['input_output'] = self._get_input_output_xml() @@ -320,6 +321,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
On 28/04/2015 10:16, Ramon Medeiros wrote:
unicode(vm_name.encode('utf-8'), errors='ignore')
I think it should be better to use "errors=xmlcharrefreplace" so we will make sure it will be XML complaint and also we will not loose any character. For example, let's say I create a guest "kimchí", using your code it will be converted to "kimch" so I will not be able to create another guest named "kimch" as it will already exists. Does that make sense?
On 28/04/2015 10:16, Ramon Medeiros wrote:
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).
Just to make sure everyone is aware about the issue you can add to the commit message the Fedora bug. As it is a Fedora bug due the systemd scope. (I mean, the issue is not presented in the distros not using systemd scope AFAIU)
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 ef97d83..972745f 100644 --- a/src/kimchi/vmtemplate.py +++ b/src/kimchi/vmtemplate.py @@ -290,7 +290,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(vm_name.encode('utf-8'), errors='ignore') + params['title'] = vm_name params['uuid'] = vm_uuid params['networks'] = self._get_networks_xml() params['input_output'] = self._get_input_output_xml() @@ -320,6 +321,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>
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 | 46 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index c8267c1..6792447 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -150,7 +150,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) + root = etree.fromstring(xml) + + # machine does not have title: use name + if root.find("title") == None: + names.append(root.find("name").text.decode('utf-8')) + continue + + # has title: use it + names.append(root.find("title").text) names = sorted(names, key=unicode.lower) return names @@ -865,14 +879,30 @@ class VMModel(object): def get_vm(name, conn): conn = conn.get() try: - # outgoing text to libvirt, encode('utf-8') - return conn.lookupByName(name.encode("utf-8")) + + for dom in conn.listAllDomains(0): + + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) + root = etree.fromstring(xml) + + # get name + # machine does not have title: use name + if root.find("title") == None: + vm_name = root.find("name").text + # has title: use it + else: + vm_name = root.find("title").text + + # name match: return dom + if vm_name == name: + return dom + 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}) + 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
On 28/04/2015 10:16, Ramon Medeiros wrote:
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 | 46 ++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 8 deletions(-)
diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index c8267c1..6792447 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -150,7 +150,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) + root = etree.fromstring(xml) + + # machine does not have title: use name + if root.find("title") == None: + names.append(root.find("name").text.decode('utf-8')) + continue +
There is some useful XML functions in xmlutils/utils.py Here you can use xpath_get_text() to get the 'title' value Something like: xml = dom.XMLDesc(...) title = xpath_get_text(xml, '/domain/title') # title tag is not present if len(title) == 0: names.append(dom.name().decode('utf-8)) else: names.append(title[0]) Also you have the libvirt fuction virDomain.name() to get the name without needing to parse the XML.
+ # has title: use it + names.append(root.find("title").text) names = sorted(names, key=unicode.lower) return names
@@ -865,14 +879,30 @@ class VMModel(object): def get_vm(name, conn): conn = conn.get() try: - # outgoing text to libvirt, encode('utf-8') - return conn.lookupByName(name.encode("utf-8"))
+ + for dom in conn.listAllDomains(0): + + xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) + root = etree.fromstring(xml) + + # get name + # machine does not have title: use name + if root.find("title") == None: + vm_name = root.find("name").text + # has title: use it + else: + vm_name = root.find("title").text + + # name match: return dom + if vm_name == name: + return dom +
I don't think you need to parse all the VMs to get the one you are looking for. You received the unicode named with non-ASCII values BUT you know how to convert it to get the real name on XML. So it should be: non_ascii_name = unicode(name.encode('utf-8'), errors='ignore') return conn.lookupByName(non_ascii_name)
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}) + raise OperationFailed("KCHVM0009E", {'name': name, 'err': e.message}) + + # not found + raise NotFoundError("KCHVM0002E", {'name': name}) +
def delete(self, name): conn = self.conn.get()
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 6792447..87d63c1 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -266,7 +266,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 37aca64..8dabae7 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -138,7 +138,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}) @@ -173,7 +173,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
As I said before you don't need to query all the guests to get a specific one as you know how to convert the non-ASCII name to only ASCII characters On 28/04/2015 10:16, Ramon Medeiros wrote:
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 6792447..87d63c1 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -266,7 +266,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 37aca64..8dabae7 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -138,7 +138,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}) @@ -173,7 +173,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
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 87d63c1..f4034d9 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -1050,7 +1050,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
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 28/04/2015 10:16, Ramon Medeiros wrote:
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 87d63c1..f4034d9 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -1050,7 +1050,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})
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
On 28/04/2015 10:16, Ramon Medeiros wrote:
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);
Probably, you will need to change it if backend uses "errors=xmlcharrefreplace"
participants (2)
-
Aline Manera -
Ramon Medeiros