[Kimchi-devel] [PATCH 6/6 - V2] [Memory HotPlug] Fix test and adds slot test

[Aline Manera]

On 01/06/2015 16:49, Rodrigo Trujillo wrote:
>
>
> On 06/01/2015 09:41 AM, Aline Manera wrote:
>>
>>
>> On 28/05/2015 10:59, Rodrigo Trujillo wrote:
>>> This patch fixed the issues caused by previous changes in the guest 
>>> xml,
>>> like use of NUMA and MAXMEMORY elements.
>>> It includes a slot number checking test as well.
>>> This patch also changes mockModel to raise libvirterror to set
>>> mem_hotplug_support correctly.
>>>
>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>> ---
>>>   src/kimchi/mockmodel.py  | 7 +++++++
>>>   tests/test_model.py      | 2 +-
>>>   tests/test_rest.py       | 4 ++--
>>>   tests/test_vmtemplate.py | 5 ++++-
>>>   4 files changed, 14 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/kimchi/mockmodel.py b/src/kimchi/mockmodel.py
>>> index c81cabb..ab9f517 100644
>>> --- a/src/kimchi/mockmodel.py
>>> +++ b/src/kimchi/mockmodel.py
>>> @@ -182,6 +182,13 @@ class MockModel(Model):
>>>           old_xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)
>>>           root = objectify.fromstring(old_xml)
>>>           dev = objectify.fromstring(xml)
>>> +        # Libvirt < 1.2.14 does not support memory devices, so 
>>> force raise the
>>> +        # error, in order to get Capabilities.mem_hotplug_support 
>>> correctly in
>>> +        # test mode
>>> +        version = 1000000*1 + 1000*2 + 14
>>
>> Why do you are checking the libvirt verison if we have a feature test 
>> for it?
>
> Oh, I should have wrote in the comments:  I use the function 
> dom.attachDeviceFlags in the feature test,
> when we run the tests, it uses the mockmodel and calls a mock 
> "attachDeviceFlags" function.
> This snippet of code change this function to handle the feature test 
> properly, because it was never receiving
> the libvirt error and returning always True.

I think we override the attachDeviceFlags method because it is not 
supported by libvirt Test Driver.
So in this case we can assume an unique return value, for example, True, 
which means the user will be able to do memory hot plug using the mock 
environment.

>>
>>> +        if dev.tag == 'memory' and libvirt.getVersion() < version:
>>> +            raise 
>>> libvirt.libvirtError('virDomainAttachDeviceFlags() failed',
>>> +                                       dom=dom)
>>>           root.devices.append(dev)
>>>
>>>           MockModel._mock_vms[dom.name()] = ET.tostring(root, 
>>> encoding="utf-8")
>>> diff --git a/tests/test_model.py b/tests/test_model.py
>>> index 88c020e..7b6153a 100644
>>> --- a/tests/test_model.py
>>> +++ b/tests/test_model.py
>>> @@ -600,7 +600,7 @@ class ModelTests(unittest.TestCase):
>>>           inst = model.Model(None,
>>>                              objstore_loc=self.tmp_store)
>>>
>>> -        orig_params = {'name': 'test', 'memory': '1024', 'cpus': '1',
>>> +        orig_params = {'name': 'test', 'memory': 1024, 'cpus': 1,
>>>                          'cdrom': UBUNTU_ISO}
>>>           inst.templates_create(orig_params)
>>>
>>> diff --git a/tests/test_rest.py b/tests/test_rest.py
>>> index 914b602..64b3414 100644
>>> --- a/tests/test_rest.py
>>> +++ b/tests/test_rest.py
>>> @@ -201,7 +201,7 @@ class RestTests(unittest.TestCase):
>>>           resp = self.request('/vms/vm-1', req, 'PUT')
>>>           self.assertEquals(400, resp.status)
>>>
>>> -        params = {'name': u'∨м-црdαtеd', 'cpus': 5, 'memory': 4096}
>>> +        params = {'name': u'∨м-црdαtеd', 'cpus': 5, 'memory': 3072}
>>>           req = json.dumps(params)
>>>           resp = self.request('/vms/vm-1', req, 'PUT')
>>>           self.assertEquals(303, resp.status)
>>> @@ -1059,7 +1059,7 @@ class RestTests(unittest.TestCase):
>>>           keys = [u'libvirt_stream_protocols', u'qemu_stream', 
>>> u'qemu_spice',
>>>                   u'screenshot', u'system_report_tool', u'update_tool',
>>>                   u'repo_mngt_tool', u'federation', u'kernel_vfio', 
>>> u'auth',
>>> -                u'nm_running']
>>> +                u'nm_running', u'mem_hotplug_support']
>>>           self.assertEquals(sorted(keys), sorted(conf.keys()))
>>>
>>>       def test_peers(self):
>>> diff --git a/tests/test_vmtemplate.py b/tests/test_vmtemplate.py
>>> index b504fbc..7304220 100644
>>> --- a/tests/test_vmtemplate.py
>>> +++ b/tests/test_vmtemplate.py
>>> @@ -83,7 +83,8 @@ class VMTemplateTests(unittest.TestCase):
>>>       def test_to_xml(self):
>>>           graphics = {'type': 'spice', 'listen': '127.0.0.1'}
>>>           vm_uuid = str(uuid.uuid4()).replace('-', '')
>>> -        t = VMTemplate({'name': 'test-template', 'cdrom': self.iso})
>>> +        t = VMTemplate({'name': 'test-template', 'cdrom': self.iso,
>>> +                       'max_memory': 3072 << 10})
>>>           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])
>>> @@ -91,6 +92,8 @@ class VMTemplateTests(unittest.TestCase):
>>>           self.assertEquals(graphics['type'], xpath_get_text(xml, 
>>> expr)[0])
>>>           expr = "/domain/devices/graphics/@listen"
>>>           self.assertEquals(graphics['listen'], xpath_get_text(xml, 
>>> expr)[0])
>>> +        expr = "/domain/maxMemory/@slots"
>>> +        self.assertEquals('2', xpath_get_text(xml, expr)[0])
>>>
>>>       def test_arg_merging(self):
>>>           """
>>
>