Re: [PATCH 2/3] Poll DASD status for 'online' or 'unformatted' (#536803)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Wed, 13 Jan 2010, Steffen Maier wrote:

On 01/12/2010 06:23 PM, David Cantrell wrote:
Give DASD devices some time to enter the online or unformatted state
before calling udevadm settle.  Max duration of loop is suggestion from
IBM.
---
 loader/linuxrc.s390 |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
index a3ee665..7b09ea4 100644
--- a/loader/linuxrc.s390
+++ b/loader/linuxrc.s390
@@ -129,6 +129,25 @@ function sysecho () {
     [ -f "$file" ] && echo $* > $file
 }

+function dasd_settle() {
+    dasd=/sys/bus/ccw/devices/$1

It would be nice, if you could make this variable also local to the
function just to be on the safe side.

Done.

+    if [ ! -d "$dasd" ]; then
+        return 0

IIRC, shell functions use 0 for true and !=0 (e.g. 1) for false just
like errorlevels returned by external commands called. This is just the
opposite of C style booleans. If the sysfs path does not exist, the
function should return false, e.g. 1:

return 1

Bah!  Done.

+    fi
+    local i=1
+    while [ $i -le 30 ] ; do
+        status="$(< /sys/bus/ccw/devices/$dasd/status 2>/dev/null)"
+        case $status in
+            online|unformatted)
+                return 1 ;;

This is the one good exit path, where we should return true.

return 0 ;;

And this one.

+            *)
+                sleep 0.1
+                i=$((i+1)) ;;
+        esac
+    done
+    return 0

Return false, since the DASD did not come available as expected within
the timeout.

return 1

And this one.

+}
+
 function startinetd()
 {
     echo
@@ -189,6 +208,10 @@ function readcmsfile() # $1=dasdport $2=filename
         echo $"DASD $dev could not be set online"
         return 1
     fi

Just to be on the safe side, I'd like to put a udev settle here as well.

udevadm settle

Done.

+    if ! dasd_settle $dev ; then
+        echo $"Could not access DASD $dev in time"
+        return 1
+    fi
     udevadm settle
     if ! cmsfscat -d /dev/dasda -a $2 > /tmp/$2; then
         echo $"Could not read conf file $2 on CMS DASD $1."
@@ -197,6 +220,10 @@ function readcmsfile() # $1=dasdport $2=filename
         echo $"DASD $dev could not be set offline again"
         return 1
     fi
+    if ! dasd_settle $dev ; then
+        echo $"Could not access DASD $dev in time"
+        return 1
+    fi

Sorry, I was unclear in my bug comment, when I suggested to use
dasd_settle "after each online action for a DASD".
I suppose this won't work on writing 0 to the online attribute, i.e.
setting it offline. We only need dasd_settle after setting a DASD
online. On setting offline, the DASD would most probably never reach a
status of online or unformatted and dasd_settle would always fail.
Usually the DASD where the CMS conf file is located will not be used
later on during installation and therefore, we don't really care what
happens after setting it offline or how long this would take. Therefore,
please remove this one dasd_settle here.

Ooops.  I thought I had only added the dasd_settle calls for setting a DASD
online.  Copy and paste got the better of me here.

     udevadm settle
     # consequences of no more module unload: loader can no longer
     # use DASD module option to online DASDs and set other DASD parameters!

- -- David Cantrell <dcantrell@xxxxxxxxxx>
Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.14 (GNU/Linux)

iEYEARECAAYFAktOH7IACgkQ5hsjjIy1VkkGxACeKkboo5yDQqBBf3OoEenbARYQ
Q5wAoIo+PuIWnVYz8pWFmIDbuLFjdFLz
=zaNM
-----END PGP SIGNATURE-----

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux