This time with fixes from Chris. Note that the linuxrc fix is just informative at the end of an install and before a reboot occurs...



---

iutil.py | 5 ++++-

loader/linuxrc.s390 | 10 ++++++++++

2 files changed, 14 insertions(+), 1 deletions(-)



diff --git a/iutil.py b/iutil.py

index 0b15bd8..0c43684 100644

--- a/iutil.py

+++ b/iutil.py

@@ -980,7 +980,10 @@ def reIPL(anaconda, loader_pid):



reipl_path = "/sys/firmware/reipl"



- ipldev = anaconda.id.bootloader.device

+ try:

+ ipldev = anaconda.platform.bootDevice().disk.name

+ except:

+ ipldev = None



if ipldev is None:

message = _("Error determining mount point type")

diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390

index d50dcbd..71e4d0e 100644

--- a/loader/linuxrc.s390

+++ b/loader/linuxrc.s390

@@ -95,6 +95,16 @@ function doshutdown()



function doreboot()

{

+ if [ -e "/sys/firmware/reipl" ]; then

+ read REIPL_TYPE < /sys/firmware/reipl/reipl_type

+ echo "reipl_type=$REIPL_TYPE"

+ pushd /sys/firmware/reipl/$REIPL_TYPE >/dev/null 2>&1

+ for i in *; do

+ echo "$i=`cat $i`"

+ done

+ popd >/dev/null 2>&1

+ fi

+

echo $"about to exec shutdown -r"

exec /sbin/shutdown -r

exit 0

--

1.6.4



--

Mark



You must be the change you wish to see in the world. -- Mahatma Gandhi

Worrying is praying for that you do not wish to happen.___________________________________________ ____
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

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

Functionally this looks fine. Cosmetic exceptions:

1) Please follow our format for git commit messages. We give a one line
description and reference the bug number as (#BUGNUMBER) at the end of that
one description. The one line description should not be longer than 80
characters, though 50 to 60 is even nicer.

2) In the commit message body, explain what's going on in the patch. Note
what was fixed and why. During the patch review process, you may have
committed several sets of changes to the original patch. You still want to
publish a single patch as well as expand the log entry (if necessary). An
interactive git rebase helps with that:

http://blog.madism.org/index.php/2007/09/09/138-git-awsome-ness-git-rebase-interactive

With that, you can squash all of your changes during the review in to a single
patch ready for 'git am' on the main repository. This is most useful if you
create a local working branch of master, to keep your work separated from the
main tracking branch.

3) Please use 4 spaces for indentation.

I went ahead and made these changes and pushed the resulting patch since you
said you were going on vacation starting Nov 18th.

Thanks,


On Tue, 17 Nov 2009, Mark Hamzy wrote:



This time with fixes from Chris. Note that the linuxrc fix is just
informative at the end of an install and before a reboot occurs...

---
iutil.py | 5 ++++-
loader/linuxrc.s390 | 10 ++++++++++
2 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/iutil.py b/iutil.py
index 0b15bd8..0c43684 100644
--- a/iutil.py
+++ b/iutil.py
@@ -980,7 +980,10 @@ def reIPL(anaconda, loader_pid):

reipl_path = "/sys/firmware/reipl"

- ipldev = anaconda.id.bootloader.device
+ try:
+ ipldev = anaconda.platform.bootDevice().disk.name
+ except:
+ ipldev = None

if ipldev is None:
message = _("Error determining mount point type")
diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
index d50dcbd..71e4d0e 100644
--- a/loader/linuxrc.s390
+++ b/loader/linuxrc.s390
@@ -95,6 +95,16 @@ function doshutdown()

function doreboot()
{
+ if [ -e "/sys/firmware/reipl" ]; then
+ read REIPL_TYPE < /sys/firmware/reipl/reipl_type
+ echo "reipl_type=$REIPL_TYPE"
+ pushd /sys/firmware/reipl/$REIPL_TYPE >/dev/null 2>&1
+ for i in *; do
+ echo "$i=`cat $i`"
+ done
+ popd >/dev/null 2>&1
+ fi
+
echo $"about to exec shutdown -r"
exec /sbin/shutdown -r
exit 0
--
1.6.4

--
Mark

You must be the change you wish to see in the world. -- Mahatma Gandhi
Worrying is praying for that you do not wish to happen.


- --
David Cantrell <dcantrell@redhat.com>

Red Hat / Honolulu, HI

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAksDQNsACgkQ5hsjjIy1VknSWgCfUvZlKtMOL9 ONAW68UQ02kt1k
gTUAoJW4DTdXeg/UdrU/lvS1sWCIpBMJ
=PaPq
-----END PGP SIGNATURE-----

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

On 11/18/2009 01:33 AM, David Cantrell wrote:
> I went ahead and made these changes and pushed the resulting patch since
> you said you were going on vacation starting Nov 18th.

> On Tue, 17 Nov 2009, Mark Hamzy wrote:
>> This time with fixes from Chris. Note that the linuxrc fix is just
>> informative at the end of an install and before a reboot occurs...

>> diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
>> index d50dcbd..71e4d0e 100644
>> --- a/loader/linuxrc.s390
>> +++ b/loader/linuxrc.s390
>> @@ -95,6 +95,16 @@ function doshutdown()
>>
>> function doreboot()
>> {
>> + if [ -e "/sys/firmware/reipl" ]; then
>> + read REIPL_TYPE < /sys/firmware/reipl/reipl_type
>> + echo "reipl_type=$REIPL_TYPE"
>> + pushd /sys/firmware/reipl/$REIPL_TYPE >/dev/null 2>&1

I see two problems with the informative code. First, anaconda supports
the two reipl types ccw for DASD and fcp for zFCP. The kernel interface
also supports other stuff such as nss (named saved segments). I think we
should only descend into the reipl type specific subdirectory and get
additional information for the two types anaconda supports because
anaconda does not know anything about the other types and how to
interpret their information. Which is fine, BTW, since those types are
not needed for installation.

>> + for i in *; do
>> + echo "$i=`cat $i`"

I'm not sure, if all sysfs attributes will always contain printable
ASCII text. I wouldn't want any potential binary data misconfigure the
console. Maybe a hexdump (e.g. od -xa) of the attributes' content would
make this safer?

>> + done
>> + popd >/dev/null 2>&1
>> + fi
>> +
>> echo $"about to exec shutdown -r"
>> exec /sbin/shutdown -r
>> exit 0

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

Steffen Maier <maier@linux.vnet.ibm.com> wrote on 11/17/2009 09:18:42 PM:



> >> function doreboot()

> >> {

> >> + * *if [ -e "/sys/firmware/reipl" ]; then

> >> + * * read REIPL_TYPE < /sys/firmware/reipl/reipl_type

> >> + * * echo "reipl_type=$REIPL_TYPE"

> >> + * * pushd /sys/firmware/reipl/$REIPL_TYPE >/dev/null 2>&1

>

> I see two problems with the informative code. First, anaconda supports

> the two reipl types ccw for DASD and fcp for zFCP. The kernel interface

> also supports other stuff such as nss (named saved segments). I think we

> should only descend into the reipl type specific subdirectory and get

> additional information for the two types anaconda supports because

> anaconda does not know anything about the other types and how to

> interpret their information. Which is fine, BTW, since those types are

> not needed for installation.



While the kernel interface supports other types, anaconda only sets one

of the two. *Which would make this code safe, right? *It also is simpler

by keeping tests and specialized code out of the picture.



> >> + * * for i in *; do

> >> + * * * * echo "$i=`cat $i`"

>

> I'm not sure, if all sysfs attributes will always contain printable

> ASCII text. I wouldn't want any potential binary data misconfigure the

> console. Maybe a hexdump (e.g. od -xa) of the attributes' content would

> make this safer?



Yeah, we could change the `cat` to `od -xa`. *Since the reIPL code only

ever wrote ASCII data, I thought that's what it would only contain. *And,

if some other reIPL value were ever set, then showing safe informational

data on where it is rebooting into would be harmless what ever it contained.



--

Mark



You must be the change you wish to see in the world. -- Mahatma Gandhi

Worrying is praying for that you do not wish to happen.

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

On 11/18/2009 04:48 AM, Mark Hamzy wrote:
> Steffen Maier <maier@linux.vnet.ibm.com> wrote on 11/17/2009 09:18:42 PM:
>
>>>> function doreboot()
>>>> {
>>>> + if [ -e "/sys/firmware/reipl" ]; then
>>>> + read REIPL_TYPE < /sys/firmware/reipl/reipl_type
>>>> + echo "reipl_type=$REIPL_TYPE"
>>>> + pushd /sys/firmware/reipl/$REIPL_TYPE >/dev/null 2>&1
>> I see two problems with the informative code. First, anaconda supports
>> the two reipl types ccw for DASD and fcp for zFCP. The kernel interface
>> also supports other stuff such as nss (named saved segments). I think we
>> should only descend into the reipl type specific subdirectory and get
>> additional information for the two types anaconda supports because
>> anaconda does not know anything about the other types and how to
>> interpret their information. Which is fine, BTW, since those types are
>> not needed for installation.
>
> While the kernel interface supports other types, anaconda only sets one
> of the two. Which would make this code safe, right? It also is simpler:
> by keeping tests and specialized code out of the picture.

I considered this informational output to also serve problem
determination should anything not work as expected with reipl support in
anaconda. Just like the case we had in the corresponding bug here. In
the general case, any binary value could be read from some sysfs attributes.

>>>> + for i in *; do
>>>> + echo "$i=`cat $i`"
>> I'm not sure, if all sysfs attributes will always contain printable
>> ASCII text. I wouldn't want any potential binary data misconfigure the
>> console. Maybe a hexdump (e.g. od -xa) of the attributes' content would
>> make this safer?
>
> Yeah, we could change the `cat` to `od -xa`. Since the reIPL code only
> ever wrote ASCII data, I thought that's what it would only contain. And,
> if some other reIPL value were ever set, then showing safe informational,
> data on where it is rebooting into would be harmless what ever it
> contained.

Currently we write or reset a certain set of syfs attributes depending
on the reipl type. In the past, we had the case that new sysfs
attributes were added. Of such new ones, we would not know about their
content representation. Hence I would assume the worst case and
therefore make this "debug" output safe for all possible cases and not
just the good ones, where we wouldn't need the output since reipl just
works as expected.

For those attributes we already know and are sure that they contain
printable ASCII text, we may make an exception and print them directly.
But that would require distinguishing between known and unknown attributes.

Steffen

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