From 616a7f2dd58dbd9d61fb8b4645c89733c7b32154 Mon Sep 17 00:00:00 2001 From: Cole Robinson Date: Sun, 16 Jun 2019 20:10:37 -0400 Subject: [PATCH] createpool: Simplify LVM volume group UI Only show the volgroup name, and nothing else, which is all mostly extraneous https://bugzilla.redhat.com/show_bug.cgi?id=1316977 --- tests/storage-xml/pool-logical-srcname.xml | 4 - .../pool-logical-target-srcname.xml | 7 -- tests/storage-xml/pool-logical.xml | 4 - tests/storage.py | 2 +- tests/uitests/createpool.py | 21 +++++- ui/createpool.ui | 73 ++++++++++-------- virtManager/createpool.py | 75 +++++++++---------- virtinst/storage.py | 18 +---- 8 files changed, 99 insertions(+), 105 deletions(-) diff --git a/tests/storage-xml/pool-logical-srcname.xml b/tests/storage-xml/pool-logical-srcname.xml index 590e8fe7..8b7ff653 100644 --- a/tests/storage-xml/pool-logical-srcname.xml +++ b/tests/storage-xml/pool-logical-srcname.xml @@ -1,10 +1,6 @@ pool-logical-srcname - vgname - - /dev/pool-logical-srcname - diff --git a/tests/storage-xml/pool-logical-target-srcname.xml b/tests/storage-xml/pool-logical-target-srcname.xml index 0882b416..91664dc2 100644 --- a/tests/storage-xml/pool-logical-target-srcname.xml +++ b/tests/storage-xml/pool-logical-target-srcname.xml @@ -1,10 +1,3 @@ pool-logical-target-srcname - - - vgfoobar - - - /dev/vgfoobar - diff --git a/tests/storage-xml/pool-logical.xml b/tests/storage-xml/pool-logical.xml index 1d6bfa7c..0f4453bd 100644 --- a/tests/storage-xml/pool-logical.xml +++ b/tests/storage-xml/pool-logical.xml @@ -1,10 +1,6 @@ pool-logical - pool-logical - - /dev/pool-logical - diff --git a/tests/storage.py b/tests/storage.py index 7b6895b4..7449e30f 100644 --- a/tests/storage.py +++ b/tests/storage.py @@ -138,7 +138,7 @@ class TestStorage(unittest.TestCase): poolobj = createPool(self.conn, StoragePool.TYPE_LOGICAL, "pool-logical", - target_path="/dev/pool-logical") + source_name="pool-logical") invol = createVol(self.conn, poolobj) createVol(self.conn, poolobj, volname=invol.name() + "input", input_vol=invol) diff --git a/tests/uitests/createpool.py b/tests/uitests/createpool.py index f0aebf26..4e484d57 100644 --- a/tests/uitests/createpool.py +++ b/tests/uitests/createpool.py @@ -21,7 +21,7 @@ class CreatePool(uiutils.UITestCase): # Test cases # ############## - def testCreatePool(self): + def testCreatePools(self): hostwin = self._open_host_window("Storage") win = self._open_create_win(hostwin) @@ -58,6 +58,21 @@ class CreatePool(uiutils.UITestCase): # Ensure it's gone uiutils.check_in_loop(lambda: cell.dead) + # Test a logical pool + win = self._open_create_win(hostwin) + typ = win.find("Type:", "combo box") + newname = "a-lvm-pool" + name.text = "a-lvm-pool" + typ.click() + win.find_fuzzy("LVM", "menu item").click() + srcname = win.find_fuzzy("Volgroup", "combo") + srcnametext = win.find_fuzzy("pool-source-name-text") + uiutils.check_in_loop(lambda: srcnametext.text == "testvg1") + srcname.click_combo_entry() + win.find_fuzzy("testvg2", "menu item").click() + finish.click() + hostwin.find(newname, "table cell") + # Test a scsi pool win = self._open_create_win(hostwin) typ = win.find("Type:", "combo box") @@ -65,7 +80,7 @@ class CreatePool(uiutils.UITestCase): name.text = "a-scsi-pool" typ.click() win.find_fuzzy("SCSI Host Adapter", "menu item").click() - win.find_fuzzy("Source Path:", "combo").click_combo_entry() + win.find_fuzzy("Source Adapter:", "combo").click_combo_entry() win.find_fuzzy("host2", "menu item").click() finish.click() hostwin.find(newname, "table cell") @@ -77,7 +92,7 @@ class CreatePool(uiutils.UITestCase): typ.click() win.find_fuzzy("RADOS Block", "menu item").click() win.find_fuzzy("Host Name:", "text").text = "example.com:1234" - win.find_fuzzy("Source Name:", "text").typeText("frob") + win.find_fuzzy("pool-source-name-text", "text").typeText("frob") finish.click() hostwin.find(newname, "table cell") diff --git a/ui/createpool.ui b/ui/createpool.ui index b6a591fd..5c7f81ab 100644 --- a/ui/createpool.ui +++ b/ui/createpool.ui @@ -153,7 +153,6 @@ end Tar_get Path: True - pool-target-path 0 @@ -193,7 +192,7 @@ True False end - sourcep: + _sourcep: True pool-source-path @@ -231,23 +230,6 @@ 3 - - - True - False - True - - - True - 25 - - - - - 1 - 3 - - Bro_wse @@ -274,6 +256,11 @@ 25 + + + pool-source-path + + 1 @@ -312,7 +299,7 @@ True False end - Source N_ame: + sourcen_a: True pool-source-name @@ -321,16 +308,6 @@ 6 - - - True - 25 - - - 1 - 6 - - True @@ -424,6 +401,42 @@ 2 + + + True + True + + + 1 + 3 + + + + + True + False + True + + + True + + + pool-source-name-text + + + + + + + pool-source-name + + + + + 1 + 6 + + diff --git a/virtManager/createpool.py b/virtManager/createpool.py index cdefe419..c604a82c 100644 --- a/virtManager/createpool.py +++ b/virtManager/createpool.py @@ -98,17 +98,15 @@ class vmmCreatePool(vmmGObjectUI): for f in ["auto"]: format_model.append([f, f]) - # Target path combo box entry - target_list = self.widget("pool-target-path") - # target_path, Label, pool class instance - 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) + combo = self.widget("pool-source-name") + # [name, label] + model = Gtk.ListStore(str, str) + model.set_sort_column_id(0, Gtk.SortType.ASCENDING) + combo.set_model(model) + combo.set_entry_text_column(0) - # Source path combo box entry source_list = self.widget("pool-source-path") - # source_path, Label, pool class instance + # [source_path, label] source_model = Gtk.ListStore(str, str) source_model.set_sort_column_id(0, Gtk.SortType.ASCENDING) source_list.set_model(source_model) @@ -123,7 +121,7 @@ class vmmCreatePool(vmmGObjectUI): self.conn.get_backend(), "pool") self.widget("pool-name").set_text(defaultname) self.widget("pool-name").grab_focus() - self.widget("pool-target-path").get_child().set_text("") + self.widget("pool-target-path").set_text("") self.widget("pool-source-path").get_child().set_text("") self.widget("pool-hostname").set_text("") self.widget("pool-iqn-chk").set_active(False) @@ -144,32 +142,26 @@ class vmmCreatePool(vmmGObjectUI): def _populate_pool_sources(self): pooltype = self._get_config_pool_type() source_list = self.widget("pool-source-path") - source_model = source_list.get_model() - source_model.clear() + source_list.get_model().clear() - target_list = self.widget("pool-target-path") - target_model = target_list.get_model() - target_model.clear() + name_list = self.widget("pool-source-name") + name_list.get_model().clear() - use_list = source_list - use_model = source_model - entry_list = [] if pooltype == StoragePool.TYPE_SCSI: 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: 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 + entry_list = [[v, v] for v in vglist] + use_list = name_list + + else: + return for e in entry_list: - use_model.append(e) - + use_list.get_model().append(e) if entry_list: use_list.set_active(0) @@ -198,8 +190,7 @@ class vmmCreatePool(vmmGObjectUI): StoragePool.TYPE_NETFS]: # Building for these simply entails creating a directory return (True, False) - elif pooltype in [StoragePool.TYPE_LOGICAL, - StoragePool.TYPE_DISK]: + elif pooltype in [StoragePool.TYPE_DISK]: # This is a dangerous operation, anything (False, True) # should be assumed to be one. return (False, True) @@ -221,10 +212,9 @@ class vmmCreatePool(vmmGObjectUI): iqn = pool.supports_iqn() builddef, buildsens = self._get_build_default(pool.type) - # We don't show source_name for logical pools, since we use - # pool-sources to avoid the need for it - src_name = (pool.supports_source_name() and - pool.type != pool.TYPE_LOGICAL) + src_name = pool.supports_source_name() + is_lvm = pool.type == StoragePool.TYPE_LOGICAL + is_scsi = pool.type == StoragePool.TYPE_SCSI # Source path browsing is meaningless for net pools if pool.type in [StoragePool.TYPE_NETFS, @@ -241,13 +231,18 @@ class vmmCreatePool(vmmGObjectUI): show_row("pool-iqn", iqn) show_row("pool-source-name", src_name) + self.widget("pool-source-name-label").set_label( + is_lvm and _("Volg_roup Name:") or _("Sou_rce Name:")) + + src_label = _("_Source Path:") if iqn: - self.widget("pool-source-label").set_label(_("_Source IQN:")) - else: - self.widget("pool-source-label").set_label(_("_Source Path:")) + src_label = _("_Source IQN:") + elif is_scsi: + src_label = _("_Source Adapter:") + self.widget("pool-source-label").set_text(src_label) if tgt: - self.widget("pool-target-path").get_child().set_text( + self.widget("pool-target-path").set_text( pool.default_target_path() or "") self.widget("pool-target-button").set_sensitive(tgt_b) @@ -255,7 +250,7 @@ class vmmCreatePool(vmmGObjectUI): self.widget("pool-build").set_active(builddef) if src_name: - self.widget("pool-source-name").set_text( + self.widget("pool-source-name").get_child().set_text( pool.default_source_name() or "") self._populate_pool_sources() @@ -275,13 +270,13 @@ class vmmCreatePool(vmmGObjectUI): ret = uiutil.get_list_selection(widget, column=column) if ret is not None: return ret - return src.get_child().get_text().strip() + return widget_name.get_child().get_text().strip() def _get_config_pool_type(self): return uiutil.get_list_selection(self.widget("pool-type")) def _get_config_target_path(self): - return self._get_visible_text("pool-target-path", column=1) + return self._get_visible_text("pool-target-path") def _get_config_source_path(self): return self._get_visible_text("pool-source-path", column=1) @@ -290,7 +285,7 @@ class vmmCreatePool(vmmGObjectUI): return self._get_visible_text("pool-hostname") def _get_config_source_name(self): - return self._get_visible_text("pool-source-name") + return self._get_visible_text("pool-source-name", column=1) def _get_config_format(self): return uiutil.get_list_selection(self.widget("pool-format")) @@ -447,7 +442,7 @@ class vmmCreatePool(vmmGObjectUI): dialog_type=Gtk.FileChooserAction.SELECT_FOLDER, start_folder=startfolder) if target: - self.widget("pool-target-path").get_child().set_text(target) + self.widget("pool-target-path").set_text(target) def _hostname_changed_cb(self, src): # If a hostname was entered, try to lookup valid pool sources. diff --git a/virtinst/storage.py b/virtinst/storage.py index 5743b5a0..2249bde4 100644 --- a/virtinst/storage.py +++ b/virtinst/storage.py @@ -16,7 +16,6 @@ from .xmlbuilder import XMLBuilder, XMLChildProperty, XMLProperty _DEFAULT_DEV_TARGET = "/dev" -_DEFAULT_LVM_TARGET_BASE = "/dev/" _DEFAULT_SCSI_TARGET = "/dev/disk/by-path" _DEFAULT_MPATH_TARGET = "/dev/mapper" @@ -233,11 +232,6 @@ class StoragePool(_StorageObject): self.type == self.TYPE_FS): return os.path.join( _preferred_default_pool_path(self.conn), self.name) - if self.type == self.TYPE_LOGICAL: - name = self.name - if self.source_name: - name = self.source_name - return _DEFAULT_LVM_TARGET_BASE + name if self.type == self.TYPE_DISK: return _DEFAULT_DEV_TARGET if self.type == self.TYPE_ISCSI or self.type == self.TYPE_SCSI: @@ -269,14 +263,6 @@ class StoragePool(_StorageObject): if self.type == StoragePool.TYPE_GLUSTER: return "gv0" - if ("target_path" in self._propstore and - self.target_path and - self.target_path.startswith(_DEFAULT_LVM_TARGET_BASE)): - # If there is a target path, parse it for an expected VG - # location, and pull the name from there - vg = self.target_path[len(_DEFAULT_LVM_TARGET_BASE):] - return vg.split("/", 1)[0] - ############## # Properties # @@ -323,7 +309,7 @@ class StoragePool(_StorageObject): def supports_target_path(self): return self.type in [ self.TYPE_DIR, self.TYPE_FS, self.TYPE_NETFS, - self.TYPE_LOGICAL, self.TYPE_DISK, self.TYPE_ISCSI, + self.TYPE_DISK, self.TYPE_ISCSI, self.TYPE_SCSI, self.TYPE_MPATH] def supports_source_name(self): @@ -333,7 +319,7 @@ class StoragePool(_StorageObject): def supports_source_path(self): return self.type in [ - self.TYPE_FS, self.TYPE_NETFS, self.TYPE_LOGICAL, + self.TYPE_FS, self.TYPE_NETFS, self.TYPE_DISK, self.TYPE_ISCSI, self.TYPE_SCSI, self.TYPE_GLUSTER]