[PATCH 1/3] Add support for adding zFCP devices in the GUI. (#994423)

Vratislav Podzimek vpodzime at redhat.com
Wed Nov 27 09:14:20 UTC 2013


On Tue, 2013-11-26 at 14:41 -0500, Samantha N. Bueno wrote:
> This gives s390x users the ability to add a zFCP device for use during
> installation. Even if a device is "offline", if the correct credentials
> are entered, the device will be detected and turned "online" for use.
> 
> Resolves: rhbz#994423
> ---
>  pyanaconda/constants.py                        |   1 +
>  pyanaconda/ui/gui/spokes/advstorage/zfcp.glade | 447 +++++++++++++++++++++++++
>  pyanaconda/ui/gui/spokes/advstorage/zfcp.py    | 189 +++++++++++
>  3 files changed, 637 insertions(+)
>  create mode 100644 pyanaconda/ui/gui/spokes/advstorage/zfcp.glade
>  create mode 100644 pyanaconda/ui/gui/spokes/advstorage/zfcp.py

> diff --git a/pyanaconda/ui/gui/spokes/advstorage/zfcp.py b/pyanaconda/ui/gui/spokes/advstorage/zfcp.py
> new file mode 100644
> index 0000000..5b55ec7
> --- /dev/null
> +++ b/pyanaconda/ui/gui/spokes/advstorage/zfcp.py
> @@ -0,0 +1,189 @@
> +# zFCP configuration dialog
> +#
> +# Copyright (C) 2013  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 at redhat.com>
> +#
> +
> +#from collections import namedtuple
> +# pylint: disable-msg=E0611
> +from gi.repository import GLib
> +
> +from pyanaconda import constants
> +from pyanaconda.threads import threadMgr, AnacondaThread
> +from pyanaconda.ui.gui import GUIObject
> +
> +from blivet.zfcp import ZFCPDevice
> +
> +__all__ = ["ZFCPDialog"]
> +
> +class ZFCPDialog(GUIObject):
> +    """ Gtk dialog which allows users to manually add zFCP devices without
> +        having previously specified them in a parm file.
> +    """
> +    builderObjects = ["zfcpDialog"]
> +    mainWidgetName = "zfcpDialog"
> +    uiFile = "spokes/advstorage/zfcp.glade"
> +
> +    def __init__(self, data, storage):
> +        GUIObject.__init__(self, data)
> +        self.storage = storage
> +        self.zfcp = self.storage.zfcp()
> +
> +        self._discoveryError = None
> +
> +        self._discoveredNodes = []
> +        self._update_devicetree = False
> +
> +    def refresh(self):
> +        self._zfcpNotebook = self.builder.get_object("zfcpNotebook")
> +
> +        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._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 _set_configure_sensitive(self, sensitivity):
> +        for child in self._configureGrid.get_children():
> +            if child == self._deviceEntry:
> +                self._deviceEntry.set_sensitive(True)
Could there be a case where the self._deviceEntry won't be
self._configureGrid's child? If not, it could be set sensitive outside
of the loop.

> +            elif child != self._conditionNotebook:
> +                child.set_sensitive(sensitivity)
> +
> +    def _device_valid(self, dev):
> +        """ Check whether the entered CCW entry is valid. """
> +        widget = self.builder.get_object("deviceEntry")
All objects should be pulled only once, in the refresh method (or in a
separate initialize method).

> +        text = widget.get_text()
> +
> +        try:
> +            device = dev.sanitizeDeviceInput(text)
> +            return device
> +        except ValueError as e:
> +            raise ValueError(e)
This except block does nothing and so do the other similar except block.

> +
> +    def _wwpn_valid(self, dev):
> +        """ Check whether the entered WWPN entry is valid. """
> +        widget = self.builder.get_object("wwpnEntry")
> +        text = widget.get_text()
> +
> +        try:
> +            wwpn = dev.sanitizeWWPNInput(text)
> +            return wwpn
> +        except ValueError as e:
> +            raise ValueError(e)
> +
> +    def _lun_valid(self, dev):
> +        """ Check whether the entered LUN entry is valid. """
> +        widget = self.builder.get_object("lunEntry")
> +        text = widget.get_text()
> +
> +        try:
> +            lun = dev.sanitizeFCPLInput(text)
> +            return lun
> +        except ValueError as e:
> +            raise ValueError(e)
> +
> +    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._set_configure_sensitive(False)
> +        self._deviceEntry.set_sensitive(False)
> +
> +        # Make a zFCP object with some dummy credentials so we can validate our
> +        # actual input
> +        self._conditionNotebook.set_current_page(1)
> +        dev = ZFCPDevice("0.0.0000", "0x0000000000000000", "0x0000000000000000")
> +        # below really, really is ugly and needs to be re-factored, but this
> +        # should give a good base idea as far as expected behavior should go
> +        try:
> +            device = self._device_valid(dev)
> +            wwpn = self._wwpn_valid(dev)
> +            lun = self._lun_valid(dev)
> +        except ValueError as e:
> +            _config_error = str(e)
> +            self.builder.get_object("deviceErrorLabel").set_text(_config_error)
> +            self._conditionNotebook.set_current_page(2)
> +
> +        spinner = self.builder.get_object("waitSpinner")
> +        spinner.start()
> +
> +        threadMgr.add(AnacondaThread(name=constants.THREAD_ZFCP_DISCOVER, target=self._discover, args=(device, wwpn, lun)))
> +        GLib.timeout_add(30, self._check_discover)
Is there any reason for not calling self._check_discover at the end of
the self._discover method instead of doing this 30ms polling?
self._check_discover would just need to be decorated with the
@gtk_action_nowait decorator so that it is run in the main thread.

> +
> +    def _check_discover(self, *args):
> +        """ After the zFCP discover thread runs, check to see whether a valid
> +            device was discovered. Display an error message if not.
> +        """
> +        if threadMgr.get(constants.THREAD_ZFCP_DISCOVER):
> +            return True
> +
> +        spinner = self.builder.get_object("waitSpinner")
> +        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._set_configure_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, *args):
> +        """ Given the configuration options from a user, attempt to discover
> +            a zFCP device. This includes searching black-listed devices.
> +        """
> +        # attempt to add the device
> +        try:
> +            self.zfcp.addFCP(args[0], args[1], args[2])
> +            self._update_devicetree = True
> +        except ValueError as e:
> +            self._discoveryError = str(e)
> +            return
> +
> +        if len(self._discoveredNodes) == 0:
self._discoveredNodes doesn't seem to be updated anywhere.

> +            self._discoveryErorr = "No devices discovered."
                              ^^typo

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list