FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu User

 
 
LinkBack Thread Tools
 
Old 06-25-2010, 12:00 AM
David Cantrell
 
Default Do not add DASD default options to rd_DASD option (#606783)

Only include option values in the rd_DASD parameter if the user has
changed the value. This should reduce the amount of data written to
zipl.conf by a reasonable amount, which should keep most use cases from
exceeding the boot parameter length limit.
---
storage/devices.py | 5 ++++-
storage/devicetree.py | 2 ++
2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/storage/devices.py b/storage/devices.py
index 0f6e892..fac0452 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -3555,6 +3555,7 @@ class DASDDevice(DiskDevice):
def __init__(self, device, **kwargs):
self.busid = kwargs.pop('busid')
self.opts = kwargs.pop('opts')
+ self.origopts = kwargs.pop('origopts')
self.dasd = kwargs.pop('dasd')
DiskDevice.__init__(self, device, **kwargs)

@@ -3562,7 +3563,9 @@ class DASDDevice(DiskDevice):
self.dasd.addDASD(self)

def getOpts(self):
- return map(lambda (k, v): "%s=%s" % (k, v,), self.opts.items())
+ return map(lambda (k, v): "%s=%s" % (k, v,),
+ filter(lambda (k, v): v != self.origopts[k],
+ self.opts.items())

def dracutSetupString(self):
args = ["rd_DASD=%s" % (self.busid,)] + self.getOpts()
diff --git a/storage/devicetree.py b/storage/devicetree.py
index 6a167e9..7228768 100644
--- a/storage/devicetree.py
+++ b/storage/devicetree.py
@@ -1191,9 +1191,11 @@ class DeviceTree(object):
kwargs["dasd"] = self.dasd
kwargs["busid"] = udev_device_get_dasd_bus_id(info)
kwargs["opts"] = {}
+ kwargs["origopts"] = {}

for attr in ['readonly', 'use_diag', 'erplog', 'failfast']:
kwargs["opts"][attr] = udev_device_get_dasd_flag(info, attr)
+ kwargs["origopts"][attr] = udev_device_get_dasd_flag(info, attr)

log.debug("%s is a dasd device" % name)
elif udev_device_is_zfcp(info):
--
1.7.0.1

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 06-25-2010, 11:50 AM
Steffen Maier
 
Default Do not add DASD default options to rd_DASD option (#606783)

I'm pretty sure this won't work.

Linuxrc.s390 sets those DASD sysfs attributes (previously with RHEL5 it
was loader by means of the dasd_mod driver module parameter dasd=) and
to my knowledge there is no other code in anaconda that ever modifies
those values. Hence the code below compares the value that has already
been set with the exact same value which has not been modified meanwhile.

Only linuxrc.s390 (or whoever is the first to write to those sysfs
attributes) is able to read the driver default values before overwriting
them. That's why the correct solution for the current design would have
to remember either the default value or--even better--just the
overridden attributes for each DASD in linuxrc.s390 and pass it on to
anaconda probably by means of a file (as with /tmp/s390net).

See also our previous discussion on default values for DASD sysfs
attributes:
https://www.redhat.com/archives/anaconda-devel-list/2009-October/msg00274.html

On 06/25/2010 02:00 AM, David Cantrell wrote:
> Only include option values in the rd_DASD parameter if the user has
> changed the value. This should reduce the amount of data written to
> zipl.conf by a reasonable amount, which should keep most use cases from
> exceeding the boot parameter length limit.
> ---
> storage/devices.py | 5 ++++-
> storage/devicetree.py | 2 ++
> 2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/storage/devices.py b/storage/devices.py
> index 0f6e892..fac0452 100644
> --- a/storage/devices.py
> +++ b/storage/devices.py
> @@ -3555,6 +3555,7 @@ class DASDDevice(DiskDevice):
> def __init__(self, device, **kwargs):
> self.busid = kwargs.pop('busid')
> self.opts = kwargs.pop('opts')
> + self.origopts = kwargs.pop('origopts')
> self.dasd = kwargs.pop('dasd')
> DiskDevice.__init__(self, device, **kwargs)
>
> @@ -3562,7 +3563,9 @@ class DASDDevice(DiskDevice):
> self.dasd.addDASD(self)
>
> def getOpts(self):
> - return map(lambda (k, v): "%s=%s" % (k, v,), self.opts.items())
> + return map(lambda (k, v): "%s=%s" % (k, v,),
> + filter(lambda (k, v): v != self.origopts[k],
> + self.opts.items())
>
> def dracutSetupString(self):
> args = ["rd_DASD=%s" % (self.busid,)] + self.getOpts()
> diff --git a/storage/devicetree.py b/storage/devicetree.py
> index 6a167e9..7228768 100644
> --- a/storage/devicetree.py
> +++ b/storage/devicetree.py
> @@ -1191,9 +1191,11 @@ class DeviceTree(object):
> kwargs["dasd"] = self.dasd
> kwargs["busid"] = udev_device_get_dasd_bus_id(info)
> kwargs["opts"] = {}
> + kwargs["origopts"] = {}
>
> for attr in ['readonly', 'use_diag', 'erplog', 'failfast']:
> kwargs["opts"][attr] = udev_device_get_dasd_flag(info, attr)
> + kwargs["origopts"][attr] = udev_device_get_dasd_flag(info, attr)
>
> log.debug("%s is a dasd device" % name)
> elif udev_device_is_zfcp(info):

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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 06-25-2010, 07:13 PM
David Cantrell
 
Default Do not add DASD default options to rd_DASD option (#606783)

On Fri, 25 Jun 2010, Steffen Maier wrote:


I'm pretty sure this won't work.

Linuxrc.s390 sets those DASD sysfs attributes (previously with RHEL5 it
was loader by means of the dasd_mod driver module parameter dasd=) and
to my knowledge there is no other code in anaconda that ever modifies
those values. Hence the code below compares the value that has already
been set with the exact same value which has not been modified meanwhile.

Only linuxrc.s390 (or whoever is the first to write to those sysfs
attributes) is able to read the driver default values before overwriting
them. That's why the correct solution for the current design would have
to remember either the default value or--even better--just the
overridden attributes for each DASD in linuxrc.s390 and pass it on to
anaconda probably by means of a file (as with /tmp/s390net).


Yes, this is better given how things work now in the installer. Here is an
updated patch to collect the values during the linuxrc.s390 run and then pull
those in during the dracutSetupString() run.

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"
for attr in $attrs; do
+ echo -n "$devbusid" >> $dracutopts
if [ "$attr" = "use_diag" ]; then
# diag discipline cannot be auto-loaded
modprobe dasd_diag_mod
+ echo -n ",use_diag=1" >> $dracutopts
fi
if [ ! -f $sys/$attr ]; then
echo $"DASD $devbusid does not provide attribute $attr"
+ echo >> $dracutopts
continue
fi
if ! sysecho $sys/$attr 1; then
echo $"Could not set attribute $attr for DASD $devbusid"
+ else
+ echo -n ",$attr=1" >> $dracutopts
fi
+ echo >> $dracutopts
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())

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
+
args = ["rd_DASD=%s" % (self.busid,)] + self.getOpts()
return ",".join(args)

--
David Cantrell <dcantrell@redhat.com>
Red Hat / Honolulu, HI

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 06-26-2010, 02:28 PM
Steffen Maier
 
Default Do not add DASD default options to rd_DASD option (#606783)

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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 07-20-2010, 06:51 PM
David Cantrell
 
Default Do not add DASD default options to rd_DASD option (#606783)

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


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@redhat.com>
Red Hat / Honolulu, HI

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

Thread Tools




All times are GMT. The time now is 09:44 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright ©2007 - 2008, www.linux-archive.org