[Kimchi-devel] [PATCH 1/4] Remove manual <metadata> manupulation

All the latest version of the major Linux distributions are now using the libvirt which has support to <metadata> manupulation through methods virDomain.setMetadata() and virDomain.metadata() So remove the manual manupulation to make the code maintenance easier. Signed-off-by: Aline Manera <alinefm(a)linux.vnet.ibm.com&gt; --- src/kimchi/i18n.py | 1 - src/kimchi/model/config.py | 2 - src/kimchi/model/featuretests.py | 24 ----------- src/kimchi/model/utils.py | 93 ++++++++-------------------------------- src/kimchi/model/vmifaces.py | 2 +- src/kimchi/model/vms.py | 19 ++++---- src/kimchi/model/vmstorages.py | 7 ++- 7 files changed, 30 insertions(+), 118 deletions(-) diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index 862df7f..116e520 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -101,7 +101,6 @@ messages = { "KCHVM0027E": _("User(s) '%(users)s' do not exist"), "KCHVM0028E": _("Group(s) '%(groups)s' do not exist"), "KCHVM0029E": _("Unable to shutdown virtual machine %(name)s. Details: %(err)s"), - "KCHVM0030E": _("Unable to get access metadata of virtual machine %(name)s. Details: %(err)s"), "KCHVM0031E": _("The guest console password must be a string."), "KCHVM0032E": _("The life time for the guest console password must be a number."), "KCHVM0033E": _("Virtual machine '%(name)s' must be stopped before cloning it."), diff --git a/src/kimchi/model/config.py b/src/kimchi/model/config.py index d894b21..fe2a529 100644 --- a/src/kimchi/model/config.py +++ b/src/kimchi/model/config.py @@ -53,7 +53,6 @@ class CapabilitiesModel(object): self.qemu_stream = False self.libvirt_stream_protocols = [] self.fc_host_support = False - self.metadata_support = False self.kernel_vfio = False self.mem_hotplug_support = False @@ -90,7 +89,6 @@ class CapabilitiesModel(object): self.qemu_stream = FeatureTests.qemu_supports_iso_stream() self.nfs_target_probe = FeatureTests.libvirt_support_nfs_probe(conn) self.fc_host_support = FeatureTests.libvirt_support_fc_host(conn) - self.metadata_support = FeatureTests.has_metadata_support(conn) self.kernel_vfio = FeatureTests.kernel_support_vfio() self.mem_hotplug_support = FeatureTests.has_mem_hotplug_support(conn) diff --git a/src/kimchi/model/featuretests.py b/src/kimchi/model/featuretests.py index 047108f..4075d7c 100644 --- a/src/kimchi/model/featuretests.py +++ b/src/kimchi/model/featuretests.py @@ -186,30 +186,6 @@ class FeatureTests(object): return True @staticmethod - def has_metadata_support(conn): - KIMCHI_META_URL = "https://github.com/kimchi-project/kimchi/" - KIMCHI_NAMESPACE = "kimchi" - with RollbackContext() as rollback: - FeatureTests.disable_libvirt_error_logging() - rollback.prependDefer(FeatureTests.enable_libvirt_error_logging) - conn_type = conn.getType().lower() - domain_type = 'test' if conn_type == 'test' else 'kvm' - arch = 'i686' if conn_type == 'test' else platform.machine() - arch = 'ppc64' if arch == 'ppc64le' else arch - dom = conn.defineXML(SIMPLE_VM_XML % {'name': FEATURETEST_VM_NAME, - 'domain': domain_type, - 'arch': arch}) - rollback.prependDefer(dom.undefine) - try: - dom.setMetadata(libvirt.VIR_DOMAIN_METADATA_ELEMENT, - "<metatest/>", KIMCHI_NAMESPACE, - KIMCHI_META_URL, - flags=libvirt.VIR_DOMAIN_AFFECT_CURRENT) - return True - except libvirt.libvirtError: - return False - - @staticmethod def kernel_support_vfio(): out, err, rc = run_command(['modprobe', 'vfio-pci']) if rc != 0: diff --git a/src/kimchi/model/utils.py b/src/kimchi/model/utils.py index 7f27edd..c053807 100644 --- a/src/kimchi/model/utils.py +++ b/src/kimchi/model/utils.py @@ -18,8 +18,8 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA import libvirt -from lxml import etree, objectify -from lxml.builder import E, ElementMaker +from lxml import etree +from lxml.builder import E from kimchi.exception import OperationFailed @@ -59,30 +59,7 @@ def update_node(root, node): return root -def _kimchi_set_metadata_node(dom, node): - # some other tools will not let libvirt create a persistent - # configuration, raise exception. - if not dom.isPersistent(): - msg = 'The VM has not a persistent configuration' - raise OperationFailed("KCHVM0030E", - {'name': dom.name(), "err": msg}) - xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) - root = etree.fromstring(xml) - kimchi = root.find("metadata/{%s}kimchi" % KIMCHI_META_URL) - - EM = ElementMaker(namespace=KIMCHI_META_URL, - nsmap={KIMCHI_NAMESPACE: KIMCHI_META_URL}) - kimchi = EM("kimchi") if kimchi is None else kimchi - - update_node(kimchi, node) - metadata = root.find("metadata") - metadata = E.metadata() if metadata is None else metadata - update_node(metadata, kimchi) - update_node(root, metadata) - dom.connect().defineXML(etree.tostring(root)) - - -def libvirt_get_kimchi_metadata_node(dom, mode="current"): +def get_kimchi_metadata_node(dom, mode="current"): if not metadata_exists(dom): return None try: @@ -94,56 +71,22 @@ def libvirt_get_kimchi_metadata_node(dom, mode="current"): return None -def set_metadata_node(dom, node, metadata_support, mode="all"): - if metadata_support: - kimchi = libvirt_get_kimchi_metadata_node(dom, mode) - kimchi = E.metadata(E.kimchi()) if kimchi is None else kimchi - - update_node(kimchi, node) - kimchi_xml = etree.tostring(kimchi) - # 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, kimchi_xml, - KIMCHI_NAMESPACE, KIMCHI_META_URL, - flags=get_vm_config_flag(dom, mode)) - else: - # FIXME remove this code when all distro libvirt supports metadata - # element - _kimchi_set_metadata_node(dom, node) - - -def _kimchi_get_metadata_node(dom, tag): - # some other tools will not let libvirt create a persistent - # configuration, just return empty - if not dom.isPersistent(): - return None - xml = dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE) - root = etree.fromstring(xml) - kimchi = root.find("metadata/{%s}kimchi" % KIMCHI_META_URL) - # remove the "kimchi" prefix of xml - if kimchi is not None: - for elem in kimchi.getiterator(): - if not hasattr(elem.tag, 'find'): - continue - i = elem.tag.find('}') - if i >= 0: - elem.tag = elem.tag[i+1:] - - objectify.deannotate(kimchi) - etree.cleanup_namespaces(kimchi) - return kimchi - return None - - -def get_metadata_node(dom, tag, metadata_support, mode="current"): - if metadata_support: - kimchi = libvirt_get_kimchi_metadata_node(dom, mode) - else: - # FIXME remove this code when all distro libvirt supports metadata - # element - kimchi = _kimchi_get_metadata_node(dom, tag) +def set_metadata_node(dom, node, mode="all"): + kimchi = get_kimchi_metadata_node(dom, mode) + kimchi = E.metadata() if kimchi is None else kimchi + + update_node(kimchi, node) + kimchi_xml = etree.tostring(kimchi) + # 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, kimchi_xml, + KIMCHI_NAMESPACE, KIMCHI_META_URL, + flags=get_vm_config_flag(dom, mode)) + +def get_metadata_node(dom, tag, mode="current"): + kimchi = get_kimchi_metadata_node(dom, mode) if kimchi is not None: node = kimchi.find(tag) if node is not None: diff --git a/src/kimchi/model/vmifaces.py b/src/kimchi/model/vmifaces.py index 93a769b..8c97786 100644 --- a/src/kimchi/model/vmifaces.py +++ b/src/kimchi/model/vmifaces.py @@ -74,7 +74,7 @@ class VMIfacesModel(object): dom = VMModel.get_vm(vm, self.conn) - os_data = VMModel.vm_get_os_metadata(dom, self.caps.metadata_support) + os_data = VMModel.vm_get_os_metadata(dom) os_version, os_distro = os_data xml = get_iface_xml(params, conn.getInfo()[0], os_distro, os_version) diff --git a/src/kimchi/model/vms.py b/src/kimchi/model/vms.py index 106e9bc..a1f1798 100644 --- a/src/kimchi/model/vms.py +++ b/src/kimchi/model/vms.py @@ -161,8 +161,7 @@ class VMsModel(object): 'err': e.get_error_message()}) cb('Updating VM metadata') - VMModel.vm_update_os_metadata(VMModel.get_vm(name, self.conn), t.info, - self.caps.metadata_support) + VMModel.vm_update_os_metadata(VMModel.get_vm(name, self.conn), t.info) cb('OK', True) def get_list(self): @@ -485,8 +484,7 @@ class VMModel(object): def _build_access_elem(self, dom, users, groups): auth = config.get("authentication", "method") - access_xml = get_metadata_node(dom, "access", - self.caps.metadata_support) + access_xml = get_metadata_node(dom, "access") auth_elem = None @@ -545,12 +543,11 @@ class VMModel(object): return node = self._build_access_elem(dom, users, groups) - set_metadata_node(dom, node, self.caps.metadata_support) + set_metadata_node(dom, node) def _get_access_info(self, dom): users = groups = list() - access_xml = (get_metadata_node(dom, "access", - self.caps.metadata_support) or + access_xml = (get_metadata_node(dom, "access") or """<access></access>""") access_info = dictize(access_xml) auth = config.get("authentication", "method") @@ -568,20 +565,20 @@ class VMModel(object): return users, groups @staticmethod - def vm_get_os_metadata(dom, metadata_support): - os_xml = (get_metadata_node(dom, "os", metadata_support) or + def vm_get_os_metadata(dom): + os_xml = (get_metadata_node(dom, "os") or """<os></os>""") os_elem = ET.fromstring(os_xml) return (os_elem.attrib.get("version"), os_elem.attrib.get("distro")) @staticmethod - def vm_update_os_metadata(dom, params, metadata_support): + def vm_update_os_metadata(dom, params): distro = params.get("os_distro") version = params.get("os_version") if distro is None: return os_elem = E.os({"distro": distro, "version": version}) - set_metadata_node(dom, os_elem, metadata_support) + set_metadata_node(dom, os_elem) def _update_graphics(self, dom, xml, params): root = objectify.fromstring(xml) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index 142b177..5b90c18 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -36,9 +36,9 @@ from kimchi.xmlutils.disk import get_vm_disk_info, get_vm_disks HOTPLUG_TYPE = ['scsi', 'virtio'] -def _get_device_bus(dev_type, dom, metadata_support): +def _get_device_bus(dev_type, dom): try: - version, distro = VMModel.vm_get_os_metadata(dom, metadata_support) + version, distro = VMModel.vm_get_os_metadata(dom) except: version, distro = ('unknown', 'unknown') return lookup(distro, version)[dev_type+'_bus'] @@ -84,8 +84,7 @@ class VMStoragesModel(object): raise InvalidParameter("KCHVMSTOR0017E") dom = VMModel.get_vm(vm_name, self.conn) - params['bus'] = _get_device_bus(params['type'], dom, - self.caps.metadata_support) + params['bus'] = _get_device_bus(params['type'], dom) params['format'] = 'raw' dev_list = [dev for dev, bus in get_vm_disks(dom).iteritems() -- 2.1.0