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

[shaohef at linux.vnet.ibm.com]

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 | 68 +++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py
index ede54a3..d0b456c 100644
--- a/src/kimchi/model/vms.py
+++ b/src/kimchi/model/vms.py
@@ -35,6 +35,8 @@ 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_vm_metadata_element
+from kimchi.model.utils import set_vm_metadata_element
 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
@@ -252,35 +254,38 @@ class VMModel(object):
         for group in groups:
             access.append(E.group(group))
 
-        return E.metadata(E.kimchi(access))
+        return access
+
+    def _vm_updata_access_metadata(self, dom, params):
+        access_xml = (get_vm_metadata_element(dom, "access") or
+                      """<access></access>""")
+        old_users = xpath_get_text(access_xml, "/access/user")
+        old_groups = xpath_get_text(access_xml, "/access/group")
+        users = params.get("users")
+        groups = params.get("groups")
+        if users is None and groups is None:
+            return
+        users = old_users if users is None else users
+        groups = old_groups if groups is None else groups
+        for user in users:
+            if not User(user).exists():
+                raise OperationFailed("KCHVM0027E", {'user': user})
+        for group in groups:
+            if not Group(group).exists():
+                raise OperationFailed("KCHVM0028E", {'group': group})
+
+        xml = self._get_metadata_node(users, groups)
+        set_vm_metadata_element(dom, ET.tostring(xml), "all")
 
     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:
-                    xpath = VM_STATIC_UPDATE_PARAMS[key]
-                    new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
+            if key in VM_STATIC_UPDATE_PARAMS:
+                xpath = VM_STATIC_UPDATE_PARAMS[key]
+                new_xml = xmlutils.xml_item_update(new_xml, xpath, val)
 
         conn = self.conn.get()
         try:
@@ -294,16 +299,6 @@ class VMModel(object):
                 conn = self.conn.get()
                 dom = conn.defineXML(new_xml)
 
-            # Update metadata element
-            root = ET.fromstring(new_xml)
-            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))
-
         except libvirt.libvirtError as e:
             dom = conn.defineXML(old_xml)
             raise OperationFailed("KCHVM0008E", {'name': dom.name(),
@@ -311,7 +306,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))
@@ -350,9 +345,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_vm_metadata_element(dom, "access") or
+                      """<access></access>""")
+        users = xpath_get_text(access_xml, "/access/user")
+        groups = xpath_get_text(access_xml, "/access/group")
 
         return {'state': state,
                 'stats': res,
-- 
1.9.0