Contains all suggestions from prior review, minus bcl's one suggestion for blivet patches, which I later realized isn't needed:
- if dev is None or dev == "":
raise ValueError("You have not specified a device number or the number is invalid")
- dev = dev.lower()
- bus = dev[:string.rfind(dev, ".") + 1]
- dev = dev[string.rfind(dev, ".") + 1:]
I'd pull the rfind out into its own variable so you can check for -1 and raise an error (assuming no . is an error)
Lack of '.' is not necessarily an error. There's now a docstring which notes valid formats for the device number, but basically either 0.0.abcd or abcd (or caps are also ok if a-f appear in the dev number).
This adds a button to the storage filter for manually adding an ECKD DASD.
Resolves: rhbz#1070115 --- pyanaconda/ui/gui/spokes/filter.glade | 19 +++++++++++++++++-- pyanaconda/ui/gui/spokes/filter.py | 12 ++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/pyanaconda/ui/gui/spokes/filter.glade b/pyanaconda/ui/gui/spokes/filter.glade index ff0426d..88e736c 100644 --- a/pyanaconda/ui/gui/spokes/filter.glade +++ b/pyanaconda/ui/gui/spokes/filter.glade @@ -1639,6 +1639,21 @@ </packing> </child> <child> + <object class="GtkButton" id="addDASDButton"> + <property name="label" translatable="yes">Add _DASD...</property> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <property name="use_underline">True</property> + <signal name="clicked" handler="on_add_dasd_clicked" swapped="no"/> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + <child> <object class="GtkButton" id="addISCSIButton"> <property name="label" translatable="yes">Add _iSCSI Target...</property> <property name="visible">True</property> @@ -1650,7 +1665,7 @@ <packing> <property name="expand">False</property> <property name="fill">True</property> - <property name="position">1</property> + <property name="position">2</property> </packing> </child> <child> @@ -1665,7 +1680,7 @@ <packing> <property name="expand">False</property> <property name="fill">True</property> - <property name="position">2</property> + <property name="position">3</property> </packing> </child> </object> diff --git a/pyanaconda/ui/gui/spokes/filter.py b/pyanaconda/ui/gui/spokes/filter.py index 97b07ca..77977e1 100644 --- a/pyanaconda/ui/gui/spokes/filter.py +++ b/pyanaconda/ui/gui/spokes/filter.py @@ -38,6 +38,7 @@ from pyanaconda.ui.gui.spokes import NormalSpoke from pyanaconda.ui.gui.spokes.advstorage.fcoe import FCoEDialog from pyanaconda.ui.gui.spokes.advstorage.iscsi import ISCSIDialog from pyanaconda.ui.gui.spokes.advstorage.zfcp import ZFCPDialog +from pyanaconda.ui.gui.spokes.advstorage.dasd import DASDDialog from pyanaconda.ui.gui.spokes.lib.cart import SelectedDisksDialog from pyanaconda.ui.gui.categories.system import SystemCategory
@@ -677,6 +678,17 @@ class FilterSpoke(NormalSpoke): # storage are displayed in the UI. self.refresh()
+ def on_add_dasd_clicked(self, widget, *args): + dialog = DASDDialog(self.data, self.storage) + + with enlightbox(self.window, dialog.window): + dialog.refresh() + dialog.run() + + # We now need to refresh so any new disks picked up by adding advanced + # storage are displayed in the UI. + self.refresh() + ## ## SEARCH TAB SIGNAL HANDLERS ##
This just adds a couple of functions which aid in manually adding DASDs. sanitize_dasd_dev_input takes a device number as a string and verifies whether its format is acceptable as a match against DASD device numbers.
online_dasd takes a device number as a string and fill remove the device from the ignore list and attempt to bring it online.
Resolves: rhbz#1070115 --- blivet/devicelibs/dasd.py | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)
diff --git a/blivet/devicelibs/dasd.py b/blivet/devicelibs/dasd.py index 5beb11d..5072c0a 100644 --- a/blivet/devicelibs/dasd.py +++ b/blivet/devicelibs/dasd.py @@ -109,6 +109,55 @@ def dasd_needs_format(dasd):
return False
+def sanitize_dasd_dev_input(dev): + """ Given a user-supplied device number, make sure the input is sane, and + raise an error if it is not. + + :param dev: string representing a DASD device number + """ + if dev is None or dev == "": + raise ValueError(_("You have not specified a device number or the number is invalid")) + dev = dev.lower() + bus = dev[:dev.rfind(".") + 1] + dev = dev[dev.rfind(".") + 1:] + + dev = "0" * (4 - len(dev)) + dev + if not bus: + return "0.0." + dev + else: + return bus + dev + +def online_dasd(dev): + """ Given a device number, switch the device to be online. + + :param dev: string representing a DASD device number; acceptable formats + include 0.0.abcd, 0.0.ABCD, abcd, ABCD, where 'abcd' are + hexadecimal numbers. + """ + online = "/sys/bus/ccw/drivers/dasd-eckd/%s/online" % (dev) + + if not os.path.exists(online): + log.info("Freeing DASD device %s" % dev) + util.run_program(["dasd_cio_free", "-d", dev]) + + if not os.path.exists(online): + raise ValueError(_("DASD device %s not found, not even in device ignore list.") + % dev) + + try: + with open(online, "r") as f: + devonline = f.readline().strip() + f.close() + if devonline == "1": + raise ValueError(_("Device %s is already online.") % dev) + else: + with open(online, "w") as f: + log.debug("echo %s > %s" % ("1", online)) + f.write("%s\n" % ("1")) + f.close() + except IOError as e: + raise ValueError(_("Could not set DASD device %(dev)s online (%(e)s).") \ + % {'dev': dev, 'e': e})
def write_dasd_conf(disks, root): """ Write /etc/dasd.conf to target system for all DASD devices
On Tue, 2014-09-02 at 08:39 -0400, Samantha N. Bueno wrote:
This just adds a couple of functions which aid in manually adding DASDs. sanitize_dasd_dev_input takes a device number as a string and verifies whether its format is acceptable as a match against DASD device numbers.
online_dasd takes a device number as a string and fill remove the device from the ignore list and attempt to bring it online.
Resolves: rhbz#1070115
blivet/devicelibs/dasd.py | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)
diff --git a/blivet/devicelibs/dasd.py b/blivet/devicelibs/dasd.py index 5beb11d..5072c0a 100644 --- a/blivet/devicelibs/dasd.py +++ b/blivet/devicelibs/dasd.py @@ -109,6 +109,55 @@ def dasd_needs_format(dasd):
return False
+def sanitize_dasd_dev_input(dev):
- """ Given a user-supplied device number, make sure the input is sane, and
raise an error if it is not.
- :param dev: string representing a DASD device number
- """
- if dev is None or dev == "":
raise ValueError(_("You have not specified a device number or the number is invalid"))
'if not dev' would be an equivalent condition here, right? Choose what looks better to you. :)
- dev = dev.lower()
- bus = dev[:dev.rfind(".") + 1]
- dev = dev[dev.rfind(".") + 1:]
- dev = "0" * (4 - len(dev)) + dev
- if not bus:
return "0.0." + dev
- else:
return bus + dev
+def online_dasd(dev):
- """ Given a device number, switch the device to be online.
- :param dev: string representing a DASD device number; acceptable formats
include 0.0.abcd, 0.0.ABCD, abcd, ABCD, where 'abcd' are
hexadecimal numbers.
- """
- online = "/sys/bus/ccw/drivers/dasd-eckd/%s/online" % (dev)
- if not os.path.exists(online):
log.info("Freeing DASD device %s" % dev)
util.run_program(["dasd_cio_free", "-d", dev])
- if not os.path.exists(online):
raise ValueError(_("DASD device %s not found, not even in device ignore list.")
% dev)
- try:
with open(online, "r") as f:
devonline = f.readline().strip()
f.close()
You don't have to close the file when using the 'with' block.
if devonline == "1":
raise ValueError(_("Device %s is already online.") % dev)
else:
with open(online, "w") as f:
log.debug("echo %s > %s" % ("1", online))
f.write("%s\n" % ("1"))
f.close()
Same here.
This is the dialog box which pops up prompting for info so more (ECKD) DASDs can be added.
Upon a successful add, users are returned to the filter. Due to the fact that DASDs are detected as local storage, newly-added DASDs here are not going to show up in the Search or System Z Devices tab. They do appear under "Local Standard Disks" back in the main storage spoke though.
Resolves: rhbz#1070115 --- pyanaconda/ui/gui/spokes/advstorage/dasd.glade | 362 +++++++++++++++++++++++++ pyanaconda/ui/gui/spokes/advstorage/dasd.py | 139 ++++++++++ 2 files changed, 501 insertions(+) create mode 100644 pyanaconda/ui/gui/spokes/advstorage/dasd.glade create mode 100644 pyanaconda/ui/gui/spokes/advstorage/dasd.py
diff --git a/pyanaconda/ui/gui/spokes/advstorage/dasd.glade b/pyanaconda/ui/gui/spokes/advstorage/dasd.glade new file mode 100644 index 0000000..bd16e7f --- /dev/null +++ b/pyanaconda/ui/gui/spokes/advstorage/dasd.glade @@ -0,0 +1,362 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- Generated with glade 3.16.1 --> +<interface> + <requires lib="gtk+" version="3.0"/> + <object class="GtkDialog" id="dasdDialog"> + <property name="can_focus">False</property> + <property name="border_width">5</property> + <property name="modal">True</property> + <property name="type_hint">dialog</property> + <property name="decorated">False</property> + <child internal-child="vbox"> + <object class="GtkBox" id="dialog-vbox1"> + <property name="can_focus">False</property> + <property name="orientation">vertical</property> + <property name="spacing">2</property> + <child internal-child="action_area"> + <object class="GtkButtonBox" id="dialog-action_area1"> + <property name="can_focus">False</property> + <property name="layout_style">end</property> + <child> + <object class="GtkButton" id="cancelButton"> + <property name="label">gtk-cancel</property> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <property name="use_stock">True</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkButton" id="okButton"> + <property name="label">gtk-ok</property> + <property name="visible">True</property> + <property name="sensitive">False</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <property name="use_stock">True</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="pack_type">end</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkBox" id="box1"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="orientation">vertical</property> + <property name="spacing">6</property> + <child> + <object class="GtkLabel" id="label1"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="xalign">0</property> + <property name="label" translatable="yes">ADD DASD STORAGE TARGET</property> + <attributes> + <attribute name="weight" value="bold"/> + </attributes> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkNotebook" id="dasdNotebook"> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="show_tabs">False</property> + <property name="show_border">False</property> + <child> + <object class="GtkGrid" id="configureGrid"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="row_spacing">6</property> + <property name="column_spacing">6</property> + <child> + <object class="GtkLabel" id="label5"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="margin_bottom">6</property> + <property name="xalign">0</property> + <property name="label" translatable="yes">To use DASD disks, you must provide the device number.</property> + <property name="wrap">True</property> + </object> + <packing> + <property name="left_attach">0</property> + <property name="top_attach">0</property> + <property name="width">3</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkEntry" id="deviceEntry"> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="hexpand">True</property> + <property name="invisible_char">?</property> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">1</property> + <property name="width">2</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkLabel" id="label6"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="xalign">0</property> + <property name="label" translatable="yes">_Device number:</property> + <property name="use_underline">True</property> + <property name="mnemonic_widget">deviceEntry</property> + </object> + <packing> + <property name="left_attach">0</property> + <property name="top_attach">1</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkNotebook" id="conditionNotebook"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="valign">center</property> + <property name="show_tabs">False</property> + <property name="show_border">False</property> + <child> + <object class="GtkButton" id="startButton"> + <property name="label" translatable="yes">_Start Discovery</property> + <property name="visible">True</property> + <property name="sensitive">False</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <property name="halign">center</property> + <property name="valign">center</property> + <property name="vexpand">True</property> + <property name="use_underline">True</property> + <signal name="clicked" handler="on_start_clicked" swapped="no"/> + </object> + </child> + <child type="tab"> + <placeholder/> + </child> + <child> + <object class="GtkBox" id="waitBox"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="halign">center</property> + <property name="spacing">5</property> + <child> + <object class="GtkSpinner" id="waitSpinner"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="active">True</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">0</property> + </packing> + </child> + <child> + <object class="GtkLabel" id="label13"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="label" translatable="yes">Discovering DASD devices. This may take a moment...</property> + </object> + <packing> + <property name="expand">False</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + </object> + <packing> + <property name="position">1</property> + </packing> + </child> + <child type="tab"> + <placeholder/> + </child> + <child> + <object class="GtkGrid" id="errorBox"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="halign">center</property> + <property name="row_spacing">6</property> + <property name="column_spacing">6</property> + <child> + <object class="GtkImage" id="image3"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="xalign">1</property> + <property name="stock">gtk-dialog-error</property> + </object> + <packing> + <property name="left_attach">0</property> + <property name="top_attach">0</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkLabel" id="label14"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="xalign">0</property> + <property name="label" translatable="yes">Device discovery failed.</property> + <attributes> + <attribute name="weight" value="bold"/> + </attributes> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">0</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkLabel" id="label22"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="xalign">0</property> + <property name="label" translatable="yes">The following error occurred discovering DASD devices. Please double check your configuration information and try again.</property> + <property name="wrap">True</property> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">1</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkLabel" id="deviceErrorLabel"> + <property name="visible">True</property> + <property name="can_focus">False</property> + <property name="xalign">0</property> + <property name="label">Error message goes here.</property> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">2</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <object class="GtkButton" id="retryButton"> + <property name="label" translatable="yes">_Retry Discovery</property> + <property name="visible">True</property> + <property name="can_focus">True</property> + <property name="receives_default">True</property> + <property name="halign">center</property> + <property name="valign">center</property> + <property name="use_underline">True</property> + <signal name="clicked" handler="on_start_clicked" swapped="no"/> + </object> + <packing> + <property name="left_attach">1</property> + <property name="top_attach">3</property> + <property name="width">1</property> + <property name="height">1</property> + </packing> + </child> + <child> + <placeholder/> + </child> + <child> + <placeholder/> + </child> + <child> + <placeholder/> + </child> + </object> + <packing> + <property name="position">2</property> + </packing> + </child> + <child type="tab"> + <placeholder/> + </child> + </object> + <packing> + <property name="left_attach">0</property> + <property name="top_attach">2</property> + <property name="width">3</property> + <property name="height">1</property> + </packing> + </child> + </object> + </child> + <child type="tab"> + <placeholder/> + </child> + <child> + <placeholder/> + </child> + <child type="tab"> + <placeholder/> + </child> + <child> + <placeholder/> + </child> + <child type="tab"> + <placeholder/> + </child> + </object> + <packing> + <property name="expand">True</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + </object> + <packing> + <property name="expand">True</property> + <property name="fill">True</property> + <property name="position">1</property> + </packing> + </child> + </object> + </child> + <action-widgets> + <action-widget response="0">cancelButton</action-widget> + <action-widget response="1">okButton</action-widget> + </action-widgets> + </object> + <object class="GtkListStore" id="nodeStore"> + <columns> + <!-- column-name nodeSelected --> + <column type="gboolean"/> + <!-- column-name nodeNotLoggedIn --> + <column type="gboolean"/> + <!-- column-name nodeName --> + <column type="gchararray"/> + <!-- column-name nodeIface --> + <column type="gchararray"/> + </columns> + </object> + <object class="GtkTreeModelFilter" id="nodeStoreFiltered"> + <property name="child_model">nodeStore</property> + </object> +</interface> diff --git a/pyanaconda/ui/gui/spokes/advstorage/dasd.py b/pyanaconda/ui/gui/spokes/advstorage/dasd.py new file mode 100644 index 0000000..555960e --- /dev/null +++ b/pyanaconda/ui/gui/spokes/advstorage/dasd.py @@ -0,0 +1,139 @@ +# DASD configuration dialog +# +# Copyright (C) 2014 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): Samantha N. Bueno sbueno@redhat.com +# + +from pyanaconda.ui.gui import GUIObject +from pyanaconda.ui.gui.utils import gtk_action_nowait + +from blivet.devicelibs.dasd import sanitize_dasd_dev_input, online_dasd + +__all__ = ["DASDDialog"] + +class DASDDialog(GUIObject): + """ Gtk dialog which allows users to manually add DASD devices without + having previously specified them in a parm file. + """ + builderObjects = ["dasdDialog"] + mainWidgetName = "dasdDialog" + uiFile = "spokes/advstorage/dasd.glade" + + def __init__(self, data, storage): + GUIObject.__init__(self, data) + self.storage = storage + self.dasd = self.storage.dasd + + self._discoveryError = None + + self._update_devicetree = False + + # grab all of the ui objects + self._dasdNotebook = self.builder.get_object("dasdNotebook") + + self._configureGrid = self.builder.get_object("configureGrid") + self._conditionNotebook = self.builder.get_object("conditionNotebook") + + self._startButton = self.builder.get_object("startButton") + self._okButton = self.builder.get_object("okButton") + self._cancelButton = self.builder.get_object("cancelButton") + + self._deviceEntry = self.builder.get_object("deviceEntry") + + self._spinner = self.builder.get_object("waitSpinner") + + def refresh(self): + self._deviceEntry.set_text("") + self._deviceEntry.set_sensitive(True) + self._startButton.set_sensitive(True) + + def run(self): + rc = self.window.run() + self.window.destroy() + # We need to call this to get the device nodes to show up + # in our devicetree. + if self._update_devicetree: + self.storage.devicetree.populate() + return rc + + def on_start_clicked(self, *args): + """ Go through the process of validating entry contents and then + attempt to add the device. + """ + # First update widgets + self._startButton.hide() + self._cancelButton.set_sensitive(False) + self._okButton.set_sensitive(False) + + self._conditionNotebook.set_current_page(1) + self._configureGrid.set_sensitive(False) + self._deviceEntry.set_sensitive(False) + + self._conditionNotebook.set_current_page(1) + try: + device = sanitize_dasd_dev_input(self._deviceEntry.get_text()) + except ValueError as e: + _config_error = str(e) + self.builder.get_object("deviceErrorLabel").set_text(_config_error) + self._conditionNotebook.set_current_page(2) + self._configureGrid.set_sensitive(True) + self._cancelButton.set_sensitive(True) + self._deviceEntry.set_sensitive(True) + return + + self._spinner.start() + + self._discover(device) + self._check_discover() + + @gtk_action_nowait + def _check_discover(self): + """ After the DASD discover thread runs, check to see whether a valid + device was discovered. Display an error message if not. + """ + + self._spinner.stop() + + if self._discoveryError: + # Failure, display a message and leave the user on the dialog so + # they can try again (or cancel) + self.builder.get_object("deviceErrorLabel").set_text(self._discoveryError) + self._discoveryError = None + self._conditionNotebook.set_current_page(2) + self._configureGrid.set_sensitive(True) + self._deviceEntry.set_sensitive(True) + else: + # Great success. Just return to the advanced storage window and let the + # UI update with the newly-added device + self.window.response(1) + return True + + self._cancelButton.set_sensitive(True) + return False + + def _discover(self, device): + """ Given the configuration options from a user, attempt to discover + a DASD device. This includes searching black-listed devices. + """ + # attempt to add the device + try: + online_dasd(device) + self._update_devicetree = True + except ValueError as e: + self._discoveryError = str(e) + return
diff --git a/pyanaconda/ui/gui/spokes/advstorage/dasd.glade b/pyanaconda/ui/gui/spokes/advstorage/dasd.glade new file mode 100644 index 0000000..bd16e7f --- /dev/null +++ b/pyanaconda/ui/gui/spokes/advstorage/dasd.glade @@ -0,0 +1,362 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- Generated with glade 3.16.1 --> +<interface>
<requires lib="gtk+" version="3.0"/>
<object class="GtkDialog" id="dasdDialog">
- <property name="can_focus">False</property>
- <property name="border_width">5</property>
Border widths should be multiples of 6. We do a much better job of this on master than on rhel7-branch, but that's probably not critical enough to go through and standardize.
<child>
<object class="GtkButton" id="cancelButton">
<property name="label">gtk-cancel</property>
...
<child>
<object class="GtkButton" id="okButton">
<property name="label">gtk-ok</property>
We're not supposed to use stock-id anymore. See 983aff8b7cb8842f129e1760975c2133f487be4b.
- Chris
On Tue, 2014-09-02 at 08:39 -0400, Samantha N. Bueno wrote:
This is the dialog box which pops up prompting for info so more (ECKD) DASDs can be added.
Upon a successful add, users are returned to the filter. Due to the fact that DASDs are detected as local storage, newly-added DASDs here are not going to show up in the Search or System Z Devices tab. They do appear under "Local Standard Disks" back in the main storage spoke though.
Resolves: rhbz#1070115
pyanaconda/ui/gui/spokes/advstorage/dasd.glade | 362 +++++++++++++++++++++++++ pyanaconda/ui/gui/spokes/advstorage/dasd.py | 139 ++++++++++ 2 files changed, 501 insertions(+) create mode 100644 pyanaconda/ui/gui/spokes/advstorage/dasd.glade create mode 100644 pyanaconda/ui/gui/spokes/advstorage/dasd.py
diff --git a/pyanaconda/ui/gui/spokes/advstorage/dasd.py b/pyanaconda/ui/gui/spokes/advstorage/dasd.py new file mode 100644 index 0000000..555960e --- /dev/null +++ b/pyanaconda/ui/gui/spokes/advstorage/dasd.py @@ -0,0 +1,139 @@ +# DASD configuration dialog +# +# Copyright (C) 2014 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): Samantha N. Bueno sbueno@redhat.com +#
+from pyanaconda.ui.gui import GUIObject +from pyanaconda.ui.gui.utils import gtk_action_nowait
+from blivet.devicelibs.dasd import sanitize_dasd_dev_input, online_dasd
+__all__ = ["DASDDialog"]
+class DASDDialog(GUIObject):
- """ Gtk dialog which allows users to manually add DASD devices without
having previously specified them in a parm file.
- """
- builderObjects = ["dasdDialog"]
- mainWidgetName = "dasdDialog"
- uiFile = "spokes/advstorage/dasd.glade"
- def __init__(self, data, storage):
GUIObject.__init__(self, data)
self.storage = storage
self.dasd = self.storage.dasd
self._discoveryError = None
self._update_devicetree = False
# grab all of the ui objects
self._dasdNotebook = self.builder.get_object("dasdNotebook")
self._configureGrid = self.builder.get_object("configureGrid")
self._conditionNotebook = self.builder.get_object("conditionNotebook")
self._startButton = self.builder.get_object("startButton")
self._okButton = self.builder.get_object("okButton")
self._cancelButton = self.builder.get_object("cancelButton")
self._deviceEntry = self.builder.get_object("deviceEntry")
self._spinner = self.builder.get_object("waitSpinner")
- def refresh(self):
self._deviceEntry.set_text("")
self._deviceEntry.set_sensitive(True)
self._startButton.set_sensitive(True)
- def run(self):
rc = self.window.run()
self.window.destroy()
# We need to call this to get the device nodes to show up
# in our devicetree.
if self._update_devicetree:
self.storage.devicetree.populate()
When I see this call I imagine something that takes quite a long time. If that's the case, we would probably need to do it before the dialog is destroyed, informing user what's going on somehow (label+spinner). Maybe as an extra patch if/once somebody complains.
return rc
- def on_start_clicked(self, *args):
""" Go through the process of validating entry contents and then
attempt to add the device.
"""
# First update widgets
self._startButton.hide()
self._cancelButton.set_sensitive(False)
self._okButton.set_sensitive(False)
self._conditionNotebook.set_current_page(1)
self._configureGrid.set_sensitive(False)
self._deviceEntry.set_sensitive(False)
I'd like to avoid studying the .glade file, but if these two widgets are on the page 1 of the _conditionNetbook, you don't have to make them insensitive when they go away anyway.
self._conditionNotebook.set_current_page(1)
This don't have to be done twice, I think. :)
try:
device = sanitize_dasd_dev_input(self._deviceEntry.get_text())
except ValueError as e:
_config_error = str(e)
self.builder.get_object("deviceErrorLabel").set_text(_config_error)
self._conditionNotebook.set_current_page(2)
self._configureGrid.set_sensitive(True)
self._cancelButton.set_sensitive(True)
self._deviceEntry.set_sensitive(True)
The same here with the (in)sensitive widgets on the page that goes away.
return
self._spinner.start()
self._discover(device)
self._check_discover()
- @gtk_action_nowait
- def _check_discover(self):
""" After the DASD discover thread runs, check to see whether a valid
device was discovered. Display an error message if not.
"""
self._spinner.stop()
if self._discoveryError:
# Failure, display a message and leave the user on the dialog so
# they can try again (or cancel)
self.builder.get_object("deviceErrorLabel").set_text(self._discoveryError)
self._discoveryError = None
self._conditionNotebook.set_current_page(2)
self._configureGrid.set_sensitive(True)
self._deviceEntry.set_sensitive(True)
And here.
Also, I've applied one fix locally already....adding pyanaconda/ui/gui/spokes/advstorage/dasd.py to po/POTFILES.in
On Tue, Sep 02, 2014 at 08:39:53AM -0400, Samantha N. Bueno wrote:
Contains all suggestions from prior review, minus bcl's one suggestion for blivet patches, which I later realized isn't needed:
- if dev is None or dev == "":
raise ValueError("You have not specified a device number or the number is invalid")
- dev = dev.lower()
- bus = dev[:string.rfind(dev, ".") + 1]
- dev = dev[string.rfind(dev, ".") + 1:]
I'd pull the rfind out into its own variable so you can check for -1 and raise an error (assuming no . is an error)
Lack of '.' is not necessarily an error. There's now a docstring which notes valid formats for the device number, but basically either 0.0.abcd or abcd (or caps are also ok if a-f appear in the dev number). _______________________________________________ anaconda-patches mailing list anaconda-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
On Tue, 2014-09-02 at 08:39 -0400, Samantha N. Bueno wrote:
Contains all suggestions from prior review, minus bcl's one suggestion for blivet patches, which I later realized isn't needed:
- if dev is None or dev == "":
raise ValueError("You have not specified a device number or the number is invalid")
- dev = dev.lower()
- bus = dev[:string.rfind(dev, ".") + 1]
- dev = dev[string.rfind(dev, ".") + 1:]
I'd pull the rfind out into its own variable so you can check for -1 and raise an error (assuming no . is an error)
Lack of '.' is not necessarily an error. There's now a docstring which notes valid formats for the device number, but basically either 0.0.abcd or abcd (or caps are also ok if a-f appear in the dev number).
Apart from the minor comments these all look good to me. Consider the comments and suggestions and 'fixing-locally' stuff. ACK.
anaconda-patches@lists.fedorahosted.org