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

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

 



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.

> +    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

> +    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 ;;

> +            *)
> +                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

> +}
> +
>  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

> +    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.

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

Steffen

Linux on System z Development

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


_______________________________________________
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