Three little things might need some fixing.
I added some general throughts on consistency which are optional from a
functional point of view.
On 06/25/2010 09:13 PM, David Cantrell wrote:
> On Fri, 25 Jun 2010, Steffen Maier wrote:
> The approach here is very simple. Just build rd_DASD= values during the
> linuxrc.s390 run and store them in /tmp/s390dracut. Then pull them in
> during
> the dracutSetupString() run if the busid matches, falling back on what we
> currently do if there isn't a line in s390dracut for the device we are
> currently looking at.
>
> ---
> loader/linuxrc.s390 | 7 +++++++
> storage/devices.py | 10 ++++++++++
> 2 files changed, 17 insertions(+), 0 deletions(-)
>
> diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
> index 26eda61..e830295 100644
> --- a/loader/linuxrc.s390
> +++ b/loader/linuxrc.s390
> @@ -2538,18 +2538,25 @@ function parse_dasd() {
> for ((devno=$lodevno; $devno <= $hidevno; ++devno)); do
> local devbusid=$(printf "%s.%04x" ${lo%.*} $devno)
> local sys="/sys/bus/ccw/devices/"$devbusid
> + local dracutopts="/tmp/s390dracut"
We could reuse the file name and file syntax of the already defined
/etc/dasd.conf for more consistency. Meanwhile we also do so with
/etc/zfcp.conf. Note that the latter has the same syntax but potentially
different content than /mnt/sysimage/etc/zfcp.conf written by
storage/zfcp.py:write. We could do the same with dasd.conf. The syntax
of each line is device bus ID followed by attribute value pairs
separated by space, so quite similar to the rd_DASD syntax.
> for attr in $attrs; do
> + echo -n "$devbusid" >> $dracutopts
(1) I guess this should only be written only once for every devno, i.e.
in the outer devno loop instead of here inside the attr loop for each
attr of the same devno.
> if [ "$attr" = "use_diag" ]; then
> # diag discipline cannot be auto-loaded
> modprobe dasd_diag_mod
> + echo -n ",use_diag=1" >> $dracutopts
(2) Not needed here. This is already covered by the 2nd next if
statement applying to all attributes including use_diag.
> fi
> if [ ! -f $sys/$attr ]; then
> echo $"DASD $devbusid does not provide
> attribute $attr"
> + echo >> $dracutopts
See below for an alternative.
> continue
> fi
> if ! sysecho $sys/$attr 1; then
> echo $"Could not set attribute $attr for
> DASD $devbusid"
> + else
> + echo -n ",$attr=1" >> $dracutopts
Since we check the syntax of DASD= before applying stuff, we might
consider writing attributes out unconditionally.
> fi
> + echo >> $dracutopts
(3) I think the newline needs to be outside the inner attribute loop.
There would also be another continue before which a newline should be
written. Maybe we just drop the special handling of continues and write
a newline at the beginning of the outer devno loop body and ignore
potential empty lines written.
> done
> if [ ! -f $sys/online ]; then
> echo $"DASD $devbusid not found"
> diff --git a/storage/devices.py b/storage/devices.py
> index 0f6e892..1e30f28 100644
> --- a/storage/devices.py
> +++ b/storage/devices.py
> @@ -3556,6 +3556,7 @@ class DASDDevice(DiskDevice):
> self.busid = kwargs.pop('busid')
> self.opts = kwargs.pop('opts')
> self.dasd = kwargs.pop('dasd')
> + self.dracutopts = "/tmp/s390dracut"
> DiskDevice.__init__(self, device, **kwargs)
>
> if self.dasd:
> @@ -3565,6 +3566,15 @@ class DASDDevice(DiskDevice):
> return map(lambda (k, v): "%s=%s" % (k, v,), self.opts.items())
Theoretically, we would no longer need to read the sysfs attributes
outselves, but could just read from /etc/dasd.conf to fill in busid and
opts. Then dracutSetupString would not need to be touched, I guess.
>
> def dracutSetupString(self):
> + if os.path.isfile(self.dracutopts):
> + f = open(self.dracutopts)
> + lines = map(lambda x: x.strip(), f.readlines())
> + f.close()
> +
> + for line in lines:
> + if line.startswith(self.busid):
> + return "rd_DASD=%s" % line
> +
If we use dasd.conf syntax, then we simply split each line by whitespace
and join it with commas.
(BTW, dracut does the reverse function internally on parsing rd_DASD
options and writing an internal /etc/dasd.conf which is in turn used by
/sbin/dasdconf.sh triggered by udev add events of DASDs on the ccw bus.)
> args = ["rd_DASD=%s" % (self.busid,)] + self.getOpts()
> return ",".join(args)
Currently, we would never get here since only those DASDs will be active
that linuxrc has activated and noted in the parameter passing file. But
it's OK to leave the code in.
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
