[PATCH 1/3] Add support for adding zFCP devices in the GUI. (#994423)
Samantha N. Bueno
sbueno+anaconda at redhat.com
Wed Nov 27 19:54:45 UTC 2013
On Wed, Nov 27, 2013 at 10:14:20AM +0100, Vratislav Podzimek wrote:
> 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
> > <>
> > +
> > + 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.
I don't think so, so I'll add that edit.
> > + 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).
Good call; fixed locally.
> > + 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.
I'm not sure I follow. This raises an error message if the user enters a
string that does not match what is expected to be considered valid
input. In _on_start_clicked, the ValueError is caught and the error
string displayed on screen.
I do agree that it's ugly/awkward to just re-raise the very same error
which was just caught though. I can re-factor these blobs.
> > + 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.
I thought it was prudent to add the poll since it can take some time for
the devices to be discovered and switched to an "online" state.
Adding the @gtk_action_nowait decorator seems to leave the installer
stuck in an infinite loop.
> > + 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.
That is correct; removed. :)
> > + self._discoveryErorr = "No devices discovered."
> ^^typo
>
> --
> Vratislav Podzimek
>
> Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
>
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
More information about the anaconda-patches
mailing list