[PATCH V2 0/3] bug fix: failed to update vm with unicode name
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> V1 -> V2 rebase add test case ShaoHe Feng (3): bug fix: failed to update vm with unicode name update test case for updating vm with unicode name change the vm name in test case src/kimchi/control/base.py | 2 +- src/kimchi/model/vms.py | 2 +- tests/test_model.py | 18 +++++++++--------- tests/test_rest.py | 7 ++++--- 4 files changed, 15 insertions(+), 14 deletions(-) -- 1.8.4.2
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> update a vm's name with unicode name, kimchi will report error. $ curl -u <user> -H 'Accept: application/json' \ -H 'Content-type: application/json' \ http://localhost:8000/vms/u13.10 -X PUT -d ' {"name": "kīмсhī-∨м"}' There are two bugs: 1. typo encode the ident with "utf8" when HTTPRedirect change it to "utf-8" 2. The name form libvirt is str, need decode. ref: https://github.com/kimchi-project/kimchi/wiki/support-unicode Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 2 +- src/kimchi/model/vms.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f50ff6e..0b656ab 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -154,7 +154,7 @@ class Resource(object): ident = update(self.ident, params) if ident != self.ident: uri_params = list(self.model_args[:-1]) - uri_params += [urllib2.quote(ident.encode('utf8'))] + uri_params += [urllib2.quote(ident.encode('utf-8'))] raise cherrypy.HTTPRedirect(self.uri_fmt % tuple(uri_params), 303) return self.get() diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index d4384a1..1244eeb 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -222,7 +222,7 @@ 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() + return dom.name().decode('utf-8') def _static_vm_update(self, dom, params): state = DOM_STATE_MAP[dom.info()[0]] -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 05:48 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
update a vm's name with unicode name, kimchi will report error. $ curl -u <user> -H 'Accept: application/json' \ -H 'Content-type: application/json' \ http://localhost:8000/vms/u13.10 -X PUT -d ' {"name": "kīмсhī-∨м"}'
There are two bugs: 1. typo encode the ident with "utf8" when HTTPRedirect change it to "utf-8"
2. The name form libvirt is str, need decode. ref: https://github.com/kimchi-project/kimchi/wiki/support-unicode
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- src/kimchi/control/base.py | 2 +- src/kimchi/model/vms.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index f50ff6e..0b656ab 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -154,7 +154,7 @@ class Resource(object): ident = update(self.ident, params) if ident != self.ident: uri_params = list(self.model_args[:-1]) - uri_params += [urllib2.quote(ident.encode('utf8'))] + uri_params += [urllib2.quote(ident.encode('utf-8'))] raise cherrypy.HTTPRedirect(self.uri_fmt % tuple(uri_params), 303)
return self.get() diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index d4384a1..1244eeb 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -222,7 +222,7 @@ 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() + return dom.name().decode('utf-8')
def _static_vm_update(self, dom, params): state = DOM_STATE_MAP[dom.info()[0]]
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> update test case for it to avoid regression. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 6 +++--- tests/test_rest.py | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_model.py b/tests/test_model.py index 4a6d2fe..ef15be5 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -448,12 +448,12 @@ class ModelTests(unittest.TestCase): 'kimchi-vm1', params) inst.vm_stop('kimchi-vm1') - params = {'name': 'new-vm'} + params = {'name': u'пeω-∨м'} self.assertRaises(OperationFailed, inst.vm_update, 'kimchi-vm1', {'name': 'kimchi-vm2'}) inst.vm_update('kimchi-vm1', params) - self.assertEquals(info['uuid'], inst.vm_lookup('new-vm')['uuid']) - rollback.prependDefer(inst.vm_delete, 'new-vm') + self.assertEquals(info['uuid'], inst.vm_lookup(u'пeω-∨м')['uuid']) + rollback.prependDefer(inst.vm_delete, u'пeω-∨м') @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_network(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index 0ed293b..da2e2b9 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- # # Project Kimchi # @@ -201,11 +202,11 @@ class RestTests(unittest.TestCase): resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(405, resp.status) - req = json.dumps({'name': 'vm-updated'}) + req = json.dumps({'name': u'∨м-црdαtеd'}) resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(303, resp.status) - vm = json.loads(self.request('/vms/vm-updated', req).read()) - self.assertEquals('vm-updated', vm['name']) + vm = json.loads(self.request('/vms/∨м-црdαtеd', req).read()) + self.assertEquals(u'∨м-црdαtеd', vm['name']) def test_vm_lifecycle(self): # Create a Template -- 1.8.4.2
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 05:48 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
update test case for it to avoid regression.
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 6 +++--- tests/test_rest.py | 7 ++++--- 2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py index 4a6d2fe..ef15be5 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -448,12 +448,12 @@ class ModelTests(unittest.TestCase): 'kimchi-vm1', params)
inst.vm_stop('kimchi-vm1') - params = {'name': 'new-vm'} + params = {'name': u'пeω-∨м'} self.assertRaises(OperationFailed, inst.vm_update, 'kimchi-vm1', {'name': 'kimchi-vm2'}) inst.vm_update('kimchi-vm1', params) - self.assertEquals(info['uuid'], inst.vm_lookup('new-vm')['uuid']) - rollback.prependDefer(inst.vm_delete, 'new-vm') + self.assertEquals(info['uuid'], inst.vm_lookup(u'пeω-∨м')['uuid']) + rollback.prependDefer(inst.vm_delete, u'пeω-∨м')
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_network(self): diff --git a/tests/test_rest.py b/tests/test_rest.py index 0ed293b..da2e2b9 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- # # Project Kimchi # @@ -201,11 +202,11 @@ class RestTests(unittest.TestCase): resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(405, resp.status)
- req = json.dumps({'name': 'vm-updated'}) + req = json.dumps({'name': u'∨м-црdαtеd'}) resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(303, resp.status) - vm = json.loads(self.request('/vms/vm-updated', req).read()) - self.assertEquals('vm-updated', vm['name']) + vm = json.loads(self.request('/vms/∨м-црdαtеd', req).read()) + self.assertEquals(u'∨м-црdαtеd', vm['name'])
def test_vm_lifecycle(self): # Create a Template
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> change kīмkhī to kīмсhī kīмсhī looks like the acssii "kimchi" Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_model.py b/tests/test_model.py index ef15be5..28480fa 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -623,17 +623,17 @@ class ModelTests(unittest.TestCase): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') - params = {'name': u'kīмkhī-∨м', 'template': u'/templates/test'} + params = {'name': u'kīмсhī-∨м', 'template': u'/templates/test'} inst.vms_create(params) - rollback.prependDefer(inst.vm_delete, u'kīмkhī-∨м') + rollback.prependDefer(inst.vm_delete, u'kīмсhī-∨м') - inst.vm_start(u'kīмkhī-∨м') - rollback.prependDefer(inst.vm_stop, u'kīмkhī-∨м') + inst.vm_start(u'kīмсhī-∨м') + rollback.prependDefer(inst.vm_stop, u'kīмсhī-∨м') - inst.vm_delete(u'kīмkhī-∨м') + inst.vm_delete(u'kīмсhī-∨м') vms = inst.vms_get_list() - self.assertFalse(u'kīмkhī-∨м' in vms) + self.assertFalse(u'kīмсhī-∨м' in vms) @unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_list_sorted(self): -- 1.8.4.2
On 02/10/2014 05:48 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
change kīмkhī to kīмсhī kīмсhī looks like the acssii "kimchi"
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py index ef15be5..28480fa 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -623,17 +623,17 @@ class ModelTests(unittest.TestCase): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')
- params = {'name': u'kīмkhī-∨м', 'template': u'/templates/test'} + params = {'name': u'kīмсhī-∨м', 'template': u'/templates/test'} inst.vms_create(params) - rollback.prependDefer(inst.vm_delete, u'kīмkhī-∨м') + rollback.prependDefer(inst.vm_delete, u'kīмсhī-∨м')
- inst.vm_start(u'kīмkhī-∨м') - rollback.prependDefer(inst.vm_stop, u'kīмkhī-∨м') + inst.vm_start(u'kīмсhī-∨м') + rollback.prependDefer(inst.vm_stop, u'kīмсhī-∨м')
- inst.vm_delete(u'kīмkhī-∨м') + inst.vm_delete(u'kīмсhī-∨м')
vms = inst.vms_get_list() - self.assertFalse(u'kīмkhī-∨м' in vms) + self.assertFalse(u'kīмсhī-∨м' in vms)
Sorry, but I could not see any differences in the lines added and removed
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_list_sorted(self):
On 02/10/2014 11:27 PM, Aline Manera wrote:
On 02/10/2014 05:48 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
change kīмkhī to kīмсhī kīмсhī looks like the acssii "kimchi"
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py index ef15be5..28480fa 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -623,17 +623,17 @@ class ModelTests(unittest.TestCase): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') - params = {'name': u'kīмkhī-∨м', 'template': u'/templates/test'} + params = {'name': u'kīмсhī-∨м', 'template': u'/templates/test'} inst.vms_create(params) - rollback.prependDefer(inst.vm_delete, u'kīмkhī-∨м') + rollback.prependDefer(inst.vm_delete, u'kīмсhī-∨м') - inst.vm_start(u'kīмkhī-∨м') - rollback.prependDefer(inst.vm_stop, u'kīмkhī-∨м') + inst.vm_start(u'kīмсhī-∨м') + rollback.prependDefer(inst.vm_stop, u'kīмсhī-∨м') - inst.vm_delete(u'kīмkhī-∨м') + inst.vm_delete(u'kīмсhī-∨м') vms = inst.vms_get_list() - self.assertFalse(u'kīмkhī-∨м' in vms) + self.assertFalse(u'kīмсhī-∨м' in vms)
Sorry, but I could not see any differences in the lines added and removed The difference is "k" -> "с"
'kīмkhī ' do not looks like "kimchi" 'kīмсhī ' looks like "kimchi" so sorry, actually 'kīмkhī ' is a typo in one of my former commit. I want to use "kimchi" instead of 'kīмkhī '
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_list_sorted(self):
-- Thanks and best regards! Sheldon Feng(冯少合)<shaohef@linux.vnet.ibm.com> IBM Linux Technology Center
On 02/10/2014 01:32 PM, Sheldon wrote:
On 02/10/2014 11:27 PM, Aline Manera wrote:
On 02/10/2014 05:48 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
change kīмkhī to kīмсhī kīмсhī looks like the acssii "kimchi"
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py index ef15be5..28480fa 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -623,17 +623,17 @@ class ModelTests(unittest.TestCase): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test') - params = {'name': u'kīмkhī-∨м', 'template': u'/templates/test'} + params = {'name': u'kīмсhī-∨м', 'template': u'/templates/test'} inst.vms_create(params) - rollback.prependDefer(inst.vm_delete, u'kīмkhī-∨м') + rollback.prependDefer(inst.vm_delete, u'kīмсhī-∨м') - inst.vm_start(u'kīмkhī-∨м') - rollback.prependDefer(inst.vm_stop, u'kīмkhī-∨м') + inst.vm_start(u'kīмсhī-∨м') + rollback.prependDefer(inst.vm_stop, u'kīмсhī-∨м') - inst.vm_delete(u'kīмkhī-∨м') + inst.vm_delete(u'kīмсhī-∨м') vms = inst.vms_get_list() - self.assertFalse(u'kīмkhī-∨м' in vms) + self.assertFalse(u'kīмсhī-∨м' in vms)
Sorry, but I could not see any differences in the lines added and removed The difference is "k" -> "с"
'kīмkhī ' do not looks like "kimchi" 'kīмсhī ' looks like "kimchi"
so sorry, actually 'kīмkhī ' is a typo in one of my former commit. I want to use "kimchi" instead of 'kīмkhī '
Thanks for open my eyes! =)
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_list_sorted(self):
Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 02/10/2014 05:48 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
change kīмkhī to kīмсhī kīмсhī looks like the acssii "kimchi"
Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> --- tests/test_model.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tests/test_model.py b/tests/test_model.py index ef15be5..28480fa 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -623,17 +623,17 @@ class ModelTests(unittest.TestCase): inst.templates_create(params) rollback.prependDefer(inst.template_delete, 'test')
- params = {'name': u'kīмkhī-∨м', 'template': u'/templates/test'} + params = {'name': u'kīмсhī-∨м', 'template': u'/templates/test'} inst.vms_create(params) - rollback.prependDefer(inst.vm_delete, u'kīмkhī-∨м') + rollback.prependDefer(inst.vm_delete, u'kīмсhī-∨м')
- inst.vm_start(u'kīмkhī-∨м') - rollback.prependDefer(inst.vm_stop, u'kīмkhī-∨м') + inst.vm_start(u'kīмсhī-∨м') + rollback.prependDefer(inst.vm_stop, u'kīмсhī-∨м')
- inst.vm_delete(u'kīмkhī-∨м') + inst.vm_delete(u'kīмсhī-∨м')
vms = inst.vms_get_list() - self.assertFalse(u'kīмkhī-∨м' in vms) + self.assertFalse(u'kīмсhī-∨м' in vms)
@unittest.skipUnless(utils.running_as_root(), 'Must be run as root') def test_vm_list_sorted(self):
participants (3)
-
Aline Manera -
shaohef＠linux.vnet.ibm.com -
Sheldon