[Kimchi-devel] [PATCH V3 2/3] VM Edit CPU/Memory: (Backend) Changes VM control and model

[Rodrigo Trujillo]

On 04/15/2014 03:45 PM, Rodrigo Trujillo wrote:
> On 04/14/2014 05:15 PM, Aline Manera wrote:
>> On 04/11/2014 03:38 PM, Rodrigo Trujillo wrote:
>>> This patch changes vm control file in order to allow user to change 
>>> cpus
>>> and memory. It also changes vms model in order to change xml properly
>>> and test if vm is running.
>>>
>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>> ---
>>>   src/kimchi/control/vms.py |  4 ++--
>>>   src/kimchi/model/vms.py   | 43 
>>> +++++++++++++++++++++++++++----------------
>>>   2 files changed, 29 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
>>> index e75b27f..0964fb7 100644
>>> --- a/src/kimchi/control/vms.py
>>> +++ b/src/kimchi/control/vms.py
>>> @@ -32,7 +32,7 @@ class VMs(Collection):
>>>   class VM(Resource):
>>>       def __init__(self, model, ident):
>>>           super(VM, self).__init__(model, ident)
>>> -        self.update_params = ["name"]
>>> +        self.update_params = ["name", "cpus", "memory"]
>>>           self.screenshot = VMScreenShot(model, ident)
>>>           self.uri_fmt = '/vms/%s'
>>>           for ident, node in sub_nodes.items():
>>> @@ -43,7 +43,7 @@ class VM(Resource):
>>>
>>>       @property
>>>       def data(self):
>>> -        return {'name': self.ident,
>>> +        return {'name': self.info['name'],
>>>                   'uuid': self.info['uuid'],
>>>                   'stats': self.info['stats'],
>>>                   'memory': self.info['memory'],
>>
>> Could we return the whole self.info object instead of listing item by 
>> item?
>>
>> @property
>> def data(self):
>>     return self.info
>>
>> Or there is something in self.info that should not be returned?
>>
>>
> Yes, I think it is possible to return only info. I am going to check
>
>>> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
>>> index 2425a43..35fbf3e 100644
>>> --- a/src/kimchi/model/vms.py
>>> +++ b/src/kimchi/model/vms.py
>>> @@ -47,7 +47,9 @@ DOM_STATE_MAP = {0: 'nostate',
>>>                    6: 'crashed'}
>>>
>>>   GUESTS_STATS_INTERVAL = 5
>>> -VM_STATIC_UPDATE_PARAMS = {'name': './name'}
>>> +VM_STATIC_UPDATE_PARAMS = {'name': './name',
>>> +                           'cpus': './vcpu',
>>> +                           'memory': './memory'}
>>>   VM_LIVE_UPDATE_PARAMS = {}
>>>
>>>   stats = {}
>>> @@ -235,33 +237,41 @@ class VMModel(object):
>>>
>>>       def update(self, name, params):
>>>           dom = self.get_vm(name, self.conn)
>>
>>> +        # Change memory unit from MB to KB
>>
>> Why?
>>
> Wrong comment I forgot to remove
>
>>>           dom = self._static_vm_update(dom, params)
>>>           self._live_vm_update(dom, params)
>>>           return dom.name().decode('utf-8')
>>>
>>>       def _static_vm_update(self, dom, params):
>>>           state = DOM_STATE_MAP[dom.info()[0]]
>>> -
>>>           old_xml = new_xml = dom.XMLDesc(0)
>>>
>>>           for key, val in params.items():
>>>               if key in VM_STATIC_UPDATE_PARAMS:
>>> +                # Machine must be powered off
>>> +                if state == 'running':
>>> +                    raise InvalidOperation("KCHVM0022E")
>>>                   xpath = VM_STATIC_UPDATE_PARAMS[key]
>>
>>> +                if key == 'memory':
>>> +                    val = val * 1024
>>
>> Could we save the memory in MB?
> This memory parameter comes from VM xml, which are being created using 
> KB. Yes, it is possible to save in MB,
> but we need to change in all VM create API. Do you know why KB was 
> chosen in the beginning of the project ?

Aline, according to our conversation and double check: Kimchi always 
save in MB, however, libvirt
transforms the value in KiB... and we get in Kib when dump the xml. So, 
we have to transform the values.

 From libvirt documentation:
"... the value will be rounded up to the nearest kibibyte by libvirt, 
and may be further rounded to the granularity supported by the hypervisor. "


>
>>
>>> +                if type(val) == int:
>>> +                    val = str(val)
>>>                   new_xml = xmlutils.xml_item_update(new_xml, xpath, 
>>> val)
>>>
>>> -        try:
>>> -            if 'name' in params:
>>> -                if state == 'running':
>>> -                    msg_args = {'name': dom.name(), 'new_name': 
>>> params['name']}
>>> -                    raise InvalidParameter("KCHVM0003E", msg_args)
>>> -                else:
>>> -                    dom.undefine()
>>> -            conn = self.conn.get()
>>> -            dom = conn.defineXML(new_xml)
>>> -        except libvirt.libvirtError as e:
>>> -            dom = conn.defineXML(old_xml)
>>> -            raise OperationFailed("KCHVM0008E", {'name': dom.name(),
>>> -                                                 'err': 
>>> e.get_error_message()})
>>> +        if not new_xml == old_xml:
>>> +            # Remove currentMemory element
>>> +            root = ElementTree.fromstring(new_xml)
>>> +            root.remove(root.find('.currentMemory'))
>>> +            new_xml = ElementTree.tostring(root, encoding="utf-8")
>>> +            try:
>>> +                conn = self.conn.get()
>>> +                dom.undefine()
>>> +                dom = conn.defineXML(new_xml)
>>> +            except libvirt.libvirtError as e:
>>> +                dom = conn.defineXML(old_xml)
>>> +                raise OperationFailed("KCHVM0008E",
>>> +                                      {'name': dom.name(),
>>> +                                       'err': e.get_error_message()})
>>>           return dom
>>>
>>>       def _live_vm_update(self, dom, params):
>>> @@ -304,7 +314,8 @@ class VMModel(object):
>>>           res['io_throughput'] = vm_stats.get('disk_io', 0)
>>>           res['io_throughput_peak'] = vm_stats.get('max_disk_io', 100)
>>>
>>> -        return {'state': state,
>>> +        return {'name': name,
>>> +                'state': state,
>>>                   'stats': res,
>>>                   'uuid': dom.UUIDString(),
>>>                   'memory': info[2] >> 10,
>>
>