[PATCH V3 0/3] bug fix: get user and group when VM is running

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> V2 -> V3: move the virDomain.metadata and virDomain.setMetadata to model/utils.py add testcase V1 -> V2: libvirt also support virDomain.metadata and virDomain.setMetadata two api. use virDomain.metadata to get the user and group. use virDomain.setMetadata to set the user and group. ShaoHe Feng (3): Add two function to set and get domain xml metadata bug fix: get user and group when vm is living. update test case to set/get user and group when VM is running src/kimchi/i18n.py | 1 + src/kimchi/model/utils.py | 41 ++++++++++++++++++++++++++++++ src/kimchi/model/vms.py | 65 +++++++++++++++++++++++------------------------ tests/test_model.py | 13 ++++++++++ 4 files changed, 87 insertions(+), 33 deletions(-) -- 1.9.0

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Libvirt support two API to set and get domain xml metadata. These two function wrap them. Put them in model/utils.py, so vm_storage and vm_iface can make use of them. For set domain xml metadata, we set live and config xml by default. For get domain xml metadata, we get the current xml by default. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/i18n.py | 1 + src/kimchi/model/utils.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 3fc3013..28b1c4d 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -86,6 +86,7 @@ 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 get access metadata of virtual machine %(name)s. Details: %(err)s"), "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/utils.py b/src/kimchi/model/utils.py index 80e1801..51acb68 100644 --- a/src/kimchi/model/utils.py +++ b/src/kimchi/model/utils.py @@ -18,6 +18,11 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA from kimchi.exception import OperationFailed +import libvirt +from lxml import etree + + +KIMCHI_META_URL = "https://github.com/kimchi-project/kimchi/metadata/" def get_vm_name(vm_name, t_name, name_list): @@ -28,3 +33,39 @@ def get_vm_name(vm_name, t_name, name_list): if vm_name not in name_list: return vm_name raise OperationFailed("KCHUTILS0003E") + + +def get_vm_config_flag(dom, mode="persistent"): + # libvirt.VIR_DOMAIN_AFFECT_CURRENT is 0 + # VIR_DOMAIN_AFFECT_LIVE is 1, VIR_DOMAIN_AFFECT_CONFIG is 2 + flag = {"live": libvirt.VIR_DOMAIN_AFFECT_LIVE, + "persistent": libvirt.VIR_DOMAIN_AFFECT_CONFIG, + "current": libvirt.VIR_DOMAIN_AFFECT_CURRENT, + "all": libvirt.VIR_DOMAIN_AFFECT_CONFIG + + libvirt.VIR_DOMAIN_AFFECT_LIVE if dom.isActive() and + dom.isPersistent() else libvirt.VIR_DOMAIN_AFFECT_CURRENT} + + return flag[mode] + + +def set_vm_metadata_element(dom, meta_xml, mode="all"): + element = etree.fromstring(meta_xml) + # From libvirt doc, Passing None for @metadata says to remove that element + # from the domain XML (passing the empty string leaves the element present) + # Do not support remove the old metadata + dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, meta_xml, + "kimchi", KIMCHI_META_URL + element.tag, + flags=get_vm_config_flag(dom, mode)) + + +def get_vm_metadata_element(dom, element, mode="current"): + try: + return dom.metadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, + KIMCHI_META_URL + element, + flags=get_vm_config_flag(dom, mode)) + except libvirt.libvirtError as e: + if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN_METADATA: + return "" + else: + raise OperationFailed("KCHVM0029E", {'name': dom.name(), + 'err': e.message}) -- 1.9.0

From: ShaoHe Feng <shaohef@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@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 65 ++++++++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 90e9537..652725a 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -36,6 +36,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 kimchi_log, run_setfacl_set_attr from kimchi.utils import template_name_from_uri @@ -251,32 +253,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': - for user in val: - if not User(user).exists(): - raise OperationFailed("KCHVM0027E", {'user': user}) - users = val - elif key == 'groups': - for group in val: - if not Group(group).exists(): - raise OperationFailed("KCHVM0028E", {'group': group}) - 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: @@ -290,16 +298,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(), @@ -307,7 +305,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)) @@ -346,9 +344,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

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> When vm is live, we should also get user and group correctly. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- tests/test_model.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_model.py b/tests/test_model.py index 357d969..0dafc6d 100644 --- a/tests/test_model.py +++ b/tests/test_model.py @@ -601,6 +601,19 @@ class ModelTests(unittest.TestCase): self.assertRaises(InvalidParameter, inst.vm_update, 'kimchi-vm1', params) + # change VM users and groups, when wm is running. + inst.vm_update(u'kimchi-vm1', + {'users': ['root'], 'groups': ['root']}) + vm_info = inst.vm_lookup(u'kimchi-vm1') + self.assertEquals(['root'], vm_info['users']) + self.assertEquals(['root'], vm_info['groups']) + # change VM users and groups by removing all elements, + # when wm is running. + inst.vm_update(u'kimchi-vm1', {'users': [], 'groups': []}) + vm_info = inst.vm_lookup(u'kimchi-vm1') + self.assertEquals([], vm_info['users']) + self.assertEquals([], vm_info['groups']) + inst.vm_poweroff('kimchi-vm1') params = {'name': u'пeω-∨м'} self.assertRaises(OperationFailed, inst.vm_update, -- 1.9.0

From: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Then kimchi can choose the divice mode depend on these info. Note, the info is the initial OS when install OS on a blank guest. OS may change after the install. So other device should still keep their model info in the vm metadata. Signed-off-by: ShaoHe Feng <shaohef@linux.vnet.ibm.com> Signed-off-by: Royce Lv <lvroyce@linux.vnet.ibm.com> --- src/kimchi/model/vms.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 652725a..5919032 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -221,6 +221,8 @@ class VMsModel(object): raise OperationFailed("KCHVM0007E", {'name': name, 'err': e.get_error_message()}) + VMModel.vm_updata_os_metadata(VMModel.get_vm(name, self.conn), t.info) + return name def get_list(self): @@ -276,6 +278,22 @@ class VMModel(object): xml = self._get_metadata_node(users, groups) set_vm_metadata_element(dom, ET.tostring(xml), "all") + @staticmethod + def vm_get_os_metadata(dom): + os_xml = (get_vm_metadata_element(dom, "OS") or + """<OS></OS>""") + OS = ET.fromstring(os_xml) + return (OS.attrib.get("version"), OS.attrib.get("distro")) + + @staticmethod + def vm_updata_os_metadata(dom, params): + distro = params.get("os_distro") + version = params.get("os_version") + if distro is None: + return + OS = E.os({"distro": distro, "version": version}) + set_vm_metadata_element(dom, ET.tostring(OS)) + def _static_vm_update(self, dom, params): state = DOM_STATE_MAP[dom.info()[0]] -- 1.9.0

On 04/22/2014 12:59 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng <shaohef@linux.vnet.ibm.com>
V2 -> V3: move the virDomain.metadata and virDomain.setMetadata to model/utils.py add testcase
V1 -> V2: libvirt also support virDomain.metadata and virDomain.setMetadata two api. use virDomain.metadata to get the user and group. use virDomain.setMetadata to set the user and group.
ShaoHe Feng (3): Add two function to set and get domain xml metadata bug fix: get user and group when vm is living. update test case to set/get user and group when VM is running
src/kimchi/i18n.py | 1 + src/kimchi/model/utils.py | 41 ++++++++++++++++++++++++++++++ src/kimchi/model/vms.py | 65 +++++++++++++++++++++++------------------------ tests/test_model.py | 13 ++++++++++ 4 files changed, 87 insertions(+), 33 deletions(-)
As I commented in previous version, virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions ---------------------------------------- During the first development, I noticed the virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions. While using those functions, some libvirt versions will report: libvir: QEMU Driver error : argument unsupported: QEMU driver does not support<metadata> element But if you manually get and set metadata element, libvirt works fine.

On 04/22/2014 01:59 PM, Aline Manera wrote:
On 04/22/2014 12:59 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng<shaohef@linux.vnet.ibm.com>
V2 -> V3: move the virDomain.metadata and virDomain.setMetadata to model/utils.py add testcase
V1 -> V2: libvirt also support virDomain.metadata and virDomain.setMetadata two api. use virDomain.metadata to get the user and group. use virDomain.setMetadata to set the user and group.
ShaoHe Feng (3): Add two function to set and get domain xml metadata bug fix: get user and group when vm is living. update test case to set/get user and group when VM is running
src/kimchi/i18n.py | 1 + src/kimchi/model/utils.py | 41 ++++++++++++++++++++++++++++++ src/kimchi/model/vms.py | 65 +++++++++++++++++++++++------------------------ tests/test_model.py | 13 ++++++++++ 4 files changed, 87 insertions(+), 33 deletions(-)
As I commented in previous version, virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions
----------------------------------------
During the first development, I noticed the virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions.
While using those functions, some libvirt versions will report:
libvir: QEMU Driver error : argument unsupported: QEMU driver does not support<metadata> element
But if you manually get and set metadata element, libvirt works fine.
I agree with the other changes you proposed in this patch but you need to keep manually manage the metadata element until we get virDomain.metadata and virDomain.setMetadata working in all livirt versions used in the supported distros.
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 2014?04?23? 00:59, Aline Manera wrote:
On 04/22/2014 12:59 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng<shaohef@linux.vnet.ibm.com>
V2 -> V3: move the virDomain.metadata and virDomain.setMetadata to model/utils.py add testcase
V1 -> V2: libvirt also support virDomain.metadata and virDomain.setMetadata two api. use virDomain.metadata to get the user and group. use virDomain.setMetadata to set the user and group.
ShaoHe Feng (3): Add two function to set and get domain xml metadata bug fix: get user and group when vm is living. update test case to set/get user and group when VM is running
src/kimchi/i18n.py | 1 + src/kimchi/model/utils.py | 41 ++++++++++++++++++++++++++++++ src/kimchi/model/vms.py | 65 +++++++++++++++++++++++------------------------ tests/test_model.py | 13 ++++++++++ 4 files changed, 87 insertions(+), 33 deletions(-)
As I commented in previous version, virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions
----------------------------------------
During the first development, I noticed the virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions.
While using those functions, some libvirt versions will report:
libvir: QEMU Driver error : argument unsupported: QEMU driver does not support<metadata> element
But if you manually get and set metadata element, libvirt works fine.
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel I went through libvirt log I find:
commit 21d13ddc5d2e1ba24fec401fd7aec0f33f0fecf0 Author: Peter Krempa <pkrempa@redhat.com> Date: Wed Feb 1 14:03:52 2012 +0100 qemu: Add support for virDomainGetMetadata and virDomainSetMetadata And in this patch: + if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(persistentDef->description); + if (metadata && + !(persistentDef->description = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(persistentDef->title); + if (metadata && + !(persistentDef->title = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_ELEMENT: + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU driver does not support" + "<metadata> element")); + goto cleanup; So in first version METADATA_ELEMENT is not supported for set and get, and late after release 1.1.2 released(my ubuntu 13.10 is 1.1.1-0ubuntu8.5), metadata_element is handled: commit ac38bff077642daa17f9a82480062ebef4c11a7b Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Sep 6 17:34:43 2013 +0200 conf: Add support for requesting of XML metadata via the API So instead of using VIR_DOMAIN_METADATA_ELEMENT, I guess we can use metadata_description, so that we can set this in live time, or we can do as Aline did as a workaround until release 1.1.2 is on all of our supported version? What do your say?

On 04/23/2014 12:21 AM, Royce Lv wrote:
On 2014?04?23? 00:59, Aline Manera wrote:
On 04/22/2014 12:59 AM, shaohef@linux.vnet.ibm.com wrote:
From: ShaoHe Feng<shaohef@linux.vnet.ibm.com>
V2 -> V3: move the virDomain.metadata and virDomain.setMetadata to model/utils.py add testcase
V1 -> V2: libvirt also support virDomain.metadata and virDomain.setMetadata two api. use virDomain.metadata to get the user and group. use virDomain.setMetadata to set the user and group.
ShaoHe Feng (3): Add two function to set and get domain xml metadata bug fix: get user and group when vm is living. update test case to set/get user and group when VM is running
src/kimchi/i18n.py | 1 + src/kimchi/model/utils.py | 41 ++++++++++++++++++++++++++++++ src/kimchi/model/vms.py | 65 +++++++++++++++++++++++------------------------ tests/test_model.py | 13 ++++++++++ 4 files changed, 87 insertions(+), 33 deletions(-)
As I commented in previous version, virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions
----------------------------------------
During the first development, I noticed the virDomain.metadata and virDomain.setMetadata does not work well in all libvirt versions.
While using those functions, some libvirt versions will report:
libvir: QEMU Driver error : argument unsupported: QEMU driver does not support<metadata> element
But if you manually get and set metadata element, libvirt works fine.
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel I went through libvirt log I find:
commit 21d13ddc5d2e1ba24fec401fd7aec0f33f0fecf0 Author: Peter Krempa <pkrempa@redhat.com> Date: Wed Feb 1 14:03:52 2012 +0100
qemu: Add support for virDomainGetMetadata and virDomainSetMetadata
And in this patch:
+ if (flags & VIR_DOMAIN_AFFECT_CONFIG) { + switch ((virDomainMetadataType) type) { + case VIR_DOMAIN_METADATA_DESCRIPTION: + VIR_FREE(persistentDef->description); + if (metadata && + !(persistentDef->description = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_TITLE: + VIR_FREE(persistentDef->title); + if (metadata && + !(persistentDef->title = strdup(metadata))) + goto no_memory; + break; + case VIR_DOMAIN_METADATA_ELEMENT: + qemuReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("QEMU driver does not support" + "<metadata> element")); + goto cleanup;
So in first version METADATA_ELEMENT is not supported for set and get, and late after release 1.1.2 released(my ubuntu 13.10 is 1.1.1-0ubuntu8.5), metadata_element is handled:
commit ac38bff077642daa17f9a82480062ebef4c11a7b Author: Peter Krempa <pkrempa@redhat.com> Date: Fri Sep 6 17:34:43 2013 +0200
conf: Add support for requesting of XML metadata via the API
So instead of using VIR_DOMAIN_METADATA_ELEMENT, I guess we can use metadata_description, so that we can set this in live time, or we can do as Aline did as a workaround until release 1.1.2 is on all of our supported version?
What do your say?
Good investigation, Royce! VIR_DOMAIN_METADATA_DESCRIPTION handles the <description> tag and VIR_DOMAIN_METADATA_TITLE handles the <title> tag So I don't think we can use it for our proposal.
participants (3)
-
Aline Manera
-
Royce Lv
-
shaohef@linux.vnet.ibm.com