From 105553563ac1347270338d0678c3b636c1149722 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Wed, 3 Jul 2019 17:55:01 -0400 Subject: [PATCH] storage: Simplify source pool enumeration Don't return an XML object stub, just return the relevant data. Make it explicit that we currently are only supporting lvm enumeration --- tests/storage-xml/pool-logical-list0.xml | 11 ----- tests/storage-xml/pool-logical-list1.xml | 11 ----- tests/storage-xml/pool-netfs-list0.xml | 11 ----- tests/storage.py | 29 +------------ virtManager/createpool.py | 52 +++++------------------- virtinst/storage.py | 44 +++++--------------- 6 files changed, 22 insertions(+), 136 deletions(-) delete mode 100644 tests/storage-xml/pool-logical-list0.xml delete mode 100644 tests/storage-xml/pool-logical-list1.xml delete mode 100644 tests/storage-xml/pool-netfs-list0.xml diff --git a/tests/storage-xml/pool-logical-list0.xml b/tests/storage-xml/pool-logical-list0.xml deleted file mode 100644 index ceeef1704..000000000 --- a/tests/storage-xml/pool-logical-list0.xml +++ /dev/null @@ -1,11 +0,0 @@ - - pool-logical-list0 - - - - testvg1 - - - /dev/testvg1 - - diff --git a/tests/storage-xml/pool-logical-list1.xml b/tests/storage-xml/pool-logical-list1.xml deleted file mode 100644 index 853e2ab79..000000000 --- a/tests/storage-xml/pool-logical-list1.xml +++ /dev/null @@ -1,11 +0,0 @@ - - pool-logical-list1 - - - - testvg2 - - - /dev/testvg2 - - diff --git a/tests/storage-xml/pool-netfs-list0.xml b/tests/storage-xml/pool-netfs-list0.xml deleted file mode 100644 index daf59f8e4..000000000 --- a/tests/storage-xml/pool-netfs-list0.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - pool-netfs-list0 - - /var/lib/libvirt/images/pool-netfs-list0 - - diff --git a/tests/storage.py b/tests/storage.py index 0c4e0a645..7b6895b4d 100644 --- a/tests/storage.py +++ b/tests/storage.py @@ -223,34 +223,7 @@ class TestStorage(unittest.TestCase): StoragePool.find_free_name(fullconn, "gluster-pool"), "gluster-pool-1") - - ############################## - # Tests for pool-sources API # - ############################## - - def _enumerateCompare(self, name, pool_list): - for pool in pool_list: - pool.name = name + str(pool_list.index(pool)) - poolobj = poolCompare(pool) - removePool(poolobj) - def testEnumerateLogical(self): - name = "pool-logical-list" lst = StoragePool.pool_list_from_sources(self.conn, StoragePool.TYPE_LOGICAL) - self._enumerateCompare(name, lst) - - def testEnumerateNetFS(self): - name = "pool-netfs-list" - host = "example.com" - lst = StoragePool.pool_list_from_sources(self.conn, - StoragePool.TYPE_NETFS, - host=host) - self._enumerateCompare(name, lst) - - def testEnumerateiSCSI(self): - host = "example.com" - lst = StoragePool.pool_list_from_sources(self.conn, - StoragePool.TYPE_ISCSI, - host=host) - self.assertTrue(len(lst) == 0) + self.assertEqual(lst, ["testvg1", "testvg2"]) diff --git a/virtManager/createpool.py b/virtManager/createpool.py index 5e735904d..cdefe4199 100644 --- a/virtManager/createpool.py +++ b/virtManager/createpool.py @@ -101,7 +101,7 @@ class vmmCreatePool(vmmGObjectUI): # Target path combo box entry target_list = self.widget("pool-target-path") # target_path, Label, pool class instance - target_model = Gtk.ListStore(str, str, object) + target_model = Gtk.ListStore(str, str) target_model.set_sort_column_id(0, Gtk.SortType.ASCENDING) target_list.set_model(target_model) target_list.set_entry_text_column(0) @@ -109,7 +109,7 @@ class vmmCreatePool(vmmGObjectUI): # Source path combo box entry source_list = self.widget("pool-source-path") # source_path, Label, pool class instance - source_model = Gtk.ListStore(str, str, object) + source_model = Gtk.ListStore(str, str) source_model.set_sort_column_id(0, Gtk.SortType.ASCENDING) source_list.set_model(source_model) source_list.set_entry_text_column(0) @@ -155,14 +155,15 @@ class vmmCreatePool(vmmGObjectUI): use_model = source_model entry_list = [] if pooltype == StoragePool.TYPE_SCSI: - entry_list = self._list_scsi_adapters() + host_list = self._list_scsi_adapters() + entry_list = [[h, h] for h in host_list] use_list = source_list use_model = source_model elif pooltype == StoragePool.TYPE_LOGICAL: - pool_list = self._list_pool_sources(pooltype) - entry_list = [[p.target_path, p.target_path, p] - for p in pool_list if p.target_path] + vglist = self._list_pool_sources(pooltype) + target_paths = ["/dev/%s" % vgname for vgname in vglist] + entry_list = [[t, t] for t in target_paths] use_list = target_list use_model = target_model @@ -175,26 +176,13 @@ class vmmCreatePool(vmmGObjectUI): def _list_scsi_adapters(self): scsi_hosts = self.conn.filter_nodedevs("scsi_host") host_list = [dev.xmlobj.host for dev in scsi_hosts] + return ["host%s" % h for h in host_list] - clean_list = [] - for h in host_list: - name = "host%s" % h - tmppool = self._make_stub_pool() - tmppool.source_path = name - - entry = [name, name, tmppool] - if name not in [l[0] for l in clean_list]: - clean_list.append(entry) - - return clean_list - - def _list_pool_sources(self, pool_type, host=None): + def _list_pool_sources(self, pool_type): plist = [] try: plist = StoragePool.pool_list_from_sources( - self.conn.get_backend(), - pool_type, - host=host) + self.conn.get_backend(), pool_type) except Exception: log.exception("Pool enumeration failed") @@ -315,31 +303,13 @@ class vmmCreatePool(vmmGObjectUI): # Object building # ################### - def _get_pool_from_sourcelist(self): - """ - If an enumerated pool source was selected, use that as the - basis for our pool object - """ - source_list = self.widget("pool-source-path") - target_list = self.widget("pool-target-path") - - pool = uiutil.get_list_selection(source_list, column=2, - check_entry=False) - if pool is None: - pool = uiutil.get_list_selection(target_list, column=2, - check_entry=False) - - return pool - def _build_xmlobj_from_xmleditor(self): xml = self._xmleditor.get_xml() log.debug("Using XML from xmleditor:\n%s", xml) return StoragePool(self.conn.get_backend(), parsexml=xml) def _make_stub_pool(self): - pool = self._get_pool_from_sourcelist() - if not pool: - pool = StoragePool(self.conn.get_backend()) + pool = StoragePool(self.conn.get_backend()) pool.type = self._get_config_pool_type() pool.name = self.widget("pool-name").get_text() return pool diff --git a/virtinst/storage.py b/virtinst/storage.py index f16a1c64c..5743b5a08 100644 --- a/virtinst/storage.py +++ b/virtinst/storage.py @@ -63,15 +63,6 @@ def _lookup_poolxml_by_path(conn, path): return None -class _EnumerateSource(XMLBuilder): - XML_NAME = "source" - - -class _EnumerateSources(XMLBuilder): - XML_NAME = "sources" - sources = XMLChildProperty(_EnumerateSource) - - class _Host(XMLBuilder): _XML_PROP_ORDER = ["name", "port"] XML_NAME = "host" @@ -98,47 +89,32 @@ class StoragePool(_StorageObject): TYPE_ZFS = "zfs" @staticmethod - def pool_list_from_sources(conn, pool_type, host=None): + def pool_list_from_sources(conn, pool_type): """ Return a list of StoragePool instances built from libvirt's pool source enumeration (if supported). :param conn: Libvirt connection - :param name: Name for the new pool :param pool_type: Pool type string from I{Types} - :param host: Option host string to poll for sources """ - if host: - source_xml = "" % host - else: - source_xml = "" + source_xml = "" try: xml = conn.findStoragePoolSources(pool_type, source_xml, 0) - except Exception as e: + except Exception as e: # pragma: no cover if conn.support.is_error_nosupport(e): return [] - raise # pragma: no cover + raise - ret = [] - sources = _EnumerateSources(conn, xml) - for source in sources.sources: - source_xml = source.get_xml() + log.debug("Libvirt returned pool sources XML:\n%s", xml) - pool_xml = "\n%s\n" % source_xml - parseobj = StoragePool(conn, parsexml=pool_xml) - parseobj.type = pool_type + import xml.etree.ElementTree as ET + root = ET.fromstring(xml) - obj = StoragePool(conn) - obj.type = pool_type - obj.source_path = parseobj.source_path - for h in parseobj.hosts: - parseobj.remove_child(h) - obj.add_child(h) - obj.source_name = parseobj.source_name - obj.format = parseobj.format + # We implicitly only support this for pool TYPE_LOGICAL + ret = [e.text for e in root.findall("./source/name")] - ret.append(obj) + log.debug("Sources returning: %s", ret) return ret @staticmethod