On Sat, 26 Jun 2010, Steffen Maier wrote:
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.
Makes sense. I've updated the patch to write out an /etc/dasd.conf file.
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.
Whoops. Moved.
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 >> $dracutoptsSee 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" >> $dracutoptsSince we check the syntax of DASD= before applying stuff, we might consider writing attributes out unconditionally.
Yeah, that makes things more simple. Changed.
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.
Another whoops. Also moved.
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.
Remove the self.dracutopts property. Still need to modify dracutSetupString(), see below for details.
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.
Yeah, simple.
(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.)
Which you have to admit is a bit stupid and ridiculous, but I will play along. We will continue mashing formats back and forth until everything works.
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.
We still need to have dracutSetupString() read in the contents of /etc/dasd.conf and then use its contents if the busid matches, otherwise go with the above code. dracutSetupString() is run per device, so we get an rd_DASD parameter for each busid that is used for the root filesystem. This may or may not be all of the DASD devices configured at install time, which is why we can't simply modify booty to munge /etc/dasd.conf to the rd_DASD parameters for zipl.conf. If we're saying the same thing, that's fine. But it's been a while since I've responded and I don't exactly remember the thought process I was in. I'll be sending a revised patch later today. Thanks for the comments on this one. -- David Cantrell <dcantrell@xxxxxxxxxx> Red Hat / Honolulu, HI _______________________________________________ Anaconda-devel-list mailing list Anaconda-devel-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/anaconda-devel-list
