[Kimchi-devel] [PATCH V9 5/7] bug fix: get user and group when vm is living.

[Royce Lv]

On 2014年04月25日 22:14, shaohef at linux.vnet.ibm.com wrote:
> From: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
>
> define a domain just edit the persistent xml.
> So we should get user and group from persistent xml instead of live xml
> when domain is living.
> Refacor the code by libvirt metadata API and fix the bug.
>
> Signed-off-by: ShaoHe Feng <shaohef at linux.vnet.ibm.com>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   src/kimchi/model/vms.py | 97 ++++++++++++++++++++++++++-----------------------
>   1 file changed, 52 insertions(+), 45 deletions(-)
>
> diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
> index 8a79f0f..8def756 100644
> --- a/src/kimchi/model/vms.py
> +++ b/src/kimchi/model/vms.py
> @@ -25,7 +25,6 @@ from xml.etree import ElementTree
>
>   import libvirt
>   from cherrypy.process.plugins import BackgroundTask
> -from lxml.builder import E
>
>   from kimchi import vnc
>   from kimchi import xmlutils
> @@ -35,6 +34,9 @@ from kimchi.exception import NotFoundError, OperationFailed
>   from kimchi.model.config import CapabilitiesModel
>   from kimchi.model.templates import TemplateModel
>   from kimchi.model.utils import get_vm_name
> +from kimchi.model.utils import get_metadata_node
> +from kimchi.model.utils import set_metadata_node
> +from kimchi.model.utils import create_metadata_node
>   from kimchi.screenshot import VMScreenshot
>   from kimchi.utils import import_class, kimchi_log, run_setfacl_set_attr
>   from kimchi.utils import template_name_from_uri
> @@ -247,48 +249,59 @@ class VMModel(object):
>           self._live_vm_update(dom, params)
>           return dom.name().decode('utf-8')
>
> -    def _get_metadata_node(self, users, groups):
> -        access = E.access()
> +    def _get_metadata_access_node(self, users, groups):
> +        node = create_metadata_node("access")
> +        access = node.access()
>           for user in users:
> -            access.append(E.user(user))
> +            access.append(node.user(user))
>
>           for group in groups:
> -            access.append(E.group(group))
> -
> -        return E.metadata(E.kimchi(access))
> +            access.append(node.group(group))
> +
> +        return access
I would say the name is a little bit confusing with 'get_metadata', this 
part deals with Element and Etree, right? What about 'build_access_elem'?
> +
> +    def _vm_updata_access_metadata(self, dom, params):
typo _vm_update_access_metadata
> +        users = groups = None
> +        if "users" in params:
> +            users = params["users"]
little advice: users = params.get('users', [])
> +            invalid_users = set(users) - set(self.users.get_list())
> +            if len(invalid_users) != 0:
> +                raise InvalidParameter("KCHVM0027E",
> +                                       {'users': ", ".join(invalid_users)})
> +        if "groups" in params:
> +            groups = params["groups"]
> +            invalid_groups = set(groups) - set(self.groups.get_list())
> +            if len(invalid_groups) != 0:
> +                raise InvalidParameter("KCHVM0028E",
> +                                       {'groups': ", ".join(invalid_groups)})
> +
> +        if users is None and groups is None:
> +            return
> +
> +        access_xml = (get_metadata_node(dom, "access") or
> +                      """<access></access>""")
> +        old_users = xpath_get_text(access_xml, "/access/user")
> +        old_groups = xpath_get_text(access_xml, "/access/group")
> +        users = old_users if users is None else users
> +        groups = old_groups if groups is None else groups
> +
> +        node = self._get_metadata_access_node(users, groups)
> +        set_metadata_node(dom, ET.tostring(node))
>
>       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"
> -        users = xpath_get_text(old_xml, metadata_xpath % "user")
> -        groups = xpath_get_text(old_xml, metadata_xpath % "group")
> -
>           for key, val in params.items():
> -            if key == 'users':
> -                invalid_users = set(val) - set(self.users.get_list())
> -                if len(invalid_users) != 0:
> -                    raise InvalidParameter("KCHVM0027E",
> -                                           {'users': ", ".join(invalid_users)})
> -                users = val
> -            elif key == 'groups':
> -                invalid_groups = set(val) - set(self.groups.get_list())
> -                if len(invalid_groups) != 0:
> -                    raise InvalidParameter("KCHVM0028E",
> -                                           {'groups':
> -                                            ", ".join(invalid_groups)})
> -                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)
> -                    xpath = VM_STATIC_UPDATE_PARAMS[key]
> -                    new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
> +            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)
> +                xpath = VM_STATIC_UPDATE_PARAMS[key]
> +                new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
>
>           conn = self.conn.get()
>           try:
> @@ -302,13 +315,6 @@ class VMModel(object):
>
>               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, encoding="utf-8"))
>           except libvirt.libvirtError as e:
>               dom = conn.defineXML(old_xml)
> @@ -317,7 +323,7 @@ class VMModel(object):
>           return dom
>
>       def _live_vm_update(self, dom, params):
> -        pass
> +        self._vm_updata_access_metadata(dom, params)
>
>       def _has_video(self, dom):
>           dom = ElementTree.fromstring(dom.XMLDesc(0))
> @@ -356,9 +362,10 @@ class VMModel(object):
>           res['io_throughput'] = vm_stats.get('disk_io', 0)
>           res['io_throughput_peak'] = vm_stats.get('max_disk_io', 100)
>
> -        xml = dom.XMLDesc(0)
> -        users = xpath_get_text(xml, "/domain/metadata/kimchi/access/user")
> -        groups = xpath_get_text(xml, "/domain/metadata/kimchi/access/group")
> +        access_xml = (get_metadata_node(dom, "access") or
> +                      """<access></access>""")
> +        users = xpath_get_text(access_xml, "/access/user")
> +        groups = xpath_get_text(access_xml, "/access/group")
>
>           return {'name': name,
>                   'state': state,