[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#406231: ppp_2.4.4rel-5 patch



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

Frans Pop wrote:
> On Monday 15 January 2007 15:34, you wrote:
>>> Yes, I know, I am aware of this issue and was aware of it from the
>>> start. The main problem is that the interfaces file does not allow
>>> comments at the end of lines with useful information, so doing
>>> something like I did for the /etc/resolv.conf file is not possible.
> 
> IMO the interfaces file should be created in the d-i environment just as 
> is done in netcfg. For me it makes sense to keep these two as much in 
> parallel as possible.

Ok.

>>> I was thinking of setting netcfg's templates to choose "Do not
>>> configure the network at this time" and make it skip directly to the
>>> hostname question. I did some tests this morning, but they are
>>> inconclusive yet. Still I am confident I can do this.
>> I have tested a few combinations of netcfg templates setting, but no
>> luck yet. Still I don't understand why this didn't help:
> 
> This really is totally broken. The value "Do not configure network" could 
> in theory be tested in later udebs and then the code there would assume 
> that you do not have networking even when in fact you do.

I see.

>> The main idea is to let the network unconfigured, but let netcfg to set
>> hosts, lo and hostname.
> 
> No, ppp-udeb should take care of that all by itself. At least if you want 
> this implemented in time for Etch.

Yes, I want that in time for Etch :-)

> I've attached an alternative patch (against current version in unstable). 
> This does things as I think it is best to do them for Etch, resulting in 
> a properly setup network and with the least risk of unexpected 
> side-effects.

Thanks, I was stuck and out of ideas.

> I've also fixed a number of minor errors in the existing script and your 
> patch.

Thanks.

> Please check it thoroughly. If you have any questions, let me know.
> Note that the patch is almost entirely untested.

I guess I could call people to test the new ppp-udeb version and setup in qemu
a server and a client (although I have failed in the past).

> +  [ Frans Pop ]
> +  * ppp-udeb: general review resulting in some corrections and restructuring
> +  * ppp-udeb: set up loopback interface in installer environment
> +  * ppp-udeb: ask for hostname and check validity (using netcfg template)
> +  * ppp-udeb: create /etc/hostname and basic /etc/hosts files
> +
> + -- Frans Pop <fjp@debian.org>  Wed, 17 Jan 2007 16:57:46 +0100
> +
>  ppp (2.4.4rel-4.1) unstable; urgency=low
>  
>    * Non-maintainer upload with maintainer's consent.
> diff -urN ppp-2.4.4rel.orig/debian/control ppp-2.4.4rel/debian/control

> diff -urN ppp-2.4.4rel.orig/debian/ppp-udeb.postinst ppp-2.4.4rel/debian/ppp-udeb.postinst
> --- ppp-2.4.4rel.orig/debian/ppp-udeb.postinst	2007-01-17 16:41:27.000000000 +0100
> +++ ppp-2.4.4rel/debian/ppp-udeb.postinst	2007-01-17 16:41:01.000000000 +0100
> @@ -74,26 +74,47 @@
>  }
>  
>  reset_if_needed() {
> -# bring down the pppoe connection made before, if that's the case
> -PIDF=/var/run/ppp-provider.pid
> -if [ -e $PIDF ]
> -then
> -	PID=$(cat $PIDF)
> -	log "found pid file $PIDF which reffers to process $PID; searching the pppd process"
> -	if [ -z "$(ps | grep "^\s*$PID" | sed "s/^\s*$PID\s.*$/$PID/")" ]
> -	then
> -		log "$PID not found; removing pid file"
> -	else
> -		log "$PID found; killing it and removing pid file"
> -		kill $PID || true
> +	# Bring down an earlier pppoe connection, if there is one
> +	PIDF=/var/run/ppp-provider.pid
> +	if [ -e $PIDF ]; then
> +		PID=$(cat $PIDF)
> +		log "found PID file $PIDF which refers to process $PID; searching for the pppd process"
> +		if [ -z "$(ps | grep "^\s*$PID" | sed "s/^\s*$PID\s.*$/$PID/")" ]; then
> +			log "$PID not found; removing pid file"
> +		else
> +			log "$PID found; killing it and removing pid file"
> +			kill $PID || true
> +		fi
> +		rm -f $PIDF
>  	fi
> -	rm -f $PIDF
> -fi
>  
> -# bring down previously rised interface
> -[ "$PPPOE" = "_" ] || ifconfig "$PPPOE" down && db_set ppp/interface "_" || true
> +	# Bring down previously raised interface
> +	[ "$PPPOE" = "_" ] || ifconfig "$PPPOE" down && db_set ppp/interface "_" || true
>  }
>  
> +valid_hostname() {
> +	if [ $(echo -n "$1" | wc -c) -lt 2 ] ||
> +	   [ $(echo -n "$1" | wc -c) -gt 63 ] ||
> +	   [ "$(echo -n "$1" | sed 's/[^-\.[:alnum:]]//g')" != "$1" ] ||
> +	   [ "$(echo -n "$1" | grep "\(^-\|-$\)")" ]; then
> +		return 1
> +	fi
> +	return 0
> +}
> +
> +# Sanity check: we rely on some netcfg functionality but cannot depend on it;
> +# netcfg should always be present, but bail out if it is not
> +if [ ! -e /bin/netcfg ]; then
> +	fail "required package netcfg is not installed"
> +	exit 1
> +fi
> +
> +# Bring up the loopback interface
> +if [ -z "$(ip link show lo up)" ]; then
> +	ip link set lo up
> +	ip addr flush dev lo
> +	ip addr add 127.0.0.1/8 dev lo
> +fi
>  
>  if [ -z "$INTERFACES" ]; then
>  	fail "no Ethernet interfaces detected"
> @@ -104,9 +125,7 @@
>  
>  reset_if_needed
>  
> -# test each of the interfaces for a concentrator,
> -# then stop when one is found.
> -
> +# Test each of the interfaces for a concentrator; stop when one is found
>  for IFACE in $INTERFACES; do
>  	if ppp_concentrator_on $IFACE; then
>  		log "setting pppoe connection on $IFACE"
> @@ -131,23 +150,71 @@
>  fi
>  
>  db_input high ppp/username || true
> -db_go || true
> +db_go || exit 30

What is the difference induced by return code 30? Will it make the
installation fail if ran non-interactively and this data is not provided?

(I don't seem to be able to find again the codes and their meaning for
main-menu, although I found it in the past.)

>  db_get ppp/username
>  USERNAME="$RET"
>  
>  db_input high ppp/password || true
> -db_go || true
> +db_go || exit 30
>  db_get ppp/password
>  PASSWORD="$RET"
>  
> -# just to be sure that the answers will not be cached if the script
> -# is run a second time
> +# Clear answers in case the script is run a second time
>  db_unregister ppp/password
>  db_unregister ppp/username
>  
>  
> +# Ask for the hostname to use for the system (using the netcfg template!)
> +while true; do
> +	db_input high netcfg/get_hostname
> +	db_go || exit 30
> +	db_get netcfg/get_hostname
> +	HOSTNAME="$RET"
> +	if valid_hostname "$HOSTNAME"; then
> +		break
> +	fi
> +	db_input high netcfg/invalid_hostname
> +	db_fset netcfg/get_hostname seen false
> +done

This looks ok.

> +# FIXME: lo snippet should not be ppp-udeb's job
> +cat > /etc/network/interfaces <<EOF
> +# This file describes the network interfaces available on your system
> +# and how to activate them. For more information, see interfaces(5).
> +
> +# The loopback network interface
> +auto lo
> +iface lo inet loopback
> +
> +# PPPoE connection
> +auto provider
> +iface provider inet ppp
> +	pre-up /sbin/ifconfig $ETH up

(Just checking)
In order for this to work, the interface name should be preserved. This
happens, AFAIK, even for interfaces which are not configured, right?

> +	provider provider
> +EOF
> +
> +# Set hostname
> +if [ "$HOSTNAME" ]; then
> +	echo "$HOSTNAME" >/etc/hostname
> +fi
> +
> +# Create a basic /etc/hosts file
> +cat > /etc/hosts <<EOF
> +127.0.0.1       localhost	$HOSTNAME
> +
> +# The following lines are desirable for IPv6 capable hosts
> +::1     ip6-localhost ip6-loopback
> +fe00::0 ip6-localnet
> +ff00::0 ip6-mcastprefix
> +ff02::1 ip6-allnodes
> +ff02::2 ip6-allrouters
> +ff02::3 ip6-allhosts
> +EOF
> +
> +
>  cat <<EOF > /etc/ppp/peers/provider
> -# kernel space PPPoE driver example configuration
> +# kernel space PPPoE driver configuration
>  #
>  # See the manual page pppd(8) for information on all the options.
>  
> @@ -207,9 +274,19 @@
>  log-output -t depmod
>  log-output -t ppp-udeb modprobe pppoe
>  
> -log-output -t ppp-udeb pppd call provider || true
> -
> -log-output apt-install ppp || true
> +RET=0
> +log-output -t ppp-udeb pppd call provider || RET=$?
> +if [ $RET -eq 19 ]; then
> +	fail "wrong login info detected"
> +	db_input critical ppp/wrong_login || true
> +	db_go || true
> +	exit 1
> +elif [ $RET -ne 0 ]; then
> +	fail "unhandled error detected"
> +	db_input critical ppp/unhandled || true
> +	db_go || true
> +	exit 1
> +fi
>  
>  #DEBHELPER#
>  
> diff -urN ppp-2.4.4rel.orig/debian/ppp-udeb.todo ppp-2.4.4rel/debian/ppp-udeb.todo
> --- ppp-2.4.4rel.orig/debian/ppp-udeb.todo	2007-01-17 16:41:27.000000000 +0100
> +++ ppp-2.4.4rel/debian/ppp-udeb.todo	2007-01-17 16:41:01.000000000 +0100
> @@ -1,4 +1,3 @@
> -- wrong authentication must be treated, not ignored
>  - how to blend well with netcfg?
>  	- the new patch proposes assigning menu order 17 to ppp-udeb, so
>  	  is ran before netcfg
> diff -urN ppp-2.4.4rel.orig/debian/rules ppp-2.4.4rel/debian/rules
> --- ppp-2.4.4rel.orig/debian/rules	2007-01-17 16:41:27.000000000 +0100
> +++ ppp-2.4.4rel/debian/rules	2007-01-17 16:41:01.000000000 +0100
> @@ -138,7 +138,7 @@
>  
>  ifdef BUILD_UDEB
>  	dh_installdirs -p ppp-udeb etc/ppp/peers/ usr/sbin/ \
> -		usr/lib/pppd/$(PPPDDIR)/ usr/lib/finish-install.d/
> +		usr/lib/pppd/$(PPPDDIR)/ usr/lib/post-base-installer.d/
>  	grep '^[a-zA-Z0-9]' extra/options > $D-udeb/etc/ppp/options
>  #	cp $B/chat/chat $D-udeb/usr/sbin/
>  	cp $B/pppd-udeb/pppd $D-udeb/usr/sbin/
> @@ -150,8 +150,8 @@
>  	cp extra/ppp-udeb.ip-up \
>  		$D-udeb/etc/ppp/ip-up
>  	chmod 0755 $D-udeb/etc/ppp/ip-up
> -	install -m755 extra/50config-target-ppp \
> -		$D-udeb/usr/lib/finish-install.d/
> +	install -m 755 extra/config-target-ppp \
> +		$D-udeb/usr/lib/post-base-installer.d/30ppp;

Why change the name of the script in the source package _and_ the name in the
d-i environment? Wouldn't it make more sense to rename it directly in the
source? (I suggest 30ppp-udeb-config.) This would also save some source code
reader (later) the trouble to understand where did the config-target-ppp file
went.

>  	dh_installdebconf
>  endif
>  
> diff -urN ppp-2.4.4rel.orig/extra/50config-target-ppp ppp-2.4.4rel/extra/50config-target-ppp
> --- ppp-2.4.4rel.orig/extra/50config-target-ppp	2007-01-17 16:41:27.000000000 +0100
> +++ ppp-2.4.4rel/extra/50config-target-ppp	1970-01-01 01:00:00.000000000 +0100
> @@ -1,6 +0,0 @@
> -#!/bin/sh -e
> -# Copy the ppp configuration to the target system.
> -
> -mkdir -p /target/etc/ppp
> -cp /etc/ppp/*-secrets /target/etc/ppp/
> -cp -a /etc/ppp/peers /target/etc/ppp/
> diff -urN ppp-2.4.4rel.orig/extra/config-target-ppp ppp-2.4.4rel/extra/config-target-ppp
> --- ppp-2.4.4rel.orig/extra/config-target-ppp	1970-01-01 01:00:00.000000000 +0100
> +++ ppp-2.4.4rel/extra/config-target-ppp	2007-01-17 16:57:33.000000000 +0100
> @@ -0,0 +1,16 @@
> +#!/bin/sh -e
> +# Configure ppp for the target system
> +# Note: netcfg takes care of general networking configuration files
> +
> +# We can only do this after ppp has been installed to ensure correct permissions
> +apt-install ppp || true
> +
> +if [ ! -d /target/etc/ppp/peers ]; then
> +	logger -t ppp-udeb "Error: directory /target/etc/ppp/peers does not exist"
> +	logger -t ppp-udeb "There may have been an error installing ppp"
> +	exit 1
> +fi
> +
> +# We copy over already existing files, so permissions are preserved
> +cp /etc/ppp/*-secrets /target/etc/ppp/
> +cp /etc/ppp/peers/provider /target/etc/ppp/peers/

Not sure if copying the whole /etc/ppp/peers/ directory isn't a better idea
(in the style of what is done with the configuration files from d-i).
Having that in mind, aren't the configuration files already copied in the
target *with* the correct permissions when this script is ran?

- --
Regards,
EddyP
=============================================
"Imagination is more important than knowledge" A.Einstein
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFFrz7aY8Chqv3NRNoRAhWFAJ9LLPlA9gEauG1VtU8z4x8P/L/gVwCfQA9V
Yzqp62xzYw4OszohIHyrCks=
=ItDP
-----END PGP SIGNATURE-----



Reply to: