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

[Aline Manera]

On 04/16/2014 03:30 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 | 17 ++---------------
>   src/kimchi/i18n.py        |  1 -
>   src/kimchi/model/vms.py   | 24 +++++++++++++++---------
>   3 files changed, 17 insertions(+), 25 deletions(-)
>
> diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py
> index ea810e4..77e6d8b 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", "users", "groups"]
> +        self.update_params = ["name", "users", "groups", "cpus", "memory"]
>           self.screenshot = VMScreenShot(model, ident)
>           self.uri_fmt = '/vms/%s'
>           for ident, node in sub_nodes.items():
> @@ -44,20 +44,7 @@ class VM(Resource):
>
>       @property
>       def data(self):
> -        return {'name': self.ident,
> -                'uuid': self.info['uuid'],
> -                'stats': self.info['stats'],
> -                'memory': self.info['memory'],
> -                'cpus': self.info['cpus'],
> -                'state': self.info['state'],
> -                'screenshot': self.info['screenshot'],
> -                'icon': self.info['icon'],
> -                'graphics': {'type': self.info['graphics']['type'],
> -                             'listen': self.info['graphics']['listen'],
> -                             'port': self.info['graphics']['port']},
> -                'users': self.info['users'],
> -                'groups': self.info['groups']
> -                }
> +        return self.info
>
>
>   class VMScreenShot(Resource):
> diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py
> index 15cfd16..5207183 100644
> --- a/src/kimchi/i18n.py
> +++ b/src/kimchi/i18n.py
> @@ -86,7 +86,6 @@ messages = {
>       "KCHVM0026E": _("Group name must be a string"),
>       "KCHVM0027E": _("User %(user)s does not exist"),
>       "KCHVM0028E": _("Group %(group)s does not exist"),

> -    "KCHVM0029E": _("Unable to update virtual machine in running state. You must power it off before."),

You added this message in previous patch. Why are you removing it now?

>       "KCHVMIF0001E": _("Interface %(iface)s does not exist in virtual machine %(name)s"),
>       "KCHVMIF0002E": _("Network %(network)s specified for virtual machine %(name)s does not exist"),
> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
> index 90e9537..a694517 100644
> --- a/src/kimchi/model/vms.py
> +++ b/src/kimchi/model/vms.py
> @@ -51,7 +51,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 = {}
> @@ -255,7 +257,6 @@ class VMModel(object):
>
>       def _static_vm_update(self, dom, params):
>           state = DOM_STATE_MAP[dom.info()[0]]
> -
>           old_xml = new_xml = dom.XMLDesc(0)
>
>           metadata_xpath = "/domain/metadata/kimchi/access/%s"
> @@ -275,6 +276,12 @@ class VMModel(object):
>                   groups = val
>               else:
>                   if key in VM_STATIC_UPDATE_PARAMS:
> +                    if key == 'memory':
> +                        # Libvirt saves memory in KiB. Retrieved xml has memory
> +                        # in KiB too, so new valeu must be in KiB here
> +                        val = val * 1024

> +                    if type(val) == int:
> +                        val = str(val)

Why do you need this?
If it is because the following function (xmlutils.xml_item_update) 
expected a string you should always do this convertion

new_xml = xmlutils.xml_item_update(new_xml, xpath, str(val))

>                       xpath = VM_STATIC_UPDATE_PARAMS[key]
>                       new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
>
> @@ -285,21 +292,19 @@ class VMModel(object):
>                       msg_args = {'name': dom.name(), 'new_name': params['name']}
>                       raise InvalidParameter("KCHVM0003E", msg_args)
>
> -                # Undefine old vm and create a new one with updated values
> +                # Undefine old vm, only if name is going to change
>                   dom.undefine()
> -                conn = self.conn.get()
> -                dom = conn.defineXML(new_xml)
>
> -            # Update metadata element
>               root = ET.fromstring(new_xml)
> +            root.remove(root.find('.currentMemory'))
> +            # Update metadata element
>               current_metadata = root.find('metadata')
>               new_metadata = self._get_metadata_node(users, groups)
>               if current_metadata is not None:
>                   root.replace(current_metadata, new_metadata)
>               else:
>                   root.append(new_metadata)
> -            dom = conn.defineXML(ET.tostring(root))
> -
> +            dom = conn.defineXML(ET.tostring(root, encoding="utf-8"))
>           except libvirt.libvirtError as e:
>               dom = conn.defineXML(old_xml)
>               raise OperationFailed("KCHVM0008E", {'name': dom.name(),
> @@ -350,7 +355,8 @@ class VMModel(object):
>           users = xpath_get_text(xml, "/domain/metadata/kimchi/access/user")
>           groups = xpath_get_text(xml, "/domain/metadata/kimchi/access/group")
>
> -        return {'state': state,
> +        return {'name': name,
> +                'state': state,
>                   'stats': res,
>                   'uuid': dom.UUIDString(),
>                   'memory': info[2] >> 10,