On 04/28/2014 03:05 PM, Royce Lv wrote:
On 2014年04月25日 22:14, shaohef(a)linux.vnet.ibm.com wrote:
> From: ShaoHe Feng <shaohef(a)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(a)linux.vnet.ibm.com>
> Signed-off-by: Royce Lv <lvroyce(a)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'?
ACK.
> +
> + def _vm_updata_access_metadata(self, dom, params):
typo _vm_update_access_metadata
ACK
> + users = groups = None
> + if "users" in params:
> + users = params["users"]
little advice: users = params.get('users', [])
when "users" if
not in params, that means users is None, we will not
change the meta xml.
when "users" is [], we will clean all users in the meta xml
> + 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,
--
Thanks and best regards!
Sheldon Feng(冯少合)<shaohef(a)linux.vnet.ibm.com>
IBM Linux Technology Center