[master/rhel7-branch] DASD storage re-write, take III

[Samantha N. Bueno]

This now includes fixes from the second review. I had some residual
issues with thread handling and updating of the GUI spoke status as
well (that I didn't fully realize until after I posted my last set), and
those issues are fixed.

Just so things are clear, I've created four delightful and exciting
screencasts for anyone interested, so get your popcorn ready:

[graphical install]
http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm

[graphical install w/zerombr in ks]
http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm

[text install]
http://sbueno.fedorapeople.org/1001070-text-20140225.webm

[text install w/zerombr in ks]
http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm

Samantha

[blivet][master/rhel7-branch] Re-write the DASD storage code (#1001070)

[Samantha N. Bueno]

This gets rid of the DASD class and instead introduces a small
number of functions to interface with and manage DASDs. This is
completely divorced from the anaconda code and much more
lightweight.

(The corresponding Fedora bug for this is 859997.)

Resolves:rhbz#1001070

---
 blivet/__init__.py        |  13 ++-
 blivet/dasd.py            | 222 ----------------------------------------------
 blivet/devicelibs/dasd.py | 123 +++++++++++++++++++++++++
 blivet/devicetree.py      |   6 +-
 4 files changed, 132 insertions(+), 232 deletions(-)
 delete mode 100644 blivet/dasd.py
 create mode 100644 blivet/devicelibs/dasd.py

diff --git a/blivet/__init__.py b/blivet/__init__.py
index 6908b2a..b56e929 100644
--- a/blivet/__init__.py
+++ b/blivet/__init__.py
@@ -77,11 +77,11 @@ import devicefactory
 from devicelibs.dm import name_from_dm_node
 from devicelibs.crypto import generateBackupPassphrase
 from devicelibs.edd import get_edd_dict
+from devicelibs.dasd import make_dasd_list, write_dasd_conf
 from udev import udev_trigger
 import iscsi
 import fcoe
 import zfcp
-import dasd
 import util
 import arch
 from flags import flags
@@ -283,6 +283,7 @@ class Blivet(object):
         self.encryptionRetrofit = False
         self.autoPartitionRequests = []
         self.eddDict = {}
+        self.dasd = []

self.__luksDevs = {}
         self.size_sets = []
@@ -291,7 +292,6 @@ class Blivet(object):
         self.iscsi = iscsi.iscsi()
         self.fcoe = fcoe.fcoe()
         self.zfcp = zfcp.ZFCP()
-        self.dasd = dasd.DASD()

self._nextID = 0
         self._dumpFile = "%s/storage.state" % tempfile.gettempdir()
@@ -399,12 +399,11 @@ class Blivet(object):
             self.iscsi.startup()
             self.fcoe.startup()
             self.zfcp.startup()
-            self.dasd.startup(None,
-                              self.config.exclusiveDisks,
-                              self.config.initializeDisks)
+            self.dasd = make_dasd_list(self.dasd, self.devices)
+
         if self.dasd:
             # Reset the internal dasd list (823534)
-            self.dasd.clear_device_list()
+            self.dasd = []

self.devicetree.reset(conf=self.config,
                               passphrase=self.encryptionPassphrase,
@@ -1672,7 +1671,7 @@ class Blivet(object):
         self.iscsi.write(ROOT_PATH, self)
         self.fcoe.write(ROOT_PATH)
         self.zfcp.write(ROOT_PATH)
-        self.dasd.write(ROOT_PATH)
+        write_dasd_conf(self.dasd, ROOT_PATH)

def turnOnSwap(self, upgrading=None):
         self.fsset.turnOnSwap(rootPath=ROOT_PATH,
diff --git a/blivet/dasd.py b/blivet/dasd.py
deleted file mode 100644
index c2ec2e4..0000000
--- a/blivet/dasd.py
+++ /dev/null
@@ -1,222 +0,0 @@
-#
-# dasd.py - DASD class
-#
-# Copyright (C) 2009, 2010  Red Hat, Inc.  All rights reserved.
-#
-# This program is free software; you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty 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, see http://www.gnu.org/licenses/.
-#
-# Red Hat Author(s): David Cantrell 
-#
-
-import sys
-import os
-from .errors import DasdFormatError
-from .devices import deviceNameToDiskByPath
-from . import util
-from . import arch
-from .udev import udev_trigger
-
-import logging
-log = logging.getLogger("blivet")
-
-import gettext
-_ = lambda x: gettext.ldgettext("blivet", x)
-P_ = lambda x, y, z: gettext.ldngettext("blivet", x, y, z)
-
-def getDasdPorts():
-    """ Return comma delimited string of valid DASD ports. """
-    ports = []
-
-    f = open("/proc/dasd/devices", "r")
-    lines = map(lambda x: x.strip(), f.readlines())
-    f.close()
-
-    for line in lines:
-        if "unknown" in line:
-            continue
-
-        if "(FBA )" in line or "(ECKD)" in line:
-            ports.append(line.split('(')[0])
-
-    return ','.join(ports)
-
-class DASD:
-    """ Controlling class for DASD interaction before the storage code in
-        anaconda has initialized.
-
-        The DASD class can determine if any DASD devices on the system are
-        unformatted and can perform a dasdfmt on them.
-    """
-
-    def __init__(self):
-        self._dasdlist = []
-        self._devices = []                  # list of DASDDevice objects
-        self.totalCylinders = 0
-        self._completedCylinders = 0.0
-        self._maxFormatJobs = 0
-        self.dasdfmt = "/sbin/dasdfmt"
-        self.commonArgv = ["-y", "-d", "cdl", "-b", "4096"]
-        self.started = False
-
-    def __call__(self):
-        return self
-
-    def startup(self, intf, exclusiveDisks, zeroMbr):
-        """ Look for any unformatted DASDs in the system and offer the user
-            the option for format them with dasdfmt or exit the installer.
-        """
-        if self.started:
-            return
-
-        self.started = True
-
-        if not arch.isS390():
-            return
-
-        # Trigger udev data about the dasd devices on the system
-        udev_trigger(action="change", name="dasd*")
-
-        log.info("Checking for unformatted DASD devices:")
-
-        for device in os.listdir("/sys/block"):
-            if not device.startswith("dasd"):
-                continue
-
-            statusfile = "/sys/block/%s/device/status" % (device,)
-            if not os.path.isfile(statusfile):
-                continue
-
-            f = open(statusfile, "r")
-            status = f.read().strip()
-            f.close()
-
-            if status in ["unformatted"] and device not in exclusiveDisks:
-                bypath = deviceNameToDiskByPath(device)
-                if not bypath:
-                    bypath = "/dev/" + device
-
-                log.info("    %s (%s) status is %s, needs dasdfmt" % (device,
-                                                                      bypath,
-                                                                      status,))
-                self._dasdlist.append((device, bypath))
-
-        if not len(self._dasdlist):
-            log.info("    no unformatted DASD devices found")
-            return
-
-        askUser = True
-
-        if zeroMbr:
-            askUser = False
-        elif not intf and not zeroMbr:
-            log.info("    non-interactive kickstart install without zerombr "
-                     "command, unable to run dasdfmt, exiting installer")
-            sys.exit(0)
-
-        c = len(self._dasdlist)
-
-        if intf and askUser:
-            devs = ''
-            for dasd, bypath in self._dasdlist:
-                devs += "%s\n" % (bypath,)
-
-            rc = intf.questionInitializeDASD(c, devs)
-            if rc == 1:
-                log.info("    not running dasdfmt, continuing installation")
-                return
-
-        # gather total cylinder count
-        argv = ["-t", "-v"] + self.commonArgv
-        for dasd, bypath in self._dasdlist:
-            buf = util.capture_output([self.dasdfmt, argv, "/dev/" + dasd])
-            for line in buf.splitlines():
-                if line.startswith("Drive Geometry: "):
-                    # line will look like this:
-                    # Drive Geometry: 3339 Cylinders * 15 Heads =  50085 Tracks
-                    cyls = long(filter(lambda s: s, line.split(' '))[2])
-                    self.totalCylinders += cyls
-                    break
-
-        # format DASDs
-        argv = ["-P"] + self.commonArgv
-        update = self._updateProgressWindow
-
-        title = P_("Formatting DASD Device", "Formatting DASD Devices", c)
-        msg = P_("Preparing %d DASD device for use with Linux..." % c,
-                 "Preparing %d DASD devices for use with Linux..." % c, c)
-
-        if intf:
-            if self.totalCylinders:
-                pw = intf.progressWindow(title, msg, 1.0)
-            else:
-                pw = intf.progressWindow(title, msg, 100, pulse=True)
-
-        for dasd, bypath in self._dasdlist:
-            log.info("Running dasdfmt on %s" % (bypath,))
-            arglist = argv + ["/dev/" + dasd]
-
-            try:
-                rc = util.run_program([self.dasdfmt] + arglist)
-            except Exception as e:
-                raise DasdFormatError(e, bypath)
-
-            if rc:
-                raise DasdFormatError("dasdfmt failed: %s" % rc, bypath)
-
-        if intf:
-            pw.pop()
-
-    def addDASD(self, dasd):
-        """ Adds a DASDDevice to the internal list of DASDs. """
-        if dasd and dasd not in self._devices:
-            self._devices.append(dasd)
-
-    def removeDASD(self, dasd):
-        """ Removes a DASDDevice from the internal list of DASDs. """
-        if dasd and dasd in self._devices:
-            self._devices.remove(dasd)
-
-    def clear_device_list(self):
-        """ Clear the device list to force re-populate on next access. """
-        self._devices = []
-
-    def write(self, ROOT_PATH):
-        """ Write /etc/dasd.conf to target system for all DASD devices
-            configured during installation.
-        """
-        if self._devices == []:
-            return
-
-        f = open(os.path.realpath(ROOT_PATH + "/etc/dasd.conf"), "w")
-        for dasd in sorted(self._devices, key=lambda d: d.name):
-            fields = [dasd.busid] + dasd.getOpts()
-            f.write("%s\n" % (" ".join(fields),))
-        f.close()
-
-    def _updateProgressWindow(self, data, callback_data=None):
-        """ Reads progress output from dasdfmt and collects the number of
-            cylinders completed so the progress window can update.
-        """
-        if not callback_data:
-            return
-
-        if data == '\n':
-            # each newline we see in this output means one more cylinder done
-            self._completedCylinders += 1.0
-            callback_data.set(self._completedCylinders / self.totalCylinders)
-
-# Create DASD singleton
-DASD = DASD()
-
-# vim:tw=78:ts=4:et:sw=4
diff --git a/blivet/devicelibs/dasd.py b/blivet/devicelibs/dasd.py
new file mode 100644
index 0000000..8281a04
--- /dev/null
+++ b/blivet/devicelibs/dasd.py
@@ -0,0 +1,123 @@
+#
+# dasd.py - DASD functions
+#
+# Copyright (C) 2013 Red Hat, Inc.  All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty 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, see http://www.gnu.org/licenses/.
+#
+# Red Hat Author(s): Samantha N. Bueno
+#
+
+import os
+from blivet.errors import DasdFormatError
+from blivet.devices import deviceNameToDiskByPath
+from blivet import util
+from blivet import arch
+
+import logging
+log = logging.getLogger("blivet")
+
+import gettext
+_ = lambda x: gettext.ldgettext("blivet", x)
+P_ = lambda x, y, z: gettext.ldngettext("blivet", x, y, z)
+
+def get_dasd_ports():
+    """ Return comma delimited string of valid DASD ports. """
+    ports = []
+
+    with open("/proc/dasd/devices", "r") as f:
+        lines = (line.strip() for line in f.readlines())
+
+    for line in lines:
+        if "unknown" in line:
+            continue
+
+        if "(FBA )" in line or "(ECKD)" in line:
+            ports.append(line.split('(')[0])
+
+    return ','.join(ports)
+
+def format_dasd(dasd):
+    """ Run dasdfmt on a DASD. Aside from one type of device noted below, this
+        function _does not_ check if a DASD needs to be formatted, but rather,
+        assumes the list passed needs formatting.
+
+        We don't need to show or update any progress bars, since disk actions
+        will be taking place all in the progress hub, which is just one big
+        progress bar.
+    """
+    try:
+        rc = util.run_program(["/sbin/dasdfmt", "-y", "-d", "cdl", "-b", "4096", "/dev/" + dasd])
+    except Exception as err:
+        raise DasdFormatError(err)
+
+    if rc:
+        raise DasdFormatError("dasdfmt failed: %s" % rc)
+
+def make_dasd_list(dasds, disks):
+    """ Create a list of DASDs recognized by the system. """
+    if not arch.isS390():
+        return
+
+    log.info("Generating DASD list...")
+    for dev in (d for d in disks if d.type == "dasd"):
+        if dev not in dasds:
+            dasds.append(dev)
+
+    return dasds
+
+def make_unformatted_dasd_list(dasds):
+    """ Return a list of DASDS which are not formatted. """
+    unformatted = []
+
+    for dasd in dasds:
+        if dasd_needs_format(dasd):
+            unformatted.append(dasd)
+
+    return unformatted
+
+def dasd_needs_format(dasd):
+    """ Check if a DASD needs to have dasdfmt run against it or not.
+        Return True if we do need dasdfmt, False if not.
+    """
+    statusfile = "/sys/block/%s/device/status" % (dasd,)
+    if not os.path.isfile(statusfile):
+        return False
+
+    with open(statusfile, "r") as f:
+        status = f.read().strip()
+
+    if status in ["unformatted"]:
+        bypath = deviceNameToDiskByPath(dasd)
+        if not bypath:
+            bypath = "/dev/" + dasd
+
+        log.info("  %s (%s) status is %s, needs dasdfmt" % (dasd, bypath,
+                                                            status,))
+        return True
+
+    return False
+
+
+def write_dasd_conf(disks, ROOT_PATH):
+    """ Write /etc/dasd.conf to target system for all DASD devices
+        configured during installation.
+    """
+    if disks == {}:
+        return
+
+    with open(os.path.realpath(ROOT_PATH + "/etc/dasd.conf"), "w") as f:
+        for dasd in sorted(disks, key=lambda d: d.name):
+            fields = [dasd.busid] + dasd.getOpts()
+            f.write("%s\n" % (" ".join(fields),))
diff --git a/blivet/devicetree.py b/blivet/devicetree.py
index 8a79f14..fa28898 100644
--- a/blivet/devicetree.py
+++ b/blivet/devicetree.py
@@ -946,7 +946,7 @@ class DeviceTree(object):
             info["ID_FS_TYPE"] = "multipath_member"

if diskType == DASDDevice:
-            self.dasd.addDASD(device)
+            self.dasd.append(device)

self._addDevice(device)
         return device
@@ -1830,7 +1830,7 @@ class DeviceTree(object):
         lvm.lvm_cc_addFilterRejectRegexp(device.name)

if isinstance(device, DASDDevice):
-            self.dasd.removeDASD(device)
+            self.dasd.remove(device)

def unhide(self, device):
         # the hidden list should be in leaves-first order
@@ -1846,7 +1846,7 @@ class DeviceTree(object):
                     parent.addChild()

if isinstance(device, DASDDevice):
-                    self.dasd.addDASD(device)
+                    self.dasd.append(device)

def setupDiskImages(self):
         """ Set up devices to represent the disk image files. """

[blivet][master/rhel7-branch] Re-write the DASD storage code (#1001070)

[Vratislav Podzimek]

> diff --git a/blivet/devicelibs/dasd.py b/blivet/devicelibs/dasd.py
> new file mode 100644
> index 0000000..8281a04
> --- /dev/null
> +++ b/blivet/devicelibs/dasd.py
> @@ -0,0 +1,123 @@
> +#
> +# dasd.py - DASD functions
> +#
> +# Copyright (C) 2013 Red Hat, Inc.  All rights reserved.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty 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, see http://www.gnu.org/licenses/.
> +#
> +# Red Hat Author(s): Samantha N. Bueno
> +#
>
> +import os
> +from blivet.errors import DasdFormatError
> +from blivet.devices import deviceNameToDiskByPath
> +from blivet import util
> +from blivet import arch
>
> +import logging
> +log = logging.getLogger("blivet")
>
> +import gettext
> +_ = lambda x: gettext.ldgettext("blivet", x)
> +P_ = lambda x, y, z: gettext.ldngettext("blivet", x, y, z)
>
> +def get_dasd_ports():
> """ Return comma delimited string of valid DASD ports. """
> ports = []
> with open("/proc/dasd/devices", "r") as f:
>    lines = (line.strip() for line in f.readlines())
> for line in lines:
>    if "unknown" in line:
>        continue
>    if "(FBA )" in line or "(ECKD)" in line:
>        ports.append(line.split('(')[0])
> return ','.join(ports)
>
> +def format_dasd(dasd):
> """ Run dasdfmt on a DASD. Aside from one type of device noted below, this
>    function _does not_ check if a DASD needs to be formatted, but rather,
>    assumes the list passed needs formatting.
>    We don't need to show or update any progress bars, since disk actions
>    will be taking place all in the progress hub, which is just one big
>    progress bar.
> """
> try:
>    rc = util.run_program(["/sbin/dasdfmt", "-y", "-d", "cdl", "-b", "4096", "/dev/" + dasd])
> except Exception as err:
>    raise DasdFormatError(err)
> if rc:
>    raise DasdFormatError("dasdfmt failed: %s" % rc)
>
> +def make_dasd_list(dasds, disks):
> """ Create a list of DASDs recognized by the system. """
> if not arch.isS390():
>    return
> log.info("Generating DASD list...")
> for dev in (d for d in disks if d.type == "dasd"):
>    if dev not in dasds:
>        dasds.append(dev)
> return dasds
>
> +def make_unformatted_dasd_list(dasds):
> """ Return a list of DASDS which are not formatted. """
> unformatted = []
> for dasd in dasds:
>    if dasd_needs_format(dasd):
>        unformatted.append(dasd)
> return unformatted
>
> +def dasd_needs_format(dasd):
> """ Check if a DASD needs to have dasdfmt run against it or not.
>    Return True if we do need dasdfmt, False if not.
> """
> statusfile = "/sys/block/%s/device/status" % (dasd,)
> if not os.path.isfile(statusfile):
>    return False
> with open(statusfile, "r") as f:
>    status = f.read().strip()
> if status in ["unformatted"]:
>    bypath = deviceNameToDiskByPath(dasd)
>    if not bypath:
>        bypath = "/dev/" + dasd
>    log.info("  %s (%s) status is %s, needs dasdfmt" % (dasd, bypath,
>                                                        status,))
>    return True
> return False
>
> +def write_dasd_conf(disks, ROOT_PATH):
> """ Write /etc/dasd.conf to target system for all DASD devices
>    configured during installation.
> """
> if disks == {}:
>    return
> with open(os.path.realpath(ROOT_PATH + "/etc/dasd.conf"), "w") as f:
>    for dasd in sorted(disks, key=lambda d: d.name):
>        fields = [dasd.busid] + dasd.getOpts()
>        f.write("%s\n" % (" ".join(fields),))

No need for the extra brackets and comma around the " ".join(fields).

[anaconda][master/rhel7-branch][PATCH 1/3] Add dialog box to warn about formatting DASDs. (#1064423)

[Samantha N. Bueno]

A GtkDialog box which shows the progress of dasdfmt as it is run
against unformatted DASDs.

Resolves: rhbz#1064423
---
 po/POTFILES.in                             |   2 +
 pyanaconda/ui/gui/spokes/lib/dasdfmt.glade | 211 +++++++++++++++++++++++++++++
 pyanaconda/ui/gui/spokes/lib/dasdfmt.py    | 132 ++++++++++++++++++
 3 files changed, 345 insertions(+)
 create mode 100644 pyanaconda/ui/gui/spokes/lib/dasdfmt.glade
 create mode 100644 pyanaconda/ui/gui/spokes/lib/dasdfmt.py

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 3d53ce4..4b341fc 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -76,6 +76,7 @@ pyanaconda/ui/gui/spokes/user.py
 pyanaconda/ui/gui/spokes/welcome.py
 pyanaconda/ui/gui/spokes/lib/accordion.py
 pyanaconda/ui/gui/spokes/lib/cart.py
+pyanaconda/ui/gui/spokes/lib/dasdfmt.py
 pyanaconda/ui/gui/spokes/lib/passphrase.py
 pyanaconda/ui/gui/spokes/lib/refresh.py
 pyanaconda/ui/gui/spokes/lib/resize.py
@@ -99,6 +100,7 @@ pyanaconda/ui/gui/spokes/advstorage/fcoe.glade
 pyanaconda/ui/gui/spokes/advstorage/iscsi.glade
 pyanaconda/ui/gui/spokes/advstorage/zfcp.glade
 pyanaconda/ui/gui/spokes/lib/cart.glade
+pyanaconda/ui/gui/spokes/lib/dasdfmt.glade
 pyanaconda/ui/gui/spokes/lib/detailederror.glade
 pyanaconda/ui/gui/spokes/lib/passphrase.glade
 pyanaconda/ui/gui/spokes/lib/refresh.glade
diff --git a/pyanaconda/ui/gui/spokes/lib/dasdfmt.glade b/pyanaconda/ui/gui/spokes/lib/dasdfmt.glade
new file mode 100644
index 0000000..2fdb0c9
--- /dev/null
+++ b/pyanaconda/ui/gui/spokes/lib/dasdfmt.glade
@@ -0,0 +1,211 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<interface>
+  <!-- interface-requires gtk+ 3.6 -->
+  <object class="GtkDialog" id="unformattedDasdDialog">
+    <property name="can_focus">False</property>
+    <property name="border_width">5</property>
+    <property name="title" translatable="yes">UNFORMATTED DASDS</property>
+    <property name="modal">True</property>
+    <property name="type_hint">dialog</property>
+    <property name="decorated">False</property>
+    <signal name="show" handler="dasdfmt" swapped="no"/>
+    <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>
+          <object class="GtkLabel" id="unformattedDasdsLabel">
+            <property name="visible">True</property>
+            <property name="can_focus">False</property>
+            <property name="label" translatable="yes">Formatting DASDs</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="formatNotebook">
+            <property name="visible">True</property>
+            <property name="can_focus">False</property>
+            <property name="show_tabs">False</property>
+            <child>
+              <placeholder/>
+            </child>
+            <child type="tab">
+              <placeholder/>
+            </child>
+            <child>
+              <object class="GtkGrid" id="formatRunning">
+                <property name="visible">True</property>
+                <property name="can_focus">False</property>
+                <property name="halign">center</property>
+                <property name="valign">center</property>
+                <property name="row_spacing">18</property>
+                <property name="column_spacing">6</property>
+                <child>
+                  <object class="GtkSpinner" id="spinner4">
+                    <property name="visible">True</property>
+                    <property name="can_focus">False</property>
+                    <property name="active">True</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="udLabel">
+                    <property name="visible">True</property>
+                    <property name="can_focus">False</property>
+                    <property name="label">Inform user which DASD is being formatted here.</property>
+                  </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="returnToHubLabel1">
+                    <property name="visible">True</property>
+                    <property name="can_focus">True</property>
+                    <property name="label" translatable="yes">You may &lt;a href=""&gt;go back to the main menu&lt;/a&gt; to complete other
+installation options while this operation completes.</property>
+                    <property name="use_markup">True</property>
+                    <property name="wrap">True</property>
+                    <property name="track_visited_links">False</property>
+                    <signal name="activate-link" handler="return_to_hub_link_clicked" swapped="no"/>
+                  </object>
+                  <packing>
+                    <property name="left_attach">0</property>
+                    <property name="top_attach">1</property>
+                    <property name="width">2</property>
+                    <property name="height">1</property>
+                  </packing>
+                </child>
+              </object>
+              <packing>
+                <property name="position">1</property>
+              </packing>
+            </child>
+            <child type="tab">
+              <placeholder/>
+            </child>
+            <child>
+              <object class="GtkGrid" id="formatComplete">
+                <property name="visible">True</property>
+                <property name="can_focus">False</property>
+                <property name="halign">center</property>
+                <property name="valign">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="stock">gtk-apply</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="label19">
+                    <property name="visible">True</property>
+                    <property name="can_focus">False</property>
+                    <property name="xalign">0</property>
+                    <property name="label" translatable="yes">Disk formatting complete.</property>
+                    <property name="wrap">True</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>
+              </object>
+              <packing>
+                <property name="position">2</property>
+              </packing>
+            </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">3</property>
+          </packing>
+        </child>
+        <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="udCancelButton">
+                <property name="label" translatable="yes">_Cancel</property>
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="receives_default">True</property>
+                <property name="use_underline">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="udOKButton">
+                <property name="label" translatable="yes">_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_underline">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">4</property>
+          </packing>
+        </child>
+      </object>
+    </child>
+    <action-widgets>
+      <action-widget response="0">udCancelButton</action-widget>
+      <action-widget response="1">udOKButton</action-widget>
+    </action-widgets>
+  </object>
+</interface>
diff --git a/pyanaconda/ui/gui/spokes/lib/dasdfmt.py b/pyanaconda/ui/gui/spokes/lib/dasdfmt.py
new file mode 100644
index 0000000..933fcaa
--- /dev/null
+++ b/pyanaconda/ui/gui/spokes/lib/dasdfmt.py
@@ -0,0 +1,132 @@
+# DASD format 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 
+#                    Chris Lumens 
+#
+
+from pyanaconda.threads import threadMgr, AnacondaThread
+from pyanaconda.ui.gui import GUIObject
+from pyanaconda.ui.gui.utils import gtk_call_once
+from pyanaconda import constants
+from pyanaconda.i18n import _
+import threading
+
+from blivet import storageInitialize
+from blivet.devicelibs.dasd import format_dasd
+from blivet.errors import DasdFormatError
+
+import logging
+log = logging.getLogger("anaconda")
+
+__all__ = ["DasdFormatDialog"]
+
+class DasdFormatDialog(GUIObject):
+    builderObjects = ["unformattedDasdDialog"]
+    mainWidgetName = "unformattedDasdDialog"
+    uiFile = "spokes/lib/dasdfmt.glade"
+
+    def __init__(self, data, storage, to_format):
+        GUIObject.__init__(self, data)
+
+        self.storage = storage
+        self.to_format = to_format
+
+        self._notebook = self.builder.get_object("formatNotebook")
+        self._cancel_button = self.builder.get_object("udCancelButton")
+        self._ok_button = self.builder.get_object("udOKButton")
+        self._label = self.builder.get_object("udLabel")
+
+        # epoch is increased when a dasd is formatted
+        self._epoch = 0
+        self._epoch_lock = threading.Lock()
+
+    def run(self):
+        rc = self.window.run()
+        self.window.destroy()
+
+        if rc == 2:
+            # User clicked uri to return to hub. We need to catch this here so
+            # that we can halt all further dialog actions
+            self._epoch_lock.acquire()
+            self._epoch += 1
+            self._epoch_lock.release()
+        return rc
+
+    def run_dasdfmt(self, epoch_started, *args):
+        """
+        Loop through our disks and run dasdfmt against them. After that loop has
+        finished, launch stage two of this operation (finish_dasdfmt).
+        """
+
+        for disk in self.to_format:
+            try:
+                #self._label.set_text(_("Formatting /dev/%s. This may take a moment." % disk))
+                gtk_call_once(self._label.set_text, _("Formatting /dev/%s. This may take a moment" % disk))
+                format_dasd(disk)
+            except DasdFormatError as err:
+                # Log errors if formatting fails, but don't halt the installer
+                log.error(str(err))
+                continue
+
+        gtk_call_once(self.finish_dasdfmt, epoch_started)
+
+    def dasdfmt(self, *args):
+        """
+        This is the handler that gets called from the GtkDialog and sets things
+        in motion. We launch the *actual* call to run dasdfmt from a thread.
+        """
+        # Run through all of the formatting
+        self._cancel_button.set_sensitive(True)
+        self._ok_button.set_sensitive(False)
+        self._notebook.set_current_page(0)
+
+        # Loop through all of our unformatted DASDs and format them
+        threadMgr.add(AnacondaThread(name=constants.THREAD_DASDFMT,
+                                     target=self.run_dasdfmt, args=(self._epoch,)))
+
+    def finish_dasdfmt(self, epoch_started):
+        """
+        This is the second stage of the dasdfmt operation; now that formatting
+        is complete, we need to reinitialize storage so that the newly
+        formatted devices are added properly to the devicetree.
+        """
+        self._epoch_lock.acquire()
+        protectedNames = [d.name for d in self.storage.protectedDevices]
+        threadMgr.add(AnacondaThread(name=constants.THREAD_STORAGE, target=storageInitialize,
+                                     args=(self.storage, self.data, protectedNames)))
+
+        threadMgr.wait(constants.THREAD_STORAGE)
+
+        if epoch_started == self._epoch:
+            # we only want to change anything on the dialog if we are in the
+            # same epoch as it; the only time we should not be running these
+            # commands is if a user clicks return_to_hub
+            self._notebook.set_current_page(1)
+            self._cancel_button.set_sensitive(False)
+            self._ok_button.set_sensitive(True)
+        self._epoch_lock.release()
+
+    def return_to_hub_link_clicked(self, label, uri):
+        """
+        The user clicked on the link that takes them back to the hub.  We need
+        to emit a special response ID indicating the user did not press OK.
+
+        NOTE: There is no button with response_id=2.
+        """
+        self.window.response(2)

[anaconda][master/rhel7-branch][PATCH 1/3] Add dialog box to warn about formatting DASDs. (#1064423)

[Vratislav Podzimek]

On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> A GtkDialog box which shows the progress of dasdfmt as it is run
> against unformatted DASDs.
> Resolves: rhbz#1064423
>
> po/POTFILES.in                             |   2 +
>  pyanaconda/ui/gui/spokes/lib/dasdfmt.glade | 211 +++++++++++++++++++++++++++++
>  pyanaconda/ui/gui/spokes/lib/dasdfmt.py    | 132 ++++++++++++++++++
>  3 files changed, 345 insertions(+)
>  create mode 100644 pyanaconda/ui/gui/spokes/lib/dasdfmt.glade
>  create mode 100644 pyanaconda/ui/gui/spokes/lib/dasdfmt.py
> diff --git a/pyanaconda/ui/gui/spokes/lib/dasdfmt.py b/pyanaconda/ui/gui/spokes/lib/dasdfmt.py
> new file mode 100644
> index 0000000..933fcaa
> --- /dev/null
> +++ b/pyanaconda/ui/gui/spokes/lib/dasdfmt.py
> @@ -0,0 +1,132 @@
> +# DASD format 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 
> +#                    Chris Lumens 
> +#
>
> +from pyanaconda.threads import threadMgr, AnacondaThread
> +from pyanaconda.ui.gui import GUIObject
> +from pyanaconda.ui.gui.utils import gtk_call_once
> +from pyanaconda import constants
> +from pyanaconda.i18n import _
> +import threading
>
> +from blivet import storageInitialize
> +from blivet.devicelibs.dasd import format_dasd
> +from blivet.errors import DasdFormatError
>
> +import logging
> +log = logging.getLogger("anaconda")
>
> +__all__ = ["DasdFormatDialog"]
>
> +class DasdFormatDialog(GUIObject):
> builderObjects = ["unformattedDasdDialog"]
> mainWidgetName = "unformattedDasdDialog"
> uiFile = "spokes/lib/dasdfmt.glade"
> def __init__(self, data, storage, to_format):
>    GUIObject.__init__(self, data)
>    self.storage = storage
>    self.to_format = to_format
>    self._notebook = self.builder.get_object("formatNotebook")
>    self._cancel_button = self.builder.get_object("udCancelButton")
>    self._ok_button = self.builder.get_object("udOKButton")
>    self._label = self.builder.get_object("udLabel")
>    # epoch is increased when a dasd is formatted
>    self._epoch = 0
>    self._epoch_lock = threading.Lock()
> def run(self):
>    rc = self.window.run()
>    self.window.destroy()
>    if rc == 2:
>        # User clicked uri to return to hub. We need to catch this here so
>        # that we can halt all further dialog actions
>        self._epoch_lock.acquire()
>        self._epoch += 1
>        self._epoch_lock.release()

The threading.Lock() objects have support for the with block. So you can
reduce these three lines into:
    with self._epoch_lock:
        self._epoch += 1

I see it's not this way in the NTPConfigDialog. We could have it nice at
least here. :)
>    return rc
> def run_dasdfmt(self, epoch_started, *args):
>    """
>    Loop through our disks and run dasdfmt against them. After that loop has
>    finished, launch stage two of this operation (finish_dasdfmt).
>    """
>    for disk in self.to_format:
>        try:
>            #self._label.set_text(_("Formatting /dev/%s. This may take a moment." % disk))

Leftover ;)
>            gtk_call_once(self._label.set_text, _("Formatting /dev/%s. This may take a moment" % disk))
>            format_dasd(disk)
>        except DasdFormatError as err:
>            # Log errors if formatting fails, but don't halt the installer
>            log.error(str(err))
>            continue
>    gtk_call_once(self.finish_dasdfmt, epoch_started)
> def dasdfmt(self, *args):
>    """
>    This is the handler that gets called from the GtkDialog and sets things
>    in motion. We launch the *actual* call to run dasdfmt from a thread.
>    """
>    # Run through all of the formatting
>    self._cancel_button.set_sensitive(True)
>    self._ok_button.set_sensitive(False)
>    self._notebook.set_current_page(0)
>    # Loop through all of our unformatted DASDs and format them
>    threadMgr.add(AnacondaThread(name=constants.THREAD_DASDFMT,
>                                 target=self.run_dasdfmt, args=(self._epoch,)))
> def finish_dasdfmt(self, epoch_started):
>    """
>    This is the second stage of the dasdfmt operation; now that formatting
>    is complete, we need to reinitialize storage so that the newly
>    formatted devices are added properly to the devicetree.
>    """
>    self._epoch_lock.acquire()
>    protectedNames = [d.name for d in self.storage.protectedDevices]
>    threadMgr.add(AnacondaThread(name=constants.THREAD_STORAGE, target=storageInitialize,
>                                 args=(self.storage, self.data, protectedNames)))
>    threadMgr.wait(constants.THREAD_STORAGE)
>    if epoch_started == self._epoch:
>        # we only want to change anything on the dialog if we are in the
>        # same epoch as it; the only time we should not be running these
>        # commands is if a user clicks return_to_hub
>        self._notebook.set_current_page(1)
>        self._cancel_button.set_sensitive(False)
>        self._ok_button.set_sensitive(True)
>    self._epoch_lock.release()
> def return_to_hub_link_clicked(self, label, uri):
>    """
>    The user clicked on the link that takes them back to the hub.  We need
>    to emit a special response ID indicating the user did not press OK.
>    NOTE: There is no button with response_id=2.
>    """
>    self.window.response(2)

[anaconda][master/rhel7-branch][PATCH 2/3] Show unformatted DASDs in the local disk store. (#1064423)

[Samantha N. Bueno]

Since the installer can handle unformatted DASDs, all of them should
be viewable and selectable from the storage spoke.

Resolves: rhbz#1064423
---
 pyanaconda/ui/lib/disks.py | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/pyanaconda/ui/lib/disks.py b/pyanaconda/ui/lib/disks.py
index 486208b..a6a7040 100644
--- a/pyanaconda/ui/lib/disks.py
+++ b/pyanaconda/ui/lib/disks.py
@@ -53,11 +53,18 @@ def getDisks(devicetree, fake=False):
         if not flags.imageInstall:
             devices += devicetree._hidden

-        disks = [d for d in devices if d.isDisk and
-                                       d.mediaPresent and
-                                       not d.format.hidden and
-                                       not (d.protected and
-                                            d.removable)]
+        disks = []
+        for d in devices:
+            if d.isDisk and not d.format.hidden and not (d.protected and d.removable):
+                # unformatted DASDs are detected with a size of 0, but they should
+                # still show up as valid disks if this function is called, since we
+                # can still use them; anaconda will know how to handle them, so they
+                # don't need to be ignored anymore
+                if d.type == "dasd":
+                    disks.append(d)
+                elif d.size > 0 and d.mediaPresent:
+                    disks.append(d)
+
     else:
         disks = []
         disks.append(FakeDisk("sda", size=300000, free=10000, serial="00001",

[anaconda][master/rhel7-branch][PATCH 3/3] Add GUI and TUI logic to handle unformatted DASDs. (#1064423)

[Samantha N. Bueno]

In both graphical and text, if a user selects unformatted DASDs
from the local disk store, they are formatted when the user opts
to proceed (either by clicking 'Done' in the GUI or 'c' to
continue in the TUI).

Following dasdfmt, storage initialization is run again in order
to properly add the new DASDs to the devicetree and seen by the
installer.

Resolves:rhbz#1064423
---
 pyanaconda/constants.py             |  1 +
 pyanaconda/ui/gui/spokes/storage.py | 81 ++++++++++++++++++++++++++++++++++++-
 pyanaconda/ui/tui/spokes/storage.py | 50 +++++++++++++++++++++--
 3 files changed, 127 insertions(+), 5 deletions(-)

diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
index cf8a8ea..8de29a9 100644
--- a/pyanaconda/constants.py
+++ b/pyanaconda/constants.py
@@ -131,6 +131,7 @@ THREAD_ISCSI_LOGIN = "AnaIscsiLoginThread"
 THREAD_GEOLOCATION_REFRESH = "AnaGeolocationRefreshThread"
 THREAD_DATE_TIME = "AnaDateTimeThread"
 THREAD_TIME_INIT = "AnaTimeInitThread"
+THREAD_DASDFMT = "AnaDasdfmtThread"

# Geolocation constants

diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index bdd48ef..e565b16 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -50,17 +50,20 @@ from pyanaconda.ui.gui.spokes.lib.cart import SelectedDisksDialog
 from pyanaconda.ui.gui.spokes.lib.passphrase import PassphraseDialog
 from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog
 from pyanaconda.ui.gui.spokes.lib.resize import ResizeDialog
+from pyanaconda.ui.gui.spokes.lib.dasdfmt import DasdFormatDialog
 from pyanaconda.ui.gui.categories.system import SystemCategory
-from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_action_wait
+from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_action_wait, ignoreEscape

from pyanaconda.kickstart import doKickstartStorage, getAvailableDiskSpace
+from blivet import storageInitialize, arch
 from blivet.size import Size
 from blivet.devices import MultipathDevice
-from blivet.errors import StorageError
+from blivet.errors import StorageError, DasdFormatError
 from blivet.errors import SanityError
 from blivet.errors import SanityWarning
 from blivet.platform import platform
 from blivet.devicelibs import swap as swap_lib
+from blivet.devicelibs.dasd import make_unformatted_dasd_list, format_dasd
 from pyanaconda.threads import threadMgr, AnacondaThread
 from pyanaconda.product import productName
 from pyanaconda.flags import flags
@@ -292,6 +295,11 @@ class StorageSpoke(NormalSpoke, StorageChecker):
         self.autoPartType = None
         self.clearPartType = CLEARPART_TYPE_NONE

+        if self.data.zerombr.zerombr and arch.isS390():
+            # run dasdfmt on any unformatted DASDs automatically
+            threadMgr.add(AnacondaThread(name=constants.THREAD_DASDFMT,
+                            target=self.run_dasdfmt))
+
         self._previous_autopart = False

self._last_clicked_overview = None
@@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):
     def _doExecute(self):
         self._ready = False
         hubQ.send_not_ready(self.__class__.__name__)
+        # ugly and stupid, but on the off-chance dasdfmt is running, we can't
+        # proceed further with this
+        while threadMgr.get(constants.THREAD_DASDFMT):
+            continue
         hubQ.send_message(self.__class__.__name__, _("Saving storage configuration..."))
         try:
             doKickstartStorage(self.storage, self.data, self.instclass)
@@ -409,6 +421,8 @@ class StorageSpoke(NormalSpoke, StorageChecker):

if not self._confirmed:
             msg = _("Not configured")
+        elif threadMgr.get(constants.THREAD_DASDFMT):
+            msg = _("Formatting DASDs")
         elif flags.automatedInstall and not self.storage.rootDevice:
             return msg
         elif self.data.ignoredisk.onlyuse:
@@ -681,6 +695,41 @@ class StorageSpoke(NormalSpoke, StorageChecker):
             if not selected and name in self.selected_disks:
                 self.selected_disks.remove(name)

+    def run_dasdfmt(self):
+        """
+        Though the same function exists in pyanaconda.ui.gui.spokes.lib.dasdfmt,
+        this instance doesn't include any of the UI pieces and should only
+        really be getting called on ks installations with "zerombr".
+        """
+        # wait for the initial storage thread to complete before taking any new
+        # actions on storage devices
+        threadMgr.wait(constants.THREAD_STORAGE)
+
+        to_format = make_unformatted_dasd_list(self.selected_disks)
+        if not to_format:
+            # nothing to do here; bail
+            return
+
+        hubQ.send_message(self.__class__.__name__, _("Formatting DASDs"))
+        for disk in to_format:
+            try:
+                format_dasd(disk)
+            except DasdFormatError as err:
+                # Log errors if formatting fails, but don't halt the installer
+                log.error(str(err))
+                continue
+
+        # now re-initialize storage to pick up the newly formatted disks
+        protectedNames = [d.name for d in self.storage.protectedDevices]
+        threadMgr.add(AnacondaThread(name=constants.THREAD_STORAGE,
+                        target=storageInitialize,
+                        args=(self.storage, self.data, protectedNames)))
+        threadMgr.wait(constants.THREAD_STORAGE)
+
+        # I really hate doing this, but the way is the way; probably the most
+        # correct way to kajigger the storage spoke into becoming ready
+        self.execute()
+
     # signal handlers
     def on_summary_clicked(self, button):
         # show the selected disks dialog
@@ -769,6 +818,34 @@ class StorageSpoke(NormalSpoke, StorageChecker):
             NormalSpoke.on_back_clicked(self, button)
             return

+        if arch.isS390():
+            # check for unformatted DASDs and launch dasdfmt if any discovered
+            dasds = make_unformatted_dasd_list(self.selected_disks)
+            if len(dasds) > 0:
+                dialog = DasdFormatDialog(self.data, self.storage, dasds)
+                ignoreEscape(dialog.window)
+                rc = self.run_lightbox_dialog(dialog)
+                if rc == 1:
+                    # User hit OK on the dialog, indicating they stayed on the
+                    # dialog until formatting completed and now needs to go back
+                    # to the main storage spoke.
+                    dialog.window.destroy()
+                    # make sure we stay on the storage spoke and don't return to
+                    # the summary hub
+                    self.skipTo = "StorageSpoke"
+                    # we have to manaually call refresh so changes are picked up
+                    self.refresh()
+                elif rc == 2:
+                    # User clicked uri to return to hub.
+                    NormalSpoke.on_back_clicked(self, button)
+                    return
+                elif rc != 2:
+                    # User either hit cancel on the dialog or closed it via escape, so
+                    # there was no formatting done.
+                    # NOTE: rc == 2 means the user clicked on the link that takes them
+                    # back to the hub.
+                    return
+
         # Figure out if the existing disk labels will work on this platform
         # you need to have at least one of the platform's labels in order for
         # any of the free space to be useful.
diff --git a/pyanaconda/ui/tui/spokes/storage.py b/pyanaconda/ui/tui/spokes/storage.py
index b4d40f1..21e67e2 100644
--- a/pyanaconda/ui/tui/spokes/storage.py
+++ b/pyanaconda/ui/tui/spokes/storage.py
@@ -27,15 +27,17 @@ from pyanaconda.ui.tui.spokes import NormalTUISpoke
 from pyanaconda.ui.tui.simpleline import TextWidget, CheckboxWidget

from pykickstart.constants import AUTOPART_TYPE_LVM, AUTOPART_TYPE_BTRFS, AUTOPART_TYPE_PLAIN
+from blivet import storageInitialize, arch
 from blivet.size import Size
-from blivet.errors import StorageError
+from blivet.errors import StorageError, DasdFormatError
 from blivet.errors import SanityError
 from blivet.errors import SanityWarning
 from blivet.devices import DASDDevice, FcoeDiskDevice, iScsiDiskDevice, MultipathDevice, ZFCPDiskDevice
+from blivet.devicelibs.dasd import format_dasd, make_unformatted_dasd_list
 from pyanaconda.flags import flags
 from pyanaconda.kickstart import doKickstartStorage
 from pyanaconda.threads import threadMgr, AnacondaThread
-from pyanaconda.constants import THREAD_STORAGE, THREAD_STORAGE_WATCHER
+from pyanaconda.constants import THREAD_STORAGE, THREAD_STORAGE_WATCHER, THREAD_DASDFMT
 from pyanaconda.i18n import _, P_, N_
 from pyanaconda.bootloader import BootLoaderError

@@ -75,6 +77,13 @@ class StorageSpoke(NormalTUISpoke):
         self.errors = []
         self.warnings = []

+        if self.data.zerombr.zerombr and arch.isS390():
+            # if zerombr is specified in a ks file and there are unformatted
+            # dasds, automatically format them
+            to_format = make_unformatted_dasd_list(self.selected_disks)
+            if to_format:
+                self.run_dasdfmt(to_format)
+
         if not flags.automatedInstall:
             # default to using autopart for interactive installs
             self.data.autopart.autopart = True
@@ -89,7 +98,7 @@ class StorageSpoke(NormalTUISpoke):
     def ready(self):
         # By default, the storage spoke is not ready.  We have to wait until
         # storageInitialize is done.
-        return self._ready and not threadMgr.get(THREAD_STORAGE_WATCHER)
+        return self._ready and not (threadMgr.get(THREAD_STORAGE_WATCHER) or threadMgr.get(THREAD_DASDFMT))

@property
     def mandatory(self):
@@ -231,6 +240,15 @@ class StorageSpoke(NormalTUISpoke):

if key == "c":
             if self.selected_disks:
+                # check selected disks to see if we have any unformatted DASDs
+                # if we're on s390x, since they need to be formatted before we
+                # can use them.
+                if arch.isS390():
+                    to_format = make_unformatted_dasd_list(self.selected_disks)
+                    if to_format:
+                        self.run_dasdfmt(to_format)
+                        return None
+
                 newspoke = AutoPartSpoke(self.app, self.data, self.storage,
                                          self.payload, self.instclass)
                 self.app.switch_screen_modal(newspoke)
@@ -247,6 +265,32 @@ class StorageSpoke(NormalTUISpoke):
         except (ValueError, KeyError, IndexError):
             return key

+    def run_dasdfmt(self, to_format):
+        """
+        This generates the list of DASDs requiring dasdfmt and runs dasdfmt
+        against them.
+        """
+        # if the storage thread is running, wait on it to complete before taking
+        # any further actions on devices; most likely to occur if user has
+        # zerombr in their ks file
+        threadMgr.wait(THREAD_STORAGE)
+
+        for disk in to_format:
+            try:
+                print(_("Formatting /dev/%s. This may take a moment." % disk))
+                format_dasd(disk)
+            except DasdFormatError as err:
+                # Log errors if formatting fails, but don't halt the installer
+                log.error(str(err))
+                continue
+
+        # when finished formatting we need to reinitialize storage
+        protectedNames = [d.name for d in self.storage.protectedDevices]
+        threadMgr.add(AnacondaThread(name=THREAD_STORAGE,
+                        target=storageInitialize,
+                        args=(self.storage, self.data, protectedNames)))
+        threadMgr.wait(THREAD_STORAGE)
+
     def apply(self):
         self.autopart = self.data.autopart.autopart
         self.data.ignoredisk.onlyuse = self.selected_disks[:]

[anaconda][master/rhel7-branch][PATCH 3/3] Add GUI and TUI logic to handle unformatted DASDs. (#1064423)

[Vratislav Podzimek]

On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> In both graphical and text, if a user selects unformatted DASDs
> from the local disk store, they are formatted when the user opts
> to proceed (either by clicking 'Done' in the GUI or 'c' to
> continue in the TUI).
>
> Following dasdfmt, storage initialization is run again in order
> to properly add the new DASDs to the devicetree and seen by the
> installer.
> Resolves:rhbz#1064423
>
> pyanaconda/constants.py             |  1 +
>  pyanaconda/ui/gui/spokes/storage.py | 81 ++++++++++++++++++++++++++++++++++++-
>  pyanaconda/ui/tui/spokes/storage.py | 50 +++++++++++++++++++++--
>  3 files changed, 127 insertions(+), 5 deletions(-)
>
> diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
> index cf8a8ea..8de29a9 100644
> --- a/pyanaconda/constants.py
> +++ b/pyanaconda/constants.py
> @@ -131,6 +131,7 @@ THREAD_ISCSI_LOGIN = "AnaIscsiLoginThread"
>  THREAD_GEOLOCATION_REFRESH = "AnaGeolocationRefreshThread"
>  THREAD_DATE_TIME = "AnaDateTimeThread"
>  THREAD_TIME_INIT = "AnaTimeInitThread"
> +THREAD_DASDFMT = "AnaDasdfmtThread"
>
> # Geolocation constants
>
> diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
> index bdd48ef..e565b16 100644
> --- a/pyanaconda/ui/gui/spokes/storage.py
> +++ b/pyanaconda/ui/gui/spokes/storage.py
> @@ -50,17 +50,20 @@ from pyanaconda.ui.gui.spokes.lib.cart import SelectedDisksDialog
>  from pyanaconda.ui.gui.spokes.lib.passphrase import PassphraseDialog
>  from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog
>  from pyanaconda.ui.gui.spokes.lib.resize import ResizeDialog
> +from pyanaconda.ui.gui.spokes.lib.dasdfmt import DasdFormatDialog
>  from pyanaconda.ui.gui.categories.system import SystemCategory
> -from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_action_wait
> +from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_action_wait, ignoreEscape
>
> from pyanaconda.kickstart import doKickstartStorage, getAvailableDiskSpace
> +from blivet import storageInitialize, arch
>  from blivet.size import Size
>  from blivet.devices import MultipathDevice
> -from blivet.errors import StorageError
> +from blivet.errors import StorageError, DasdFormatError
>  from blivet.errors import SanityError
>  from blivet.errors import SanityWarning
>  from blivet.platform import platform
>  from blivet.devicelibs import swap as swap_lib
> +from blivet.devicelibs.dasd import make_unformatted_dasd_list, format_dasd
>  from pyanaconda.threads import threadMgr, AnacondaThread
>  from pyanaconda.product import productName
>  from pyanaconda.flags import flags
> @@ -292,6 +295,11 @@ class StorageSpoke(NormalSpoke, StorageChecker):
>          self.autoPartType = None
>          self.clearPartType = CLEARPART_TYPE_NONE
>    if self.data.zerombr.zerombr and arch.isS390():
>        # run dasdfmt on any unformatted DASDs automatically
>        threadMgr.add(AnacondaThread(name=constants.THREAD_DASDFMT,
>                        target=self.run_dasdfmt))
>    self._previous_autopart = False
>
>    self._last_clicked_overview = None
>
> @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):
>      def _doExecute(self):
>          self._ready = False
>          hubQ.send_not_ready(self.__class__.__name__)
>    # ugly and stupid, but on the off-chance dasdfmt is running, we can't
>    # proceed further with this
>    while threadMgr.get(constants.THREAD_DASDFMT):
>        continue

Is there any reason for doing this busy wait instead of calling
threadMgr.wait(constants.THREAD_DASDFMT)? I believe the result would be
the same.

[anaconda][master/rhel7-branch][PATCH 3/3] Add GUI and TUI logic to handle unformatted DASDs. (#1064423)

[Vratislav Podzimek]

On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> In both graphical and text, if a user selects unformatted DASDs
> from the local disk store, they are formatted when the user opts
> to proceed (either by clicking 'Done' in the GUI or 'c' to
> continue in the TUI).
>
> Following dasdfmt, storage initialization is run again in order
> to properly add the new DASDs to the devicetree and seen by the
> installer.
> Resolves:rhbz#1064423
>
> pyanaconda/constants.py             |  1 +
>  pyanaconda/ui/gui/spokes/storage.py | 81 ++++++++++++++++++++++++++++++++++++-
>  pyanaconda/ui/tui/spokes/storage.py | 50 +++++++++++++++++++++--
>  3 files changed, 127 insertions(+), 5 deletions(-)
>
> diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
> index cf8a8ea..8de29a9 100644
> --- a/pyanaconda/constants.py
> +++ b/pyanaconda/constants.py
> @@ -131,6 +131,7 @@ THREAD_ISCSI_LOGIN = "AnaIscsiLoginThread"
>  THREAD_GEOLOCATION_REFRESH = "AnaGeolocationRefreshThread"
>  THREAD_DATE_TIME = "AnaDateTimeThread"
>  THREAD_TIME_INIT = "AnaTimeInitThread"
> +THREAD_DASDFMT = "AnaDasdfmtThread"
>
> # Geolocation constants
>
> diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
> index bdd48ef..e565b16 100644
> --- a/pyanaconda/ui/gui/spokes/storage.py
> +++ b/pyanaconda/ui/gui/spokes/storage.py
> @@ -50,17 +50,20 @@ from pyanaconda.ui.gui.spokes.lib.cart import SelectedDisksDialog
>  from pyanaconda.ui.gui.spokes.lib.passphrase import PassphraseDialog
>  from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog
>  from pyanaconda.ui.gui.spokes.lib.resize import ResizeDialog
> +from pyanaconda.ui.gui.spokes.lib.dasdfmt import DasdFormatDialog
>  from pyanaconda.ui.gui.categories.system import SystemCategory
> -from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_action_wait
> +from pyanaconda.ui.gui.utils import enlightbox, gtk_call_once, gtk_action_wait, ignoreEscape
>
> from pyanaconda.kickstart import doKickstartStorage, getAvailableDiskSpace
> +from blivet import storageInitialize, arch
>  from blivet.size import Size
>  from blivet.devices import MultipathDevice
> -from blivet.errors import StorageError
> +from blivet.errors import StorageError, DasdFormatError
>  from blivet.errors import SanityError
>  from blivet.errors import SanityWarning
>  from blivet.platform import platform
>  from blivet.devicelibs import swap as swap_lib
> +from blivet.devicelibs.dasd import make_unformatted_dasd_list, format_dasd
>  from pyanaconda.threads import threadMgr, AnacondaThread
>  from pyanaconda.product import productName
>  from pyanaconda.flags import flags
> @@ -292,6 +295,11 @@ class StorageSpoke(NormalSpoke, StorageChecker):
>          self.autoPartType = None
>          self.clearPartType = CLEARPART_TYPE_NONE
>    if self.data.zerombr.zerombr and arch.isS390():
>        # run dasdfmt on any unformatted DASDs automatically
>        threadMgr.add(AnacondaThread(name=constants.THREAD_DASDFMT,
>                        target=self.run_dasdfmt))
>    self._previous_autopart = False
>
>    self._last_clicked_overview = None
>
> @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):
>      def _doExecute(self):
>          self._ready = False
>          hubQ.send_not_ready(self.__class__.__name__)
>    # ugly and stupid, but on the off-chance dasdfmt is running, we can't
>    # proceed further with this
>    while threadMgr.get(constants.THREAD_DASDFMT):
>        continue
>    hubQ.send_message(self.__class__.__name__, _("Saving storage configuration..."))
>    try:
>        doKickstartStorage(self.storage, self.data, self.instclass)
>
> @@ -409,6 +421,8 @@ class StorageSpoke(NormalSpoke, StorageChecker):
>      if not self._confirmed:
>          msg = _("Not configured")
>    elif threadMgr.get(constants.THREAD_DASDFMT):
>        msg = _("Formatting DASDs")
>    elif flags.automatedInstall and not self.storage.rootDevice:
>        return msg
>    elif self.data.ignoredisk.onlyuse:
>
> @@ -681,6 +695,41 @@ class StorageSpoke(NormalSpoke, StorageChecker):
>              if not selected and name in self.selected_disks:
>                  self.selected_disks.remove(name)
> def run_dasdfmt(self):
>    """
>    Though the same function exists in pyanaconda.ui.gui.spokes.lib.dasdfmt,
>    this instance doesn't include any of the UI pieces and should only
>    really be getting called on ks installations with "zerombr".
>    """
>    # wait for the initial storage thread to complete before taking any new
>    # actions on storage devices
>    threadMgr.wait(constants.THREAD_STORAGE)
>    to_format = make_unformatted_dasd_list(self.selected_disks)
>    if not to_format:
>        # nothing to do here; bail
>        return
>    hubQ.send_message(self.__class__.__name__, _("Formatting DASDs"))
>    for disk in to_format:
>        try:
>            format_dasd(disk)
>        except DasdFormatError as err:
>            # Log errors if formatting fails, but don't halt the installer
>            log.error(str(err))
>            continue
>    # now re-initialize storage to pick up the newly formatted disks
>    protectedNames = [d.name for d in self.storage.protectedDevices]
>    threadMgr.add(AnacondaThread(name=constants.THREAD_STORAGE,
>                    target=storageInitialize,
>                    args=(self.storage, self.data, protectedNames)))
>    threadMgr.wait(constants.THREAD_STORAGE)

One more thing, sorry. I believe that adding a thread and waiting for it
to finish right after that is the same as calling the function directly.

[Vratislav Podzimek]

On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> This now includes fixes from the second review. I had some residual
> issues with thread handling and updating of the GUI spoke status as
> well (that I didn't fully realize until after I posted my last set), and
> those issues are fixed.
>
> Just so things are clear, I've created four delightful and exciting
> screencasts for anyone interested, so get your popcorn ready:
>
> [graphical install]
> http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm
>
> [graphical install w/zerombr in ks]
> http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm
>
> [text install]
> http://sbueno.fedorapeople.org/1001070-text-20140225.webm
>
> [text install w/zerombr in ks]
> http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm
> From my perspective, these patches are good to go after doing few local

changes fixing the neatpicks. Thanks for working on this so hard!

[Samantha N. Bueno]

On Wed, Feb 26, 2014 at 10:31:24AM +0100, Vratislav Podzimek wrote:
> On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> > This now includes fixes from the second review. I had some residual
> > issues with thread handling and updating of the GUI spoke status as
> > well (that I didn't fully realize until after I posted my last set), and
> > those issues are fixed.
> >
> > Just so things are clear, I've created four delightful and exciting
> > screencasts for anyone interested, so get your popcorn ready:
> >
> > [graphical install]
> > http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm
> >
> > [graphical install w/zerombr in ks]
> > http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm
> >
> > [text install]
> > http://sbueno.fedorapeople.org/1001070-text-20140225.webm
> >
> > [text install w/zerombr in ks]
> > http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm
>
> From my perspective, these patches are good to go after doing few local
> changes fixing the neatpicks. Thanks for working on this so hard!

Great! Didn't want to respond to each message individually since they
were so small, but I've applied all your suggestions. Thanks very much
for the reviews. Re: this one piece though--
> > @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):             
> >      def _doExecute(self):                                                        
> >          self._ready = False                                                      
> >          hubQ.send_not_ready(self.__class__.__name__)
> >    # ugly and stupid, but on the off-chance dasdfmt is running,
> >
> > we can't
> >    # proceed further with this                                              
> >    while threadMgr.get(constants.THREAD_DASDFMT):                           
> >        continue                                                             
>
> Is there any reason for doing this busy wait instead of calling                     
> threadMgr.wait(constants.THREAD_DASDFMT)? I believe the result would be             
> the same.

The only reason I do that is to avoid directly calling THREAD_DASDFMT to
try and reduce the UI freezing. I guess it is kind of the same though
since it is constantly checking the thread. I can't seem to come up
with a better way though. That is my only residual issue with this patch
set though.

Regardless, as there's some discussion over these patches in bz#1064423,
I'm not going to push these yet until that's resolved.

I know some folks from IBM watch this list, but I'm Cc'ing Peter
Oberparleiter just in case since I'd really like confirmation-- Is
there any potential risk of data loss for running dasdfmt on a cpfmtxa
formatted DASD? My understanding has always been that if a DASD has been
formatted with cpfmtxa, there is nothing on it, and thus no risk of data
loss. So I don't see that it should be necessary to warn a user before
running dasdfmt.
Obviously anyone else who watches this list and is knowledgeable in the
s390x realm, feel free to give input as well if you know. :)

Samantha

[Vratislav Podzimek]

On Thu, 2014-02-27 at 09:17 -0500, Samantha N. Bueno wrote:
> On Wed, Feb 26, 2014 at 10:31:24AM +0100, Vratislav Podzimek wrote:
> > On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> > > This now includes fixes from the second review. I had some residual
> > > issues with thread handling and updating of the GUI spoke status as
> > > well (that I didn't fully realize until after I posted my last set), and
> > > those issues are fixed.
> > >
> > > Just so things are clear, I've created four delightful and exciting
> > > screencasts for anyone interested, so get your popcorn ready:
> > >
> > > [graphical install]
> > > http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm
> > >
> > > [graphical install w/zerombr in ks]
> > > http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm
> > >
> > > [text install]
> > > http://sbueno.fedorapeople.org/1001070-text-20140225.webm
> > >
> > > [text install w/zerombr in ks]
> > > http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm
> >
> > From my perspective, these patches are good to go after doing few local
> > changes fixing the neatpicks. Thanks for working on this so hard!
>
> Great! Didn't want to respond to each message individually since they
> were so small, but I've applied all your suggestions. Thanks very much
> for the reviews. Re: this one piece though--
> > > @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):             
> > >      def _doExecute(self):                                                        
> > >          self._ready = False                                                      
> > >          hubQ.send_not_ready(self.__class__.__name__)
> > >    # ugly and stupid, but on the off-chance dasdfmt is running,
> > >
> > > we can't
> > >    # proceed further with this                                              
> > >    while threadMgr.get(constants.THREAD_DASDFMT):                           
> > >        continue                                                             
> >
> > Is there any reason for doing this busy wait instead of calling                     
> > threadMgr.wait(constants.THREAD_DASDFMT)? I believe the result would be             
> > the same.
>
> The only reason I do that is to avoid directly calling THREAD_DASDFMT to
> try and reduce the UI freezing. I guess it is kind of the same though
> since it is constantly checking the thread. I can't seem to come up
> with a better way though. That is my only residual issue with this patch
> set though.

The _doExecute method is called in a non-main thread, so it shouldn't
freeze the UI.

If you send me the final version of your patches, I'll apply them and
try to find out what may be the cause of the hangs you are seeing.

[Peter Oberparleiter]

On 27.02.2014 15:17, Samantha N. Bueno wrote:
> On Wed, Feb 26, 2014 at 10:31:24AM +0100, Vratislav Podzimek wrote:
> > On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> > > This now includes fixes from the second review. I had some residual
> > > issues with thread handling and updating of the GUI spoke status as
> > > well (that I didn't fully realize until after I posted my last set), and
> > > those issues are fixed.
> > >
> > > Just so things are clear, I've created four delightful and exciting
> > > screencasts for anyone interested, so get your popcorn ready:
> > >
> > > [graphical install]
> > > http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm
> > >
> > > [graphical install w/zerombr in ks]
> > > http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm
> > >
> > > [text install]
> > > http://sbueno.fedorapeople.org/1001070-text-20140225.webm
> > >
> > > [text install w/zerombr in ks]
> > > http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm
> >
> > From my perspective, these patches are good to go after doing few local
> > changes fixing the neatpicks. Thanks for working on this so hard!
>
> Great! Didn't want to respond to each message individually since they
> were so small, but I've applied all your suggestions. Thanks very much
> for the reviews. Re: this one piece though--
> > > @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):             
> > >      def _doExecute(self):                                                        
> > >          self._ready = False                                                      
> > >          hubQ.send_not_ready(self.__class__.__name__)
> > >    # ugly and stupid, but on the off-chance dasdfmt is running,
> > >
> > > we can't
> > >    # proceed further with this                                              
> > >    while threadMgr.get(constants.THREAD_DASDFMT):                           
> > >        continue                                                             
> >
> > Is there any reason for doing this busy wait instead of calling                     
> > threadMgr.wait(constants.THREAD_DASDFMT)? I believe the result would be             
> > the same.
>
> The only reason I do that is to avoid directly calling THREAD_DASDFMT to
> try and reduce the UI freezing. I guess it is kind of the same though
> since it is constantly checking the thread. I can't seem to come up
> with a better way though. That is my only residual issue with this patch
> set though.
>
> Regardless, as there's some discussion over these patches in bz#1064423,
> I'm not going to push these yet until that's resolved.
>
> I know some folks from IBM watch this list, but I'm Cc'ing Peter
> Oberparleiter just in case since I'd really like confirmation-- Is
> there any potential risk of data loss for running dasdfmt on a cpfmtxa
> formatted DASD? My understanding has always been that if a DASD has been
> formatted with cpfmtxa, there is nothing on it, and thus no risk of data
> loss. So I don't see that it should be necessary to warn a user before
> running dasdfmt.

I'm not sure I have enough context information to fully answer this
question. As you stated, z/VM's CPFMTXA will format the DASD, usually
clearing all data in the process, just like dasdfmt will do. I don't
know where data loss beyond the clearing of the DASD would come in
here...
> Obviously anyone else who watches this list and is knowledgeable in the
> s390x realm, feel free to give input as well if you know. :)

[Peter Oberparleiter]

On 28.02.2014 10:53, Peter Oberparleiter wrote:
> On 27.02.2014 15:17, Samantha N. Bueno wrote:
> > On Wed, Feb 26, 2014 at 10:31:24AM +0100, Vratislav Podzimek wrote:
> > > On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> > > > This now includes fixes from the second review. I had some residual
> > > > issues with thread handling and updating of the GUI spoke status as
> > > > well (that I didn't fully realize until after I posted my last set), and
> > > > those issues are fixed.
> > > >
> > > > Just so things are clear, I've created four delightful and exciting
> > > > screencasts for anyone interested, so get your popcorn ready:
> > > >
> > > > [graphical install]
> > > > http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm
> > > >
> > > > [graphical install w/zerombr in ks]
> > > > http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm
> > > >
> > > > [text install]
> > > > http://sbueno.fedorapeople.org/1001070-text-20140225.webm
> > > >
> > > > [text install w/zerombr in ks]
> > > > http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm
> > >
> > > From my perspective, these patches are good to go after doing few local
> > > changes fixing the neatpicks. Thanks for working on this so hard!
> >
> > Great! Didn't want to respond to each message individually since they
> > were so small, but I've applied all your suggestions. Thanks very much
> > for the reviews. Re: this one piece though--
> > > > @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):             
> > > >      def _doExecute(self):                                                        
> > > >          self._ready = False                                                      
> > > >          hubQ.send_not_ready(self.__class__.__name__)
> > > >    # ugly and stupid, but on the off-chance dasdfmt is running,
> > > >
> > > > we can't
> > > >    # proceed further with this                                              
> > > >    while threadMgr.get(constants.THREAD_DASDFMT):                           
> > > >        continue                                                             
> > >
> > > Is there any reason for doing this busy wait instead of calling                     
> > > threadMgr.wait(constants.THREAD_DASDFMT)? I believe the result would be             
> > > the same.
> >
> > The only reason I do that is to avoid directly calling THREAD_DASDFMT to
> > try and reduce the UI freezing. I guess it is kind of the same though
> > since it is constantly checking the thread. I can't seem to come up
> > with a better way though. That is my only residual issue with this patch
> > set though.
> >
> > Regardless, as there's some discussion over these patches in bz#1064423,
> > I'm not going to push these yet until that's resolved.
> >
> > I know some folks from IBM watch this list, but I'm Cc'ing Peter
> > Oberparleiter just in case since I'd really like confirmation-- Is
> > there any potential risk of data loss for running dasdfmt on a cpfmtxa
> > formatted DASD? My understanding has always been that if a DASD has been
> > formatted with cpfmtxa, there is nothing on it, and thus no risk of data
> > loss. So I don't see that it should be necessary to warn a user before
> > running dasdfmt.
>
> I'm not sure I have enough context information to fully answer this
> question. As you stated, z/VM's CPFMTXA will format the DASD, usually
> clearing all data in the process, just like dasdfmt will do. I don't
> know where data loss beyond the clearing of the DASD would come in
> here...
> > Obviously anyone else who watches this list and is knowledgeable in the
> > s390x realm, feel free to give input as well if you know. :)

Here's some more information from our DASD expert Stefan Weinhuber:

cpfmtxa is a z/VM tool that is usually used to prepare a disk for use
with z/VM, e.g. as spool device or to store minidisks. Some people use
it on DASDs they have previously used with other operating systems, to
make sure that there is no old format left. A disk formatted with
cpfmtxa contains a format that is not recognized and supported by Linux,
but it does contain a format, and may contain data.

[Samantha N. Bueno]

On Mon, Mar 03, 2014 at 11:09:31AM +0100, Peter Oberparleiter wrote:
> On 28.02.2014 10:53, Peter Oberparleiter wrote:
> > On 27.02.2014 15:17, Samantha N. Bueno wrote:
> > > On Wed, Feb 26, 2014 at 10:31:24AM +0100, Vratislav Podzimek wrote:
> > > > On Wed, 2014-02-26 at 00:07 -0500, Samantha N. Bueno wrote:
> > > > > This now includes fixes from the second review. I had some residual
> > > > > issues with thread handling and updating of the GUI spoke status as
> > > > > well (that I didn't fully realize until after I posted my last set), and
> > > > > those issues are fixed.
> > > > >
> > > > > Just so things are clear, I've created four delightful and exciting
> > > > > screencasts for anyone interested, so get your popcorn ready:
> > > > >
> > > > > [graphical install]
> > > > > http://sbueno.fedorapeople.org/1001070-graphical-20140225.webm
> > > > >
> > > > > [graphical install w/zerombr in ks]
> > > > > http://sbueno.fedorapeople.org/1001070-graphical-zerombr-20140225.webm
> > > > >
> > > > > [text install]
> > > > > http://sbueno.fedorapeople.org/1001070-text-20140225.webm
> > > > >
> > > > > [text install w/zerombr in ks]
> > > > > http://sbueno.fedorapeople.org/1001070-text-zerombr-20140225.webm
> > > >
> > > > From my perspective, these patches are good to go after doing few local
> > > > changes fixing the neatpicks. Thanks for working on this so hard!
> > >
> > > Great! Didn't want to respond to each message individually since they
> > > were so small, but I've applied all your suggestions. Thanks very much
> > > for the reviews. Re: this one piece though--
> > > > > @@ -357,6 +365,10 @@ class StorageSpoke(NormalSpoke, StorageChecker):             
> > > > >      def _doExecute(self):                                                        
> > > > >          self._ready = False                                                      
> > > > >          hubQ.send_not_ready(self.__class__.__name__)
> > > > >    # ugly and stupid, but on the off-chance dasdfmt is running,
> > > > >
> > > > > we can't
> > > > >    # proceed further with this                                              
> > > > >    while threadMgr.get(constants.THREAD_DASDFMT):                           
> > > > >        continue                                                             
> > > >
> > > > Is there any reason for doing this busy wait instead of calling                     
> > > > threadMgr.wait(constants.THREAD_DASDFMT)? I believe the result would be             
> > > > the same.
> > >
> > > The only reason I do that is to avoid directly calling THREAD_DASDFMT to
> > > try and reduce the UI freezing. I guess it is kind of the same though
> > > since it is constantly checking the thread. I can't seem to come up
> > > with a better way though. That is my only residual issue with this patch
> > > set though.
> > >
> > > Regardless, as there's some discussion over these patches in bz#1064423,
> > > I'm not going to push these yet until that's resolved.
> > >
> > > I know some folks from IBM watch this list, but I'm Cc'ing Peter
> > > Oberparleiter just in case since I'd really like confirmation-- Is
> > > there any potential risk of data loss for running dasdfmt on a cpfmtxa
> > > formatted DASD? My understanding has always been that if a DASD has been
> > > formatted with cpfmtxa, there is nothing on it, and thus no risk of data
> > > loss. So I don't see that it should be necessary to warn a user before
> > > running dasdfmt.
> >
> > I'm not sure I have enough context information to fully answer this
> > question. As you stated, z/VM's CPFMTXA will format the DASD, usually
> > clearing all data in the process, just like dasdfmt will do. I don't
> > know where data loss beyond the clearing of the DASD would come in
> > here...
> > > Obviously anyone else who watches this list and is knowledgeable in the
> > > s390x realm, feel free to give input as well if you know. :)
>
> Here's some more information from our DASD expert Stefan Weinhuber:
>
> cpfmtxa is a z/VM tool that is usually used to prepare a disk for use
> with z/VM, e.g. as spool device or to store minidisks. Some people use
> it on DASDs they have previously used with other operating systems, to
> make sure that there is no old format left. A disk formatted with
> cpfmtxa contains a format that is not recognized and supported by Linux,
> but it does contain a format, and may contain data.

Thanks, that's actually very helpful. Due to the 'may contain data'
possibility, I'll add the 'format' button back in instead of just
auto-formatting selected DASDs then.

Thanks again to both of you.

Samantha