[Kimchi-devel] [PATCH v5] Backend support for templates with sockets, cores, and threads

[Christy Perez]

On 10/15/2014 02:46 AM, Zhou Zheng Sheng wrote:
> on 2014/10/15 03:55, Christy Perez wrote:
>> It turns out the tests are failing because of that cast to dict() that I
>> took out in the last patchset. *That* is why I had put it in. It seems
>> like the tests need some tweaking if I don't want to have to cast that
>> to a dict(). Any preferences on whether to update a bunch of tests, or
>> just have a cast for now?
>>
> 
> Since I suggested you remove the dict() call, I'm curious about the
> error. So I tried to run "make check". I got some exceptions as follow.
> 
> ERROR: test_template_storage_customise (test_model.ModelTests)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/edward/src/kimchi/tests/test_model.py", line 629, in
> test_template_storage_customise
>     'test', params)
>   File "/usr/lib64/python2.7/unittest/case.py", line 513, in assertRaises
>     callableObj(*args, **kwargs)
>   File "/home/edward/src/kimchi/src/kimchi/model/templates.py", line
> 186, in update
>     if not self._validate_updated_cpu_params(new_t):
>   File "/home/edward/src/kimchi/src/kimchi/model/templates.py", line
> 226, in _validate_updated_cpu_params
>     topology = cpu_info.get('topology')
> AttributeError: 'unicode' object has no attribute 'get'
> 
> 
> 
> ERROR: test_delete_running_vm (test_model.ModelTests)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/edward/src/kimchi/tests/test_model.py", line 1155, in
> test_delete_running_vm
>     inst.vms_create(params)
>   File "/home/edward/src/kimchi/src/kimchi/model/vms.py", line 215, in
> create
>     volumes=vol_list)
>   File "/home/edward/src/kimchi/src/kimchi/vmtemplate.py", line 386, in
> to_vm_xml
>     params['cpu_info'] = self._get_cpu_xml()
>   File "/home/edward/src/kimchi/src/kimchi/vmtemplate.py", line 366, in
> _get_cpu_xml
>     cpu_topo = cpu_info.get('topology')
> AttributeError: 'unicode' object has no attribute 'get'
> 
> 
> I think they were because cpu_info was set to an empty string in
> "TemplatesModel.create()", when there is no cpu_info.
> 
> In "VMTemplate._get_cpu_xml()" and
> "TemplateModel._validate_updated_cpu_params()", it checks if cpu_info is
> None, so it can safely use "cpu_info.get()" if it's not None. This means
> that other parts of the code assumes cpu_info a dict. However in
> "TemplatesModel.create()", it sets "params['cpu_info'] = ''", so it
> breaks the assumption. Although empty string is not None, but it does
> not provide dict-like access methods.
> 
> Maybe this is why the tests are failing. After I change it to
> "params['cpu_info'] = None", the tests can pass.
I'll look more at that. Thanks.

> 
>> On 10/14/2014 01:51 PM, Christy Perez wrote:
>>> I just ran the test suite on this, and there are some errors I need to
>>> look into, so hold off on reviewing this until I sent a v6.
>>>
>>> Thanks,
>>>
> 
> I also find that po and pot files are included in this patch. I think
> it's better to update po and pot files in a separated patch.
It's going to be hard to break them out, now. But I agree with you in
theory. :) I'll see what I can do for the next one.


>