On 01/22/2010 12:21 AM, David Cantrell wrote:
> Remove linuxrc.s390's usage of /sbin/ip, /sbin/ifconfig, and /sbin/route
> and instead just write the settings to the ifcfg-DEVICE file, set the
> device online in sysfs, and start NetworkManager. This prevents a bug
> where linuxrc.s390 was bringing up the interface, then loader would
> start, it would launch NM, and immediately the network connection would
> come down.
Apparently, this looks like a bug in NM. If it was well-behaved, it
wouldn't touch already correctly configured devices and just kept them
running.
Changing linuxrc.s390 is just a workaround and even more so removes a
whole lot of usability that was just introduced and users liked it.
Therefore, I disagree with below changes.
More specific comments below.
> Starting NM earlier prevents this and simplifies
> linuxrc.s390 a lot.
> ---
> loader/linuxrc.s390 | 313 ++++-----------------------------------------------
> 1 files changed, 22 insertions(+), 291 deletions(-)
>
> diff --git a/loader/linuxrc.s390 b/loader/linuxrc.s390
> index 7a6be69..dfdd8d9 100644
> --- a/loader/linuxrc.s390
> +++ b/loader/linuxrc.s390
> @@ -660,30 +660,6 @@ function set_device_online() {
> fi
> }
>
> -# sets device up and blocks until device appears to be up
> -function set_device_up() {
> - if [ -z "$DEVICE" ]; then
> - echo $"Could not determine interface name to bring up device $SUBCHANNELS"
> - return 1
> - fi
> - # Device does not come up fast enough to use "ip" to configure, so block.
> - # While OSA come up themselves after setting online,
> - # e.g. HiperSockets won't => set them up explicitly for the following check
> - debug ip link set up $DEVICE
> - local i=1
> - while : ; do
> - local tst=$(ip -o link show up dev $DEVICE)
> - [ -n "$tst" ] && break
> - sleep 1
> - i=$((i+1))
> - if [ "$i" -gt 10 ]; then
> - echo $"Could not bring up device $DEVICE within timeout"
> - return 1
> - fi
> - done
> - return 0
> -}
> -
By removing this, you rely on enough time to have passed until NM gets
started which is fragile. Reminds me of device_cio_free or dasd_settle
et al.
> @@ -805,7 +726,6 @@ function rollback_config() {
> fi
> fi
> [ -z "$mtu_was_set" ] && unset MTU
> - [ -z "$mmtu_was_set" ] && unset MMTU
This is still required for show_parm() to work. I never fully understood
why this magic is there to distinguish between user input and parm/conf
file input. However, I wouldn't want to break any old feature that was
in linuxrc.s390 before I touched it.
> @@ -1632,17 +1552,6 @@ function syntax_check_macaddr() {
> return 1
> }
>
> -function handle_macaddr() {
> - # - try to set macaddr right here w/ error handlg.
> - # device needs to be online
> - if debug ifconfig $DEVICE hw ether $MACADDR; then
> - return 0
> - fi
> - echo $"MAC address $MACADDR could not be configured for"
> - echo $" $SUBCHANNELS (network device $DEVICE)"
> - return 1
> -}
> -
Have you tested this successfully with a real (non-virtual) OSA in
layer2 mode?
I'm just not sure, if NM will apply this MAC address early on the device.
> -### NETWORK
> -
> -function do_network() {
> - echo
> - echo $"The NETWORK parameter isn't used anymore and will be ignored."
> - echo $" It is sufficient to specify IPADDR and NETMASK."
> - echo
> -}
> -
> -### BROADCAST
> -
> -function do_broadcast() {
> - echo
> - echo $"The BROADCAST parameter isn't used anymore and will be ignored."
> - echo $" It is sufficient to specify IPADDR and NETMASK."
> - echo
> -}
> -
Those two functions are there on purpose, to help users with old
parm/conf file and to acquaint them with the changes we introduced.
Otherwise they would most probably never get to know that there are
parameters in their parm/conf files they do not longer need or they
wonder why they don't get applied any more. Please keep those functions.
> ### NETMASK (IPv6)
>
> function syntax_check_prefix_v6() {
> @@ -2015,43 +1906,6 @@ function do_netmask_v6() {
>
> ### GATEWAY (IPv4)
>
> -function configure_ipv4_gateway() {
> - # FIXME:
> - # - Strictly speaking we should first check reachability of gateway
> - # and then configure the gateway route.
> - # This would require a new intermediate workflow_item step
> - # so that the user might continue despite unreachable gateway.
> - # done: Only adding default route might add multiple undesired default
> - # routes on redoing the parameter item, so delete default route
> - # before adding a new one.
> - ip -4 route del default dev $DEVICE >& /dev/null
> - [ -z "$GATEWAY" ] && return 0
> - if ! tv route add default gw $GATEWAY dev $DEVICE; then
> - echo $"Could net set default route on device $DEVICE via gateway $GATEWAY"
> - return 1
> - fi
> - # BH FIXME: Workaround for manual MACADDR, need ping to update arp table
> - echo $"Trying to reach gateway $GATEWAY..."
> - if [ "$NETTYPE" = "ctc" ]; then
> - # (virtual) CTC(/A) seems to need some time to get functional
> - local i=1
> - while : ; do
> - ping -c 1 $GATEWAY >& /dev/null && break
> - i=$((i+1))
> - if [ "$i" -gt 3 ]; then
> - echo $"Could not reach gateway $GATEWAY within timeout"
> - return 1
> - fi
> - done
> - else
> - if ! ping -c 1 $GATEWAY >& /dev/null; then
> - echo $"Could not reach your default gateway $GATEWAY"
> - return 1
> - fi
> - fi
> - return 0
> -}
> -
Now, we are starting to loose significant parts of usability, which was
the sole purpose of rewriting linuxrc.s390 in the first place. It is a
step back towards the old linuxrc which would only ask all parameters
and then try to configure the network ignoring error cases and not
helping the user to confirm his/her entries were syntactically and even
more so semantically correct.
> ### GATEWAY (IPv6)
>
> -function configure_ipv6_gateway() {
> - # FIXME:
> - # - Strictly speaking we should first check reachability of gateway
> - # and then configure the gateway route.
> - # This would require a new intermediate workflow_item step
> - # so that the user might continue despite unreachable gateway.
> - # done: Only adding default route might add multiple undesired default
> - # routes on redoing the parameter item, so delete default route
> - # before adding a new one.
> - ip -6 route del default dev $DEVICE >& /dev/null
> - [ -z "$GATEWAY" ] && return 0
> - # IPv6 http://www.ibiblio.org/pub/Linux/docs/HOWTO/other-formats/html_single/Linux+IPv6-HOWTO.html#AEN1147
> - # ip -6 route add ::/0 dev $DEVICE via $GATEWAY
> - # (Could also be learned by autoconfiguration on the link:
> - # after IP address setup and device up,
> - # see if default route has been learned
> - # ip -6 route show | grep ^default
> - # However, we currently use manual IPv6 configuration only.)
> - if ! debug ip -6 route add ::/0 dev $DEVICE via $GATEWAY; then
> - echo $"Could net set default route on device $DEVICE"
> - echo $" via gateway $GATEWAY"
> - return 1
> - fi
> - # BH FIXME: Workaround for manual MACADDR, need ping to update arp table
> - echo $"Trying to reach gateway $GATEWAY..."
> - if ! ping6 -c 1 $GATEWAY >& /dev/null; then
> - echo $"Could not reach your default gateway $GATEWAY"
> - return 1
> - fi
> - return 0
> -}
> -
usability--;
> @@ -2261,49 +2061,6 @@ function syntax_check_dns() {
> fi
> }
>
> -function handle_dns() {
> - # - foreach DNS try if server is reachable by one ping
> - [ -z "$DNS" ] && return 0
> - local dnsitem
> - local allgood="yes"
> - echo $"Trying to reach DNS servers..."
> - if [ "$ipv6" ]; then
> - while read dnsitem; do
> - if ! ping6 -c 1 $dnsitem >& /dev/null; then
> - echo $"Could not ping DNS server (might still serve DNS requests): $dnsitem"
> - allgood="no"
> - # this should not be a hard failure since some network
> - # environments may prevent pings to DNS servers
> - # => prevent workflow_item_menu in kickstart mode
> - fi
> - done < <(echo $DNS | sed 's/,/\n/g')
> - else
> - while read dnsitem; do
> - # Some network environment may prevent a DNS server from being
> - # reachable by ping, so it would make sense to use nslookup.
> - # However, nslookup fails with "Resolver Error 0 (no error)"
> - # at this stage of the setup progress => not useful
> - if ! ping -c 1 $dnsitem >& /dev/null; then
> - echo $"Could not ping DNS server: $dnsitem"
> -# if nslookup $dnsitem $dnsitem >& /dev/null; then
> -# echo $" but could resolve DNS server with itself: $dnsitem"
> -# else
> -# echo $"Could not resolve DNS server with itself: $dnsitem"
> -# allgood="no"
> -# fi
> -# elif ! nslookup $dnsitem $dnsitem >& /dev/null; then
> -# echo $"Could not resolve DNS server with itself: $dnsitem"
> - allgood="no"
> - fi
> - done < <(echo $DNS | sed 's/:/\n/g')
> - fi
> - if [ "$allgood" = "yes" ]; then
> - return 0
> - else
> - return 1
> - fi
> -}
> -
usability--;
I think it's less than zero already.
> @@ -2674,9 +2431,7 @@ EOF
> [ "$PORTNO" ] && echo "PORTNO=$PORTNO"
> [ "$PEERID" ] && echo "PEERID=$PEERID"
> [ "$CTCPROT" ] && echo "CTCPROT=$CTCPROT"
> - if [ -n "$mmtu_was_set" ]; then
> - echo "MMTU=\"$MMTU\""
> - elif [ -n "$mtu_was_set" ]; then
> + if [ -n "$mtu_was_set" ]; then
> echo "MTU=$MTU"
> fi
> [ "$DNS" ] && echo "DNS=$DNS"
>
> @@ -2786,18 +2541,14 @@ fi
> # Perform a network installation
>
> [ -n "$MTU" ] && mtu_was_set=$MTU
> -[ -n "$MMTU" ] && mmtu_was_set=$MMTU
What does this change cause? Did you test that this does not break any
previously available behavior?
> [ -n "$VSWITCH" ] && vswitch_was_set=$VSWITCH
>
> [ -n "$CHANDEV" ] && do_chandev
> -[ -n "$NETWORK" ] && do_network
> -[ -n "$BROADCAST" ] && do_broadcast
Those two should definitely stay.
> else # ctc0
> - if [ -z "$NETMASK" ]; then
> - # If the user did not supply netmask, we add the right one.
> - # Netmask MUST be present,
> - # or pumpSetupInterface() blows routes.
> - NETMASK="255.255.255.255"
> - fi
This does not work well with show_parms() which shows NETMASK=$NETMASK
unconditionally.
> - # don't ask for MTU, but use it if set in the parm file
> - # don't overwrite MMTU if it has been set for CTC
> - [ "$NETTYPE" = "ctc" -a -z "$MTU" -a -z "$MMTU" ] && \
> - MMTU="mtu 1500"
Are you sure this keeps working for CTC?
> @@ -2964,9 +2695,7 @@ NETTYPE="$NETTYPE"
> IPADDR="$IPADDR"
> GATEWAY="$GATEWAY"
> MTU="$MTU"
> -NETWORK="$NETWORK"
> NETMASK="$NETMASK"
> -BROADCAST="$BROADCAST"
> SEARCHDNS="$SEARCHDNS"
> PEERID="$PEERID"
> SUBCHANNELS="$SUBCHANNELS"
> @@ -2987,7 +2716,7 @@ fi
> cat >> /tmp/install.cfg << EOF
> export LANG PORTNAME S390ARCH TEXTDOMAIN TEXTDOMAINDIR
> export HOSTNAME DEVICE NETTYPE IPADDR GATEWAY MTU
> -export NETWORK NETMASK BROADCAST DNS DNS1 DNS2 SEARCHDNS
> +export NETMASK DNS DNS1 DNS2 SEARCHDNS
You remove parts of install.cfg but the rest remains. I don't understand
what this would mean for loader.
> export PEERID ONBOOT SUBCHANNELS CTCPROT
> EOF
> # immediately read it in again to export these into the shell below
> @@ -3002,7 +2731,7 @@ if [ ! -d "$NETSCRIPTS" ]; then
> mkdir -p $NETSCRIPTS
> fi
>
> -# to please NetworkManager on startup in loader before loader reconfigures net
> +# nm-system-settings reads the ifcfg-DEVICE files
> cat > /etc/sysconfig/network << EOF
> HOSTNAME=$HOSTNAME
> EOF
> @@ -3012,7 +2741,6 @@ DEVICE=$DEVICE
> ONBOOT=yes
> BOOTPROTO=static
> GATEWAY=$GATEWAY
> -BROADCAST=$BROADCAST
I suppose NM doesn't need this.
But does initscripts' network service still work with OSA, Hipersockets,
LCS, and CTC in all its flavors after the BROADCAST option is written no
more?
As long as NM does not support features such as bonding, data center
servers still need network service.
> @@ -3084,6 +2814,7 @@ EOF
> # reboot on SIGUSR2
> trap doreboot SIGUSR2
>
> + /usr/sbin/NetworkManager --pid-file=/var/run/NetworkManager/NetworkManager.pid
Would we have to wait for NM as in iface.c:iface_start_NetworkManager()
for the network to be configured before sshd binds in startinetd?
And will loader stop calling iface_start_NetworkManager() or does
NetworkManager detect itself that it is already running?
> startinetd
>
> if [ -n "$RUNKS" ]; then
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
