All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC ONLY 0/5] Move argument validation to root handlers
@ 2009-07-03 21:40 David Dillow
       [not found] ` <cover.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-03 21:40 UTC (permalink / raw)
  To: initramfs-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow

This patchset will need to be rebased onto Harold's series from today, but
I wanted to post where I was at to get some feedback anyway.

This series moves the dracut syntax parsing and validation into the root
handlers, and leaves the legacy syntax translation and other warnings to
the command line argument parsers. Moving the parsing into the handlers
centralizes the code and allows us to leverage that work to implement a
mechanism to wait for a spanning tree to stop blocking traffic prior to
attempting a mount.

Parts of the last patch should probably be merged with patch 4 -- there
is no need to add a 'checkdhcp' command to the root handlers only to turn
around and rename it in the next patch -- and is still rather rough. I'm
not keen on using /tmp/server to pass IPs back to netroot, but having it
echo'd from the netroot handler means we break when using rdnetdebug. That
is easily fixable by not redirecting stdout during rdnetdebug, but I need
to look closer to make sure that doesn't break anything else.

Also, the waiting for spanning tree parts will need to be adapted to handle
multiple IP addresses for iSCSI, if and when we support that.

I've tried to not change syntax support during the conversion, limiting
support to the existing ones. The current test suite passes at all steps,
but a new test suite to test our parsing (with expected failures) may be
a good thing to add.

iSCSI remains a mess -- I think we need to look at what we're supporitng
there, and improve the validation. I moved some checks into iscsiroot, but
it is a bit unclear where the legacy syntaxes came from, and what we
really want to support going forward. Also, we need to be able to parse the
iSCSI firmware tables to figure out which IP to use for arping.

David Dillow (5):
  netroot: remove unused hook
  netroot: remove unhelpful argument checks
  netroot: remove netif from handler invocation
  netroot: move dracut syntax validation to root handlers
  PROOF-OF-CONCEPT: wait for spanning tree timeout via arping

 dracut                               |    2 +-
 modules.d/40network/dhclient-script  |   11 ++-
 modules.d/40network/ifup             |    8 ++-
 modules.d/40network/install          |    2 +-
 modules.d/40network/netroot          |  101 +++++++++++++++-------
 modules.d/95iscsi/iscsiroot          |   57 +++++++++----
 modules.d/95iscsi/parse-iscsiroot.sh |   21 +----
 modules.d/95nbd/nbdroot              |   37 +++++---
 modules.d/95nbd/parse-nbdroot.sh     |   34 ++------
 modules.d/95nfs/nfsroot              |  158 ++++++++++++++++++----------------
 modules.d/95nfs/parse-nfsroot.sh     |  155 ++++++++++++---------------------
 11 files changed, 302 insertions(+), 284 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [RFC ONLY 1/5] netroot: remove unused hook
       [not found] ` <cover.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
@ 2009-07-03 21:40   ` David Dillow
       [not found]     ` <bd4d79bc048ad3944156f86eafb62cf4bc29b1f9.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  2009-07-03 21:40   ` [RFC ONLY 2/5] netroot: remove unhelpful argument checks David Dillow
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-03 21:40 UTC (permalink / raw)
  To: initramfs-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow

The netroot hook is no longer used.
---
This is a left over from before Philippe's multinic work.

 dracut                      |    2 +-
 modules.d/40network/netroot |    3 ---
 2 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/dracut b/dracut
index f10af83..c71cc63 100755
--- a/dracut
+++ b/dracut
@@ -102,7 +102,7 @@ if [[ -f $outfile && ! $force ]]; then
     exit 1
 fi
 
-hookdirs="cmdline pre-udev pre-trigger netroot pre-mount pre-pivot mount emergency"
+hookdirs="cmdline pre-udev pre-trigger pre-mount pre-pivot mount emergency"
 
 readonly initdir=$(mktemp -d -t initramfs.XXXXXX)
 trap 'rm -rf "$initdir"' 0 # clean up after ourselves no matter how we die.
diff --git a/modules.d/40network/netroot b/modules.d/40network/netroot
index f719d93..645bbba 100755
--- a/modules.d/40network/netroot
+++ b/modules.d/40network/netroot
@@ -92,9 +92,6 @@ done
 [ -e /tmp/net.$netif.hostname ]    && . /tmp/net.$netif.hostname
 [ -e /tmp/net.$netif.resolv.conf ] && cp -f /tmp/net.$netif.resolv.conf /etc/resolv.conf
 
-# Source netroot hooks before we start the handler
-source_all netroot
-
 # Run the handler; don't store the root, it may change from device to device
 # XXX other variables to export?
 if $handler $netif $netroot $NEWROOT; then
-- 
1.6.0.6

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC ONLY 2/5] netroot: remove unhelpful argument checks
       [not found] ` <cover.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  2009-07-03 21:40   ` [RFC ONLY 1/5] netroot: remove unused hook David Dillow
@ 2009-07-03 21:40   ` David Dillow
       [not found]     ` <0535c73bd25fdcaa9bf4de0bc4b041b74d11e547.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  2009-07-03 21:40   ` [RFC ONLY 3/5] netroot: remove netif from handler invocation David Dillow
                     ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-03 21:40 UTC (permalink / raw)
  To: initramfs-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow

The argument checks for the network root handlers are not very useful, as
there is no error message; things just fail to work. Since this should only
ever be a problem when developing dracut, it is not too unfriendly to expect
developers to add rdnetdebug to the command line when working on this area.

Remove them for now, as the arguments will be going through some flux and
we can add them back later with more helpful messages if it is desired.
---
 modules.d/95iscsi/iscsiroot |   10 ----------
 modules.d/95nbd/nbdroot     |    9 ---------
 modules.d/95nfs/nfsroot     |    9 ---------
 3 files changed, 0 insertions(+), 28 deletions(-)

diff --git a/modules.d/95iscsi/iscsiroot b/modules.d/95iscsi/iscsiroot
index c66f9d2..2b041cc 100755
--- a/modules.d/95iscsi/iscsiroot
+++ b/modules.d/95iscsi/iscsiroot
@@ -16,16 +16,6 @@ if getarg rdnetdebug; then
     set -x
 fi
 
-# Huh? Empty $1?
-[ -z "$1" ] && exit 1
-
-# Huh? Empty $2?
-[ -z "$2" ] && exit 1
-
-# Huh? Empty $3? This isn't really necessary, since NEWROOT isn't 
-# used here. But let's be consistent
-[ -z "$3" ] && exit 1
-
 # root is in the form root=iscsi:[<servername>]:[<protocol>]:[<port>]:[<LUN>]:<targetname>
 netif="$1"
 root="$2"
diff --git a/modules.d/95nbd/nbdroot b/modules.d/95nbd/nbdroot
index 90f6ef8..c7f24e8 100755
--- a/modules.d/95nbd/nbdroot
+++ b/modules.d/95nbd/nbdroot
@@ -10,15 +10,6 @@ if getarg rdnetdebug; then
     set -x
 fi
 
-# Huh? Empty $1?
-[ -z "$1" ] && exit 1
-
-# Huh? Empty $2?
-[ -z "$2" ] && exit 1
-
-# Huh? Empty $3?
-[ -z "$3" ] && exit 1
-
 # root is in the form root=nbd:srv:port[:fstype[:rootflags[:nbdopts]]]
 netif="$1"
 root="$2"
diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
index 0da8ee3..f005378 100755
--- a/modules.d/95nfs/nfsroot
+++ b/modules.d/95nfs/nfsroot
@@ -48,15 +48,6 @@ if getarg rdnetdebug ; then
     set -x
 fi
 
-# Huh? Empty $1?
-[ -z "$1" ] && exit 1
-
-# Huh? Empty $2?
-[ -z "$2" ] && exit 1
-
-# Huh? Empty $3?
-[ -z "$3" ] && exit 1
-
 # root is in the form root=nfs[4]:[server:]path[:options], either from
 # cmdline or dhcp root-path
 netif="$1"
-- 
1.6.0.6

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC ONLY 3/5] netroot: remove netif from handler invocation
       [not found] ` <cover.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  2009-07-03 21:40   ` [RFC ONLY 1/5] netroot: remove unused hook David Dillow
  2009-07-03 21:40   ` [RFC ONLY 2/5] netroot: remove unhelpful argument checks David Dillow
@ 2009-07-03 21:40   ` David Dillow
       [not found]     ` <44494af931ad839908cb4542f7c35ae59a4ef65e.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  2009-07-03 21:40   ` [RFC ONLY 4/5] netroot: move dracut syntax validation to root handlers David Dillow
  2009-07-03 21:40   ` [RFC ONLY 5/5] PROOF-OF-CONCEPT: wait for spanning tree timeout via arping David Dillow
  4 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-03 21:40 UTC (permalink / raw)
  To: initramfs-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow

Now that netroot is serialized and only one interface can be configured
at a time, netif is almost completely unneeded by the netroot handlers.
The remaining hold out is nfsroot, which uses it to get the command line
overrides and the DHCP root information.

By adding that information to the calling convention, we can get rid
of netif, and prepare for the next steps of moving parsing and validation
into the handlers themselves.
---
 modules.d/40network/netroot |   16 ++++++++++------
 modules.d/95iscsi/iscsiroot |    7 +++----
 modules.d/95nbd/nbdroot     |    9 ++++-----
 modules.d/95nfs/nfsroot     |   20 ++++++++------------
 4 files changed, 25 insertions(+), 27 deletions(-)

diff --git a/modules.d/40network/netroot b/modules.d/40network/netroot
index 645bbba..c9ebf4c 100755
--- a/modules.d/40network/netroot
+++ b/modules.d/40network/netroot
@@ -44,15 +44,20 @@ done
 netif=$1
 [ -e "/tmp/net.bootdev" ] && read netif < /tmp/net.bootdev
 
+# Load command line overrides to get the server override
+[ -e /tmp/net.$netif.override ] && . /tmp/net.$netif.override
+[ -n "$srv" ] && server_id=$srv
+
+# Load dhcp options and get the first fallback to the server
+[ -e /tmp/dhclient.$netif.dhcpopts ] && . /tmp/dhclient.$netif.dhcpopts
+[ -z "$server_id" ] && server_id=$new_dhcp_server_identifier
+
 # Figure out the handler for root=dhcp by recalling all netroot cmdline 
 # handlers
 if [ "$netroot" = "dhcp" ] ; then
     # Unset root so we can check later
     unset root
 
-    # Load dhcp options
-    [ -e /tmp/dhclient.$netif.dhcpopts ] && . /tmp/dhclient.$netif.dhcpopts
-
     # If we have a specific bootdev with no dhcpoptions or empty root-path, 
     # we die. Otherwise we just warn
     if [ -z "$new_root_path" ] ; then
@@ -92,9 +97,8 @@ done
 [ -e /tmp/net.$netif.hostname ]    && . /tmp/net.$netif.hostname
 [ -e /tmp/net.$netif.resolv.conf ] && cp -f /tmp/net.$netif.resolv.conf /etc/resolv.conf
 
-# Run the handler; don't store the root, it may change from device to device
-# XXX other variables to export?
-if $handler $netif $netroot $NEWROOT; then
+# Run the handler to mount/login into the root device
+if $handler "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
     # Network rootfs mount successful
     for iface in $IFACES ; do
 	[ -f /tmp/dhclient.$iface.lease ] &&    cp /tmp/dhclient.$iface.lease    /tmp/net.$iface.lease
diff --git a/modules.d/95iscsi/iscsiroot b/modules.d/95iscsi/iscsiroot
index 2b041cc..923141c 100755
--- a/modules.d/95iscsi/iscsiroot
+++ b/modules.d/95iscsi/iscsiroot
@@ -11,14 +11,13 @@
 PATH=$PATH:/sbin:/usr/sbin
 
 if getarg rdnetdebug; then
-    exec > /tmp/iscsiroot.$1.$$.out
-    exec 2>> /tmp/iscsiroot.$1.$$.out
+    exec > /tmp/iscsiroot.$$.out
+    exec 2>> /tmp/iscsiroot.$$.out
     set -x
 fi
 
 # root is in the form root=iscsi:[<servername>]:[<protocol>]:[<port>]:[<LUN>]:<targetname>
-netif="$1"
-root="$2"
+root="$1"
 
 # read static conf settings
 for conf in conf/conf.d/*; do
diff --git a/modules.d/95nbd/nbdroot b/modules.d/95nbd/nbdroot
index c7f24e8..f76211c 100755
--- a/modules.d/95nbd/nbdroot
+++ b/modules.d/95nbd/nbdroot
@@ -5,15 +5,14 @@
 PATH=$PATH:/sbin:/usr/sbin
 
 if getarg rdnetdebug; then
-    exec > /tmp/nbdroot.$1.$$.out
-    exec 2>> /tmp/nbdroot.$1.$$.out
+    exec > /tmp/nbdroot.$$.out
+    exec 2>> /tmp/nbdroot.$$.out
     set -x
 fi
 
 # root is in the form root=nbd:srv:port[:fstype[:rootflags[:nbdopts]]]
-netif="$1"
-root="$2"
-NEWROOT="$3"
+root="$1"
+NEWROOT="$4"
 
 # If it's not nbd we don't continue
 [ "${root%%:*}" = "nbd" ] || return
diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
index f005378..847ec60 100755
--- a/modules.d/95nfs/nfsroot
+++ b/modules.d/95nfs/nfsroot
@@ -43,16 +43,17 @@ root_to_var() {
 PATH=$PATH:/sbin:/usr/sbin
 
 if getarg rdnetdebug ; then 
-    exec > /tmp/nfsroot.$1.$$.out
-    exec 2>> /tmp/nfsroot.$1.$$.out
+    exec > /tmp/nfsroot.$$.out
+    exec 2>> /tmp/nfsroot.$$.out
     set -x
 fi
 
 # root is in the form root=nfs[4]:[server:]path[:options], either from
 # cmdline or dhcp root-path
-netif="$1"
-root="$2"
-NEWROOT="$3"
+root="$1"
+server_id=$2
+root_path=$3
+NEWROOT="$4"
 
 # Continue if nfs prefix
 case "${root%%:*}" in
@@ -62,20 +63,15 @@ esac
 
 root_to_var $root
 
-#Load other data that might provide info
-[ -f /tmp/net.$netif.override ] && . /tmp/net.$netif.override
-[ -f /tmp/dhclient.$netif.dhcpopts ] && . /tmp/dhclient.$netif.dhcpopts
-
 #Empty path means try dhcp root-path, this is ok here since parse-nfsroot.sh
 #already takes care of nfs:... formatted root-path
-[ -z "$path" ] && root_to_var $nfs:$new_root_path
+[ -z "$path" ] && root_to_var $nfs:$root_path
 
 #Empty path defaults to "/tftpboot/%s" only in nfsroot.txt legacy mode
 [ -z "$path" ] && [ "$(getarg root=)" = "/dev/nfs" ] && path="/tftpboot/%s"
 
 if [ -z "$server" ] ; then
-    # XXX new_dhcp_next_server is unconfirmed this is an assumption
-    for var in $srv $new_dhcp_server_identifier $new_dhcp_next_server $new_root_path '' ; do
+    for var in $server_id $root_path '' ; do
 	[ -n "$var" ] && server=$var && break;
     done
 
-- 
1.6.0.6

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC ONLY 4/5] netroot: move dracut syntax validation to root handlers
       [not found] ` <cover.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
                     ` (2 preceding siblings ...)
  2009-07-03 21:40   ` [RFC ONLY 3/5] netroot: remove netif from handler invocation David Dillow
@ 2009-07-03 21:40   ` David Dillow
       [not found]     ` <d1aed501ae771d769efcd370f47184f12b9800d6.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  2009-07-03 21:40   ` [RFC ONLY 5/5] PROOF-OF-CONCEPT: wait for spanning tree timeout via arping David Dillow
  4 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-03 21:40 UTC (permalink / raw)
  To: initramfs-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow

This reduces the argument parsers in the cmdline hook to giving warnings
about usage and translating legacy syntaxes to dracut's syntax. The root
handlers themselves become responsible for validating the dracut syntax.
This simplifies the cmdline parsers and centralizes the syntax parsers,
which keeps them from multiplying as new functionality is added.

/sbin/netroot unfortunately grows a special case for root=dhcp to understand
that [ip:]:/path in the root-path option is handled by nfsroot.
---
 modules.d/40network/netroot          |   27 ++----
 modules.d/95iscsi/iscsiroot          |   30 ++++++-
 modules.d/95iscsi/parse-iscsiroot.sh |   21 +----
 modules.d/95nbd/nbdroot              |   19 ++++-
 modules.d/95nbd/parse-nbdroot.sh     |   34 ++------
 modules.d/95nfs/nfsroot              |  140 ++++++++++++++++++-------------
 modules.d/95nfs/parse-nfsroot.sh     |  155 +++++++++++++---------------------
 7 files changed, 203 insertions(+), 223 deletions(-)

diff --git a/modules.d/40network/netroot b/modules.d/40network/netroot
index c9ebf4c..2cf51fa 100755
--- a/modules.d/40network/netroot
+++ b/modules.d/40network/netroot
@@ -52,12 +52,7 @@ netif=$1
 [ -e /tmp/dhclient.$netif.dhcpopts ] && . /tmp/dhclient.$netif.dhcpopts
 [ -z "$server_id" ] && server_id=$new_dhcp_server_identifier
 
-# Figure out the handler for root=dhcp by recalling all netroot cmdline 
-# handlers
 if [ "$netroot" = "dhcp" ] ; then
-    # Unset root so we can check later
-    unset root
-
     # If we have a specific bootdev with no dhcpoptions or empty root-path, 
     # we die. Otherwise we just warn
     if [ -z "$new_root_path" ] ; then
@@ -66,20 +61,13 @@ if [ "$netroot" = "dhcp" ] ; then
 	exit 1
     fi
 
-    # Set netroot to new_root_path, so cmdline parsers don't call
-    netroot=$new_root_path
-
-    for f in ./cmdline/90*.sh; do
-	[ -f "$f" ] && . "$f";
-    done
-else 
-    rootok="1"
+    case "$new_root_path" in
+	[0-9]*:/*|/*) netroot=nfs::: ;;
+	*:|*:*) netroot=$new_root_path ;;
+	*) die "DHCP root-path has no known root protocol" ;;
+    esac
 fi
 
-# Check: do we really know how to handle (net)root?
-[ -z "$root" ] && die "No or empty root= argument"
-[ -z "$rootok" ] && die "Don't know how to handle 'root=$root'"
-
 handler=${netroot%%:*}
 handler=${handler%%4}
 handler="/sbin/${handler}root"
@@ -87,6 +75,9 @@ if [ -z "$netroot" ] || [ ! -e "$handler" ] ; then
     die "No handler for netroot type '$netroot'"
 fi
 
+# Now that we have DHCP information, we can fully validate netroot
+$handler checkdhcp $netroot "$server_id" "$new_root_path" || exit 1
+
 # We're here, so we can assume that upping interfaces is now ok
 [ -z "$IFACES" ] && IFACES="$netif"
 for iface in $IFACES ; do
@@ -98,7 +89,7 @@ done
 [ -e /tmp/net.$netif.resolv.conf ] && cp -f /tmp/net.$netif.resolv.conf /etc/resolv.conf
 
 # Run the handler to mount/login into the root device
-if $handler "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
+if $handler mount "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
     # Network rootfs mount successful
     for iface in $IFACES ; do
 	[ -f /tmp/dhclient.$iface.lease ] &&    cp /tmp/dhclient.$iface.lease    /tmp/net.$iface.lease
diff --git a/modules.d/95iscsi/iscsiroot b/modules.d/95iscsi/iscsiroot
index 923141c..d79b663 100755
--- a/modules.d/95iscsi/iscsiroot
+++ b/modules.d/95iscsi/iscsiroot
@@ -16,8 +16,18 @@ if getarg rdnetdebug; then
     set -x
 fi
 
-# root is in the form root=iscsi:[<servername>]:[<protocol>]:[<port>]:[<LUN>]:<targetname>
-root="$1"
+case "$1" in
+    check|checkdhcp)	check_only=1 ;;
+    mount)		;;
+    *)			die "$0 called with invalid command '$1'" ;;
+esac
+
+# root is in the form
+# root=iscsi:[<servername>]:[<protocol>]:[<port>]:[<LUN>]:<targetname>
+root=$2
+server_id=$3
+root_path=$4
+NEWROOT=$5
 
 # read static conf settings
 for conf in conf/conf.d/*; do
@@ -27,10 +37,26 @@ done
 # If it's not iscsi we don't continue
 [ "${root%%:*}" = "iscsi" ] || exit 1
 
+# Check required arguments. there's only one, but it's at the end
+if ! getarg iscsi_firmware; then
+    case "${netroot##iscsi:*:*:*:*:}" in
+	"$netroot"|'') die "Argument targetname for iscsiroot is missing";;
+    esac
+fi
+
 # XXX modprobe crc32c should go in the cmdline parser, but I haven't yet
 # figured out a way how to check whether this is built-in or not
 modprobe crc32c
 
+# ISCSI actually supported?
+if [ ! -e /sys/devices/virtual/iscsi_transport ]; then
+    if ! modprobe iscsi_tcp; then
+	 die "iscsiroot requested but kernel/initrd does not support iscsi"
+    fi
+fi
+
+[ -n "$check_only" ] && exit 0
+
 if getarg iscsi_firmware ; then
 	iscsistart -b
 	exit 0
diff --git a/modules.d/95iscsi/parse-iscsiroot.sh b/modules.d/95iscsi/parse-iscsiroot.sh
index 1ba4277..1774c5e 100755
--- a/modules.d/95iscsi/parse-iscsiroot.sh
+++ b/modules.d/95iscsi/parse-iscsiroot.sh
@@ -55,21 +55,10 @@ if [ -n "$iscsi_firmware" ] ; then
     netroot=${netroot:-iscsi}
 fi
 
-# If it's not iscsi we don't continue
-[ "${netroot%%:*}" = "iscsi" ] || return
+if [ "${netroot%%:*}" = "iscsi" ]; then
+    /sbin/iscsiroot check $netroot || exit 1
 
-# Check required arguments. there's only one, but it's at the end
-if [ -z "$iscsi_firmware" ] ; then
-    case "${netroot##iscsi:*:*:*:*:}" in
-	$netroot|'') die "Argument targetname for iscsiroot is missing";;
-    esac
+    # All good, keep init from complaining
+    rootok=1
+    [ -z "$root" ] && root="iscsi"
 fi
-
-# ISCSI actually supported?
-[ -e /sys/devices/virtual/iscsi_transport ] || modprobe iscsi_tcp || die "iscsiroot requested but kernel/initrd does not support iscsi"
-
-# Done, all good!
-rootok=1
-
-# Shut up init error check
-[ -z "$root" ] && root="iscsi"
diff --git a/modules.d/95nbd/nbdroot b/modules.d/95nbd/nbdroot
index f76211c..55c1b88 100755
--- a/modules.d/95nbd/nbdroot
+++ b/modules.d/95nbd/nbdroot
@@ -10,9 +10,17 @@ if getarg rdnetdebug; then
     set -x
 fi
 
+case "$1" in
+    check|checkdhcp)	check_only=1 ;;
+    mount)		;;
+    *)			die "$0 called with invalid command '$1'" ;;
+esac
+
 # root is in the form root=nbd:srv:port[:fstype[:rootflags[:nbdopts]]]
-root="$1"
-NEWROOT="$4"
+root=$2
+server_id=$3
+root_path=$4
+NEWROOT=$5
 
 # If it's not nbd we don't continue
 [ "${root%%:*}" = "nbd" ] || return
@@ -37,6 +45,13 @@ if [ -z "$nbdfstype" ]; then
     nbdfstype=auto
 fi
 
+incol2 /proc/devices nbd || modprobe nbd ||
+    die "NBD root requested but no kernel support for NBD client"
+
+[ -z "$nbdserver" ] && die "NBD root configuration missing server"
+[ -z "$nbdport" ] && die "NBD root configuration missing port"
+[ -n "$check_only" ] && exit 0
+
 # look through the NBD options and pull out the ones that need to
 # go before the host etc. Append a ',' so we know we terminate the loop
 nbdopts=${nbdopts},
diff --git a/modules.d/95nbd/parse-nbdroot.sh b/modules.d/95nbd/parse-nbdroot.sh
index 4e0302a..54085f5 100755
--- a/modules.d/95nbd/parse-nbdroot.sh
+++ b/modules.d/95nbd/parse-nbdroot.sh
@@ -9,19 +9,6 @@
 # root= takes precedence over netroot= if root=nbd[...]
 #
 
-# Sadly there's no easy way to split ':' separated lines into variables
-netroot_to_var() {
-    local v=${1}:
-    set --
-    while [ -n "$v" ]; do
-        set -- "$@" "${v%%:*}"
-        v=${v#*:}
-    done
-
-    unset server port 
-    server=$2; port=$3;
-}
-
 # Don't continue if root is ok
 [ -n "$rootok" ] && return
 
@@ -33,24 +20,15 @@ netroot_to_var() {
 if [ "${root%%:*}" = "nbd" ] ; then
     if [ -n "$netroot" ] ; then
 	warn "root takes precedence over netroot. Ignoring netroot"
-
     fi
     netroot=$root
 fi
 
-# If it's not nbd we don't continue
-[ "${netroot%%:*}" = "nbd" ] || return
-
 # Check required arguments
-netroot_to_var $netroot
-[ -z "$server" ] && die "Argument server for nbdroot is missing"
-[ -z "$port" ] && die "Argument port for nbdroot is missing"
+if [ "${netroot%%:*}" = "nbd" ]; then
+    /sbin/nbdroot check "$netroot" || exit 1
 
-# NBD actually supported?
-incol2 /proc/devices nbd || modprobe nbd || die "nbdroot requested but kernel/initrd does not support nbd"
-
-# Done, all good!
-rootok=1
-
-# Shut up init error check
-[ -z "$root" ] && root="nbd"
+    # Done, all good!
+    [ -z "$root" ] && root=netroot
+    rootok=1
+fi
diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
index 847ec60..8cbfbb8 100755
--- a/modules.d/95nfs/nfsroot
+++ b/modules.d/95nfs/nfsroot
@@ -1,43 +1,5 @@
 #!/bin/sh
 
-# Copy from parse-nfsroot.sh
-root_to_var() {
-    local v=${1}:
-    set --
-    while [ -n "$v" ]; do
-	set -- "$@" "${v%%:*}"
-	v=${v#*:}
-    done
-
-    unset nfs server path options
-
-    # Ugly: Can't -z test $path after the case, since it might be allowed
-    # to be empty for root=nfs
-    nfs=$1
-    case $# in
-    0|1);;
-    2)	path=$2;;
-    3)
-    # This is ultra ugly. But we can't decide in which position path
-    # sits without checking if the string starts with '/'
-    case $2 in
-	/*) path=$2; options=$3;;
-	*) server=$2; path=$3;;
-    esac
-    ;;
-    *)	server=$2; path=$3; options=$4;
-    esac
-    
-    # Does it really start with '/'?
-    [ -n "${path%%/*}" ] && path="error";
-    
-    #Fix kernel legacy style separating path and options with ','
-    if [ "$path" != "${path#*,}" ] ; then
-	options=${path#*,}
-	path=${path%%,*}
-    fi
-}
-
 . /lib/dracut-lib.sh
 
 PATH=$PATH:/sbin:/usr/sbin
@@ -48,39 +10,99 @@ if getarg rdnetdebug ; then
     set -x
 fi
 
+case "$1" in
+    check)	basic_check=1 ;;
+    checkdhcp)	full_check=1 ;;
+    mount)	;;
+    *)		die "$0 called with invalid command '$1'" ;;
+esac
+
 # root is in the form root=nfs[4]:[server:]path[:options], either from
 # cmdline or dhcp root-path
-root="$1"
-server_id=$2
-root_path=$3
-NEWROOT="$4"
-
-# Continue if nfs prefix
-case "${root%%:*}" in
-    nfs|nfs4);;
-    *) return;;
-esac
+root=$2
+server_id=$3
+root_path=$4
+NEWROOT=$5
+
+# First, parse out fields from the [net]root= field
+nfs=${root%%:*}; root=${root#*:}
+server=${root%%:*}; root=${root#*:}
+path=${root%%:*}
+options=${root#*:}
+
+# Handle alternate syntax of path,options
+if [ "$options" = "$path" -a "${root#*,}" != "$root" ]; then
+    path=${root%%,*}
+    options=${root#*,}
+fi
 
-root_to_var $root
+# Catch the case when no additional flags are set
+[ "$path" = "$options" ] && unset options
+
+# Translate the DHCP root-path option to a standard form, removing the
+# nfs[4]: prefix.
+#
+# Order matters in this statement, put comma separated option matches up front
+# to avoid false matching when multiple mount options given
+case "$root_path" in
+    ''|nfs:*)			# empty or dracut nfs:... syntax
+	root_path=${root_path#*:} ;;
+    nfs4:*)			# nfs4:... syntax
+	root_path=${root_path#*:}
+	nfs=nfs4 ;;
+    *:/*,*)			# server:/path,options
+	root_path=${root_path%%,*}:${root_path#*,} ;;
+    *:/*|*:/*:*)		# server:/path or server:/path:options
+	root_path=$root_path ;;
+    /*,*)			# /path,options
+	root_path=:${root_path%%,*}:${root_path#*,} ;;
+    /*)				# /path or /path:options
+	root_path=:${root_path} ;;
+    *) die "Unsupported NFS format in DHCP root-path option" ;;
+esac
 
-#Empty path means try dhcp root-path, this is ok here since parse-nfsroot.sh
-#already takes care of nfs:... formatted root-path
-[ -z "$path" ] && root_to_var $nfs:$root_path
+dhcp_root_server=${root_path%%:*}; root_path=${root_path#*:}
+dhcp_path=${root_path%%:*}
+dhcp_options=${root_path#*:}
+
+# Catch the case when no additional flags are set
+[ "$dhcp_path" = "$dhcp_options" ] && unset dhcp_options
+
+# Now, merge the configurations, preferring the command line arguments first
+[ -z "$server" ] && server=$dhcp_root_server
+[ -z "$server" ] && server=$server_id
+[ -z "$path" ] && path=$dhcp_path
+[ -z "$options" ] && options=$dhcp_options
+[ -z "$nfs" ] && nfs=nfs
+
+# Make sure we have support for this
+if ! incol2 /proc/filesystems $nfs; then
+    modprobe nfs
+    if ! incol2 /proc/filesystems $nfs; then
+	die "NFS root requested, but no kernel support"
+    fi
+fi
 
 #Empty path defaults to "/tftpboot/%s" only in nfsroot.txt legacy mode
 [ -z "$path" ] && [ "$(getarg root=)" = "/dev/nfs" ] && path="/tftpboot/%s"
 
-if [ -z "$server" ] ; then
-    for var in $server_id $root_path '' ; do
-	[ -n "$var" ] && server=$var && break;
-    done
+# Check that path start with a /, if we have one
+[ -n "$path" -a "${path#/}" = "$path" ] &&
+	die "NFS path argument must start with /"
 
-    # XXX This blindly assumes that if new_root_path has to used that 
-    # XXX it really can be used as server
-    server=${server%%:*}
+# If we're only doing the basic checks, just see if we need a server
+# Everything else can be filled in later by DHCP, so we need to wait
+if [ -n "$basic_check" ]; then
+    [ -z "$server" ] && exit 127
+    exit 0
 fi
 
+# Check for required arguments
 [ -z "$server" ] && die "Required parameter 'server' is missing"
+[ -z "$path" ] && die "NFS root requires a path"
+
+# If we're just validating our options, we're done
+[ -n "$full_check" ] && exit 0
 
 # Kernel replaces first %s with host name, and falls back to the ip address
 # if it isn't set. Only the first %s is substituted.
diff --git a/modules.d/95nfs/parse-nfsroot.sh b/modules.d/95nfs/parse-nfsroot.sh
index 23fc3be..cf508c9 100755
--- a/modules.d/95nfs/parse-nfsroot.sh
+++ b/modules.d/95nfs/parse-nfsroot.sh
@@ -1,134 +1,93 @@
 #!/bin/sh
 #
-# Preferred format:
+# Dracut syntax:
 #	root=nfs[4]:[server:]path[:options]
+#	root=dhcp root=nfs[4]:[server:]path[:options]
 #
-# This syntax can come from DHCP root-path as well.
+# Legacy DHCP syntax:
+#	root=dhcp root-path=[server:]/path[(:|,)options]
 #
-# Legacy format:
+# Legacy mkinitrd syntax:
+#	root=server:/path
+#
+# Legacy kernel syntax:
 #	root=/dev/nfs nfsroot=[server:]path[,options]
 #
-# In Legacy root=/dev/nfs mode, if the 'nfsroot' parameter is not given
+# In legacy kernel syntax mode, if the 'nfsroot' parameter is not given
 # on the command line or is empty, the dhcp root-path is used as 
 # [server:]path[:options] or the default "/tftpboot/%s" will be used.
 #
 # If server is unspecified it will be pulled from one of the following
 # sources, in order:
 #	static ip= option on kernel command line
-#	DHCP next-server option
-#	DHCP server-id option
 #       DHCP root-path option
+#	DHCP server-id option
 #
 # NFSv4 is only used if explicitly requested with nfs4: prefix, otherwise
 # NFSv3 is used.
 #
 
-# Sadly there's no easy way to split ':' separated lines into variables
-netroot_to_var() {
-    local v=${1}:
-    set --
-    while [ -n "$v" ]; do
-	set -- "$@" "${v%%:*}"
-	v=${v#*:}
-    done
-
-    unset nfs server path options
-
-    nfs=$1
-    # Ugly: Can't -z test #path after the case, since it might be allowed
-    # to be empty for root=nfs
-    case $# in
-    0|1);;
-    2)	path=${2:-error};;
-    3)
-    # This is ultra ugly. But we can't decide in which position path
-    # sits without checking if the string starts with '/'
-    case $2 in
-	/*) path=$2; options=$3;;
-	*) server=$2; path=${3:-error};;
-    esac
-    ;;
-    *)	server=$2; path=${3:-error}; options=$4;
-    esac
-    
-    # Does it really start with '/'?
-    [ -n "${path%%/*}" ] && path="error";
-    
-    #Fix kernel legacy style separating path and options with ','
-    if [ "$path" != "${path#*,}" ] ; then
-	options=${path#*,}
-	path=${path%%,*}
-    fi
-}
-
 #Don't continue if root is ok
 [ -n "$rootok" ] && return
 
 # This script is sourced, so root should be set. But let's be paranoid
 [ -z "$root" ] && root=$(getarg root=)
 [ -z "$netroot" ] && netroot=$(getarg netroot=)
-[ -z "$nfsroot" ] && nfsroot=$(getarg nfsroot=)
-
-# netroot= cmdline argument must be ignored, but must be used if
-# we're inside netroot to parse dhcp root-path
-if [ -n "$netroot" ] ; then
-    if [ "$netroot" = "$(getarg netroot=)" ] ; then
-        warn "Ignoring netroot argument for NFS"
-        netroot=$root
-    fi
-else
-    netroot=$root;
-fi 
 
-# LEGACY: nfsroot= is valid only if root=/dev/nfs
-if [ -n "$nfsroot" ] ; then
-    # @deprecated
-    warn "Argument nfsroot is deprecated and might be removed in a future release. See http://apps.sourceforge.net/trac/dracut/wiki/commandline for more information."
-    if [ "$(getarg root=)" != "/dev/nfs"  ]; then
-	die "Argument nfsroot only accepted for legacy root=/dev/nfs"
-    fi
-    netroot=nfs:$nfsroot;
-fi
-
-case "$netroot" in
-    /dev/nfs) netroot=nfs;;
-    /dev/*) unset netroot; return;;
-# LEGACY: root=<server-ip>:/<path
-    [0-9]*:/*|[0-9]*\.[0-9]*\.[0-9]*[!:]|/*)
-       netroot=nfs:$netroot;;
+# Disallow netroot=nfs[4]:...
+case "${netroot%%:*}" in
+    nfs|nfs4) die "netroot=nfs[4].... not allowed, use root=nfs[4]...." ;;
 esac
 
-# Continue if nfs
-case "${netroot%%:*}" in
-    nfs|nfs4|/dev/nfs);;
-    *) unset netroot; return;;
+# Convert mkinitrd's root=ip:/path format for NFS to our syntax
+# FIXME need to warn about deprecation?
+case "$root" in
+    [0-9]*:/*) root=nfs:$root ;;
 esac
 
-# Check required arguments
-netroot_to_var $netroot
-[ "$path" = "error" ] && die "Argument nfsroot must contain a valid path!"
+# Enforce legacy nfsroot only with root=/dev/nfs
+if getarg nfsroot && [ "$root" != "/dev/nfs" ]; then
+    die "Legacy nfsroot= can only be combined with root=/dev/nfs"
+fi
 
-# Set fstype, might help somewhere
-fstype=${nfs#/dev/}
+# Convert root=/dev/nfs nfsroot=[[server:]/path[,options]] to our syntax
+if [ "$root" = "/dev/nfs" ]; then
+    # @deprecated
+    warn "root=/dev/nfs support is deprecated and may be removed in a future release. See http://apps.sourceforge.net/trac/dracut/wiki/commandline for more information."
 
-# NFS actually supported? Some more uglyness here: nfs3 or nfs4 might not
-# be in the module...
-if ! incol2 /proc/filesystems $fstype ; then
-    modprobe nfs
-    incol2 /proc/filesystems $fstype || die "nfsroot type $fstype requested but kernel/initrd does not support nfs"
-fi
+    nfsroot=$(getarg nfsroot=)
+    nfsroot_ip=${nfsroot%%:*}; nfsroot=${nfsroot#*:}
+    nfsroot_path=${nfsroot%%,*}
+    nfsroot_opts=${nfsroot#*,}
 
-# Rewrite root so we don't have to parse this uglyness later on again
-netroot="$fstype:$server:$path:$options"
+    [ "$nfsroot_opts" = "$nfsroot_path" ] && unset nfsroot_opts
+    [ "$nfsroot_ip" = "$nfsroot_path" ] && unset nfsroot_ip
 
-# If we don't have a server, we need dhcp
-if [ -z "$server" ] ; then
-    DHCPORSERVER="1"
-fi;
+    root=nfs:$nfsroot_ip:$nfsroot_path:$nfsroot_opts
+    unset nfsroot
+fi
 
-# Done, all good!
-rootok=1
+# Move root=nfs[4]:... to netroot= for handling
+if [ "${root%%:*}" = "nfs" -o "${root%%:*}" = "nfs4" ]; then
+    if [ -n "$netroot" ]; then
+	warn "root takes precedence over netroot. Ignoring netroot"
+    fi
+    netroot=$root
+fi
 
-# Shut up init error check or make sure that block parser wont get 
-# confused by having /dev/nfs[4]
-root="$fstype"
+# Validate our arguments
+case "${netroot%%:*}" in
+    nfs|nfs4)
+	/sbin/nfsroot check "$netroot"
+	case "$?" in
+	    127) DHCPORSERVER="1" ;;
+       	    0)   ;;
+       	    *)   exit 1 ;;
+	esac
+
+	# We're good. Keep the root check in init happy and avoid
+	# confusing the block parser with /dev/nfs
+	rootok=1
+	root=nfs
+	;;
+esac
-- 
1.6.0.6

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [RFC ONLY 5/5] PROOF-OF-CONCEPT: wait for spanning tree timeout via arping
       [not found] ` <cover.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
                     ` (3 preceding siblings ...)
  2009-07-03 21:40   ` [RFC ONLY 4/5] netroot: move dracut syntax validation to root handlers David Dillow
@ 2009-07-03 21:40   ` David Dillow
       [not found]     ` <8cf3d034e4611fbe661d4585fbfd1a03a6fd094b.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
  4 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-03 21:40 UTC (permalink / raw)
  To: initramfs-u79uwXL29TY76Z2rM5mHXA; +Cc: David Dillow

When operating on a switch running a spanning tree, it can take up to
100 seconds for our port to start forwarding traffic after comming up.
Normally, this can be eaten by DHCP's request for an IP, but if we have
a static configuration or need to down the interface to change the MTU, we
will have to eat the timeout again.

Most of the network root protocols will timeout while waiting for traffic
to flow again in this case, so we use ARP pings to determine when the
link is available prior to trying the mount/disk login.
---
 modules.d/40network/dhclient-script |   11 ++++--
 modules.d/40network/ifup            |    8 +++-
 modules.d/40network/install         |    2 +-
 modules.d/40network/netroot         |   67 +++++++++++++++++++++++++++++------
 modules.d/95iscsi/iscsiroot         |   28 +++++++++-----
 modules.d/95nbd/nbdroot             |   12 +++++--
 modules.d/95nfs/nfsroot             |    9 +++--
 7 files changed, 104 insertions(+), 33 deletions(-)

diff --git a/modules.d/40network/dhclient-script b/modules.d/40network/dhclient-script
index 5b36a40..e440df9 100755
--- a/modules.d/40network/dhclient-script
+++ b/modules.d/40network/dhclient-script
@@ -13,15 +13,20 @@ setup_interface() {
 
     [ -f /tmp/net.$netif.override ] && . /tmp/net.$netif.override
 
+    echo mynet=$ip/$mask > /tmp/net.$netif.up
+
     if [ -n "$mtu" ] ; then
 	echo ip link set $netif down
 	echo ip link set $netif mtu $mtu
 	echo ip link set $netif up
-    fi > /tmp/net.$netif.up
+    fi >> /tmp/net.$netif.up
 
-    echo ip addr add $ip${mask:+/$mask} ${bcast:+broadcast $bcast} dev $netif >> /tmp/net.$netif.up
+    echo ip addr add $ip/$mask ${bcast:+broadcast $bcast} dev $netif >> /tmp/net.$netif.up
 
-    [ -n "$gw" ] && echo ip route add default via $gw dev $netif > /tmp/net.$netif.gw
+    if [ -n "$gw" ]; then
+	echo gw=$gw > /tmp/net.$netif.gw
+	echo ip route add default via $gw dev $netif >> /tmp/net.$netif.gw
+    fi
 
     [ -n "${search}${domain}" ] && echo search $search $domain > /tmp/net.$netif.resolv.conf
     if  [ -n "$namesrv" ] ; then
diff --git a/modules.d/40network/ifup b/modules.d/40network/ifup
index 89017bb..404d6d8 100755
--- a/modules.d/40network/ifup
+++ b/modules.d/40network/ifup
@@ -32,13 +32,17 @@ do_dhcp() {
 
 # Handle static ip configuration
 do_static() {
-{
+    {
+	echo mynet=$ip/$mask
 	echo ip link set $netif up 
 	echo ip addr flush dev $netif
 	echo ip addr add $ip/$mask dev $netif
     } > /tmp/net.$netif.up
 
-    [ -n "$gw" ] && echo ip route add default via $gw dev $netif > /tmp/net.$netif.gw
+    if [ -n "$gw" ]; then
+	 echo gw=$gw > /tmp/net.$netif.gw
+	 echo ip route add default via $gw dev $netif >> /tmp/net.$netif.gw
+    fi
     [ -n "$hostname" ] && echo hostname $hostname > /tmp/net.$netif.hostname
 
     echo online > /sys/class/net/$netif/uevent
diff --git a/modules.d/40network/install b/modules.d/40network/install
index 0b76cbd..20e8963 100755
--- a/modules.d/40network/install
+++ b/modules.d/40network/install
@@ -1,5 +1,5 @@
 #!/bin/bash
-dracut_install ip dhclient hostname
+dracut_install ip dhclient hostname arping date
 # Include wired net drivers, excluding wireless
 for modname in $(find "/lib/modules/$kernel/kernel/drivers" -name '*.ko'); do
   if nm -uPA $modname | grep -q eth_type_trans; then
diff --git a/modules.d/40network/netroot b/modules.d/40network/netroot
index 2cf51fa..58bbd2e 100755
--- a/modules.d/40network/netroot
+++ b/modules.d/40network/netroot
@@ -1,5 +1,29 @@
 #!/bin/sh
 
+apply_mask() {
+    local ip=$1
+    local mask=$2
+    local out i
+
+    for i in 1 2 3 4; do
+	out=$out.$(( ${ip%%.*} & ${mask%%.*} ))
+	ip=${ip#*.}
+	mask=${mask#*.}
+    done
+    echo ${out#.}
+}
+
+is_local() {
+    local server=$1
+    local mynet=$2
+    local mask net
+
+    mask=${mynet#*/}
+    mynet=$(apply_mask ${mynet%/*} $mask)
+    net=$(apply_mask $server $mask)
+    [ "$net" = "$mynet" ]
+}
+
 PATH=$PATH:/sbin:/usr/sbin
 
 . /lib/dracut-lib.sh
@@ -75,8 +99,10 @@ if [ -z "$netroot" ] || [ ! -e "$handler" ] ; then
     die "No handler for netroot type '$netroot'"
 fi
 
-# Now that we have DHCP information, we can fully validate netroot
-$handler checkdhcp $netroot "$server_id" "$new_root_path" || exit 1
+# Now that we have DHCP information, we can get our server
+$handler server $netroot "$server_id" "$new_root_path" || exit 1
+[ -s /tmp/server ] || die "Bug in $handler: did not create /tmp/server"
+read target < /tmp/server
 
 # We're here, so we can assume that upping interfaces is now ok
 [ -z "$IFACES" ] && IFACES="$netif"
@@ -88,8 +114,21 @@ done
 [ -e /tmp/net.$netif.hostname ]    && . /tmp/net.$netif.hostname
 [ -e /tmp/net.$netif.resolv.conf ] && cp -f /tmp/net.$netif.resolv.conf /etc/resolv.conf
 
+# Wait for traffic to be passable before we continue
+is_local $target $mynet || target=$gw
+
+# FIXME make 120 configurable
+TIMEOUT=$(( $(date +%s) + 120 ))
+while [ -z "$proceed" -a $(date +%s) -lt $TIMEOUT ]; do
+    for iface in $IFACES; do
+	arping -q -f -c 1 -I $iface $target && proceed=1 && break
+    done
+done
+
 # Run the handler to mount/login into the root device
-if $handler mount "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
+if [ -n "$proceed" ] &&
+	$handler mount "$netroot" "$server_id" "$new_root_path" $NEWROOT;
+then
     # Network rootfs mount successful
     for iface in $IFACES ; do
 	[ -f /tmp/dhclient.$iface.lease ] &&    cp /tmp/dhclient.$iface.lease    /tmp/net.$iface.lease
@@ -98,14 +137,20 @@ if $handler mount "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
 
     # Save used netif for later use
     [ ! -f /tmp/net.ifaces ] && echo $netif > /tmp/net.ifaces
-else 
+    exit 0
+fi
+
+if [ -n "$proceed" ]; then
     warn "Mounting root via '$netif' failed"
-    # If we're trying with multiple interfaces, put that one down.
-    # ip down/flush ensures that routeing info goes away as well
-    if [ -z "$BOOTDEV" ] ; then
-	ip link set $netif down
-	ip addr flush dev $netif
-	echo "#empty" > /etc/resolv.conf
-    fi
+else
+    warn "Unable to ARP ping $target via $netif"
+fi
+
+# If we're trying with multiple interfaces, put that one down.
+# ip down/flush ensures that routeing info goes away as well
+if [ -z "$BOOTDEV" ] ; then
+    ip link set $netif down
+    ip addr flush dev $netif
+    echo "#empty" > /etc/resolv.conf
 fi
 exit 0
diff --git a/modules.d/95iscsi/iscsiroot b/modules.d/95iscsi/iscsiroot
index d79b663..7f2efa2 100755
--- a/modules.d/95iscsi/iscsiroot
+++ b/modules.d/95iscsi/iscsiroot
@@ -17,9 +17,10 @@ if getarg rdnetdebug; then
 fi
 
 case "$1" in
-    check|checkdhcp)	check_only=1 ;;
-    mount)		;;
-    *)			die "$0 called with invalid command '$1'" ;;
+    check)	check_only=1 ;;
+    server)	server_only=1 ;;
+    mount)	;;
+    *)		die "$0 called with invalid command '$1'" ;;
 esac
 
 # root is in the form
@@ -55,13 +56,6 @@ if [ ! -e /sys/devices/virtual/iscsi_transport ]; then
     fi
 fi
 
-[ -n "$check_only" ] && exit 0
-
-if getarg iscsi_firmware ; then
-	iscsistart -b
-	exit 0
-fi
-
 # override conf settings by command line options
 arg=$(getarg iscsi_initiator)
 [ -n "$arg" ] && iscsi_initiator=$arg
@@ -100,6 +94,20 @@ iscsi_lun=$1; shift
 iscsi_target_name=$*
 IFS="$OLDIFS"
 
+[ -n "$check_only" ] && exit 0
+
+# FIXME need support for service discovery if no server given, or
+# parsing the firmware tables for an IP if iscsi_firmware is in effect
+if [ -n "$server_only" ]; then
+    echo $iscsi_target_ip > /tmp/server
+    exit 0
+fi
+
+if getarg iscsi_firmware ; then
+	iscsistart -b
+	exit 0
+fi
+
 # XXX is this needed?
 getarg ro && iscsirw=ro
 getarg rw && iscsirw=rw
diff --git a/modules.d/95nbd/nbdroot b/modules.d/95nbd/nbdroot
index 55c1b88..9450b84 100755
--- a/modules.d/95nbd/nbdroot
+++ b/modules.d/95nbd/nbdroot
@@ -11,9 +11,10 @@ if getarg rdnetdebug; then
 fi
 
 case "$1" in
-    check|checkdhcp)	check_only=1 ;;
-    mount)		;;
-    *)			die "$0 called with invalid command '$1'" ;;
+    check)	check_only=1 ;;
+    server)	server_only=1 ;;
+    mount)	;;
+    *)		die "$0 called with invalid command '$1'" ;;
 esac
 
 # root is in the form root=nbd:srv:port[:fstype[:rootflags[:nbdopts]]]
@@ -52,6 +53,11 @@ incol2 /proc/devices nbd || modprobe nbd ||
 [ -z "$nbdport" ] && die "NBD root configuration missing port"
 [ -n "$check_only" ] && exit 0
 
+if [ -n "$server_only" ]; then
+    echo $nbdserver > /tmp/server
+    exit 0
+fi
+
 # look through the NBD options and pull out the ones that need to
 # go before the host etc. Append a ',' so we know we terminate the loop
 nbdopts=${nbdopts},
diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
index 8cbfbb8..acfd6b5 100755
--- a/modules.d/95nfs/nfsroot
+++ b/modules.d/95nfs/nfsroot
@@ -12,7 +12,7 @@ fi
 
 case "$1" in
     check)	basic_check=1 ;;
-    checkdhcp)	full_check=1 ;;
+    server)	server_only=1 ;;
     mount)	;;
     *)		die "$0 called with invalid command '$1'" ;;
 esac
@@ -101,8 +101,11 @@ fi
 [ -z "$server" ] && die "Required parameter 'server' is missing"
 [ -z "$path" ] && die "NFS root requires a path"
 
-# If we're just validating our options, we're done
-[ -n "$full_check" ] && exit 0
+# If we're just looking for the server...
+if [ -n "$server_only" ]; then
+    echo $server > /tmp/server
+    exit 0
+fi
 
 # Kernel replaces first %s with host name, and falls back to the ip address
 # if it isn't set. Only the first %s is substituted.
-- 
1.6.0.6

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 1/5] netroot: remove unused hook
       [not found]     ` <bd4d79bc048ad3944156f86eafb62cf4bc29b1f9.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
@ 2009-07-06 12:56       ` Seewer Philippe
  0 siblings, 0 replies; 17+ messages in thread
From: Seewer Philippe @ 2009-07-06 12:56 UTC (permalink / raw)
  To: David Dillow; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

David Dillow wrote:
> The netroot hook is no longer used.
> ---
> This is a left over from before Philippe's multinic work.

I intentionally left the hook in there to give other modules the option 
   to add a hook to between successful and completed interface 
configuration and actual mount/login calls.

Most of the stuff that I was thinking about that could go in there can 
now go inside pre-mount. But if someone needs port authentication or 
similar stuff, this needs to run between interface configuration and 
mount/login. I think the hook should stay.

> 
>  dracut                      |    2 +-
>  modules.d/40network/netroot |    3 ---
>  2 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/dracut b/dracut
> index f10af83..c71cc63 100755
> --- a/dracut
> +++ b/dracut
> @@ -102,7 +102,7 @@ if [[ -f $outfile && ! $force ]]; then
>      exit 1
>  fi
>  
> -hookdirs="cmdline pre-udev pre-trigger netroot pre-mount pre-pivot mount emergency"
> +hookdirs="cmdline pre-udev pre-trigger pre-mount pre-pivot mount emergency"
>  
>  readonly initdir=$(mktemp -d -t initramfs.XXXXXX)
>  trap 'rm -rf "$initdir"' 0 # clean up after ourselves no matter how we die.
> diff --git a/modules.d/40network/netroot b/modules.d/40network/netroot
> index f719d93..645bbba 100755
> --- a/modules.d/40network/netroot
> +++ b/modules.d/40network/netroot
> @@ -92,9 +92,6 @@ done
>  [ -e /tmp/net.$netif.hostname ]    && . /tmp/net.$netif.hostname
>  [ -e /tmp/net.$netif.resolv.conf ] && cp -f /tmp/net.$netif.resolv.conf /etc/resolv.conf
>  
> -# Source netroot hooks before we start the handler
> -source_all netroot
> -
>  # Run the handler; don't store the root, it may change from device to device
>  # XXX other variables to export?
>  if $handler $netif $netroot $NEWROOT; then
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 2/5] netroot: remove unhelpful argument checks
       [not found]     ` <0535c73bd25fdcaa9bf4de0bc4b041b74d11e547.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
@ 2009-07-06 13:00       ` Seewer Philippe
  0 siblings, 0 replies; 17+ messages in thread
From: Seewer Philippe @ 2009-07-06 13:00 UTC (permalink / raw)
  To: David Dillow; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

David Dillow wrote:
> The argument checks for the network root handlers are not very useful, as
> there is no error message; things just fail to work. Since this should only
> ever be a problem when developing dracut, it is not too unfriendly to expect
> developers to add rdnetdebug to the command line when working on this area.
> 
> Remove them for now, as the arguments will be going through some flux and
> we can add them back later with more helpful messages if it is desired.

I admit that argument checks without error messages aren't that good an 
idea. But as long as we have scripts that need arguments these should be 
checked. I've wasted too many hours of debugging because people weren't 
paranoid enough about their arguments.

Removeing checks for arguments that are no longer necessary is ok, but 
please change the others to output an error message.

> ---
>  modules.d/95iscsi/iscsiroot |   10 ----------
>  modules.d/95nbd/nbdroot     |    9 ---------
>  modules.d/95nfs/nfsroot     |    9 ---------
>  3 files changed, 0 insertions(+), 28 deletions(-)
> 
> diff --git a/modules.d/95iscsi/iscsiroot b/modules.d/95iscsi/iscsiroot
> index c66f9d2..2b041cc 100755
> --- a/modules.d/95iscsi/iscsiroot
> +++ b/modules.d/95iscsi/iscsiroot
> @@ -16,16 +16,6 @@ if getarg rdnetdebug; then
>      set -x
>  fi
>  
> -# Huh? Empty $1?
> -[ -z "$1" ] && exit 1
> -
> -# Huh? Empty $2?
> -[ -z "$2" ] && exit 1
> -
> -# Huh? Empty $3? This isn't really necessary, since NEWROOT isn't 
> -# used here. But let's be consistent
> -[ -z "$3" ] && exit 1
> -
>  # root is in the form root=iscsi:[<servername>]:[<protocol>]:[<port>]:[<LUN>]:<targetname>
>  netif="$1"
>  root="$2"
> diff --git a/modules.d/95nbd/nbdroot b/modules.d/95nbd/nbdroot
> index 90f6ef8..c7f24e8 100755
> --- a/modules.d/95nbd/nbdroot
> +++ b/modules.d/95nbd/nbdroot
> @@ -10,15 +10,6 @@ if getarg rdnetdebug; then
>      set -x
>  fi
>  
> -# Huh? Empty $1?
> -[ -z "$1" ] && exit 1
> -
> -# Huh? Empty $2?
> -[ -z "$2" ] && exit 1
> -
> -# Huh? Empty $3?
> -[ -z "$3" ] && exit 1
> -
>  # root is in the form root=nbd:srv:port[:fstype[:rootflags[:nbdopts]]]
>  netif="$1"
>  root="$2"
> diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
> index 0da8ee3..f005378 100755
> --- a/modules.d/95nfs/nfsroot
> +++ b/modules.d/95nfs/nfsroot
> @@ -48,15 +48,6 @@ if getarg rdnetdebug ; then
>      set -x
>  fi
>  
> -# Huh? Empty $1?
> -[ -z "$1" ] && exit 1
> -
> -# Huh? Empty $2?
> -[ -z "$2" ] && exit 1
> -
> -# Huh? Empty $3?
> -[ -z "$3" ] && exit 1
> -
>  # root is in the form root=nfs[4]:[server:]path[:options], either from
>  # cmdline or dhcp root-path
>  netif="$1"
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 3/5] netroot: remove netif from handler invocation
       [not found]     ` <44494af931ad839908cb4542f7c35ae59a4ef65e.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
@ 2009-07-06 13:56       ` Seewer Philippe
       [not found]         ` <4A520291.7010203-omB+W0Dpw2o@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Seewer Philippe @ 2009-07-06 13:56 UTC (permalink / raw)
  To: David Dillow; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

David Dillow wrote:
> Now that netroot is serialized and only one interface can be configured
> at a time, netif is almost completely unneeded by the netroot handlers.
> The remaining hold out is nfsroot, which uses it to get the command line
> overrides and the DHCP root information.

Loosing arguments is always good.

The only thing I don't like about this is that netroot needs to
implement a few specialities just for NFS. I'd prefer netroot to be
(mount-)protocol independent. 

On the other hand, I like the idea that network root handlers shouldn't
care a thing about network configuration and options.

Question: Is it possible that other netroot handlers we haven't
implemented or thought about might need to know about interfaces?

Discussion: Instead of implementing the dhcp options server failover
just for nfs, why not extend this to all protocols?

> 
> By adding that information to the calling convention, we can get rid
> of netif, and prepare for the next steps of moving parsing and validation
> into the handlers themselves.
> ---
>  modules.d/40network/netroot |   16 ++++++++++------
>  modules.d/95iscsi/iscsiroot |    7 +++----
>  modules.d/95nbd/nbdroot     |    9 ++++-----
>  modules.d/95nfs/nfsroot     |   20 ++++++++------------
>  4 files changed, 25 insertions(+), 27 deletions(-)
> 
> diff --git a/modules.d/40network/netroot b/modules.d/40network/netroot
> index 645bbba..c9ebf4c 100755
> --- a/modules.d/40network/netroot
> +++ b/modules.d/40network/netroot
> @@ -44,15 +44,20 @@ done
>  netif=$1
>  [ -e "/tmp/net.bootdev" ] && read netif < /tmp/net.bootdev
>  
> +# Load command line overrides to get the server override
> +[ -e /tmp/net.$netif.override ] && . /tmp/net.$netif.override
> +[ -n "$srv" ] && server_id=$srv
> +
> +# Load dhcp options and get the first fallback to the server
> +[ -e /tmp/dhclient.$netif.dhcpopts ] && . /tmp/dhclient.$netif.dhcpopts
> +[ -z "$server_id" ] && server_id=$new_dhcp_server_identifier
> +
>  # Figure out the handler for root=dhcp by recalling all netroot cmdline 
>  # handlers
>  if [ "$netroot" = "dhcp" ] ; then
>      # Unset root so we can check later
>      unset root
>  
> -    # Load dhcp options
> -    [ -e /tmp/dhclient.$netif.dhcpopts ] && . /tmp/dhclient.$netif.dhcpopts
> -
>      # If we have a specific bootdev with no dhcpoptions or empty root-path, 
>      # we die. Otherwise we just warn
>      if [ -z "$new_root_path" ] ; then
> @@ -92,9 +97,8 @@ done
>  [ -e /tmp/net.$netif.hostname ]    && . /tmp/net.$netif.hostname
>  [ -e /tmp/net.$netif.resolv.conf ] && cp -f /tmp/net.$netif.resolv.conf /etc/resolv.conf
>  
> -# Run the handler; don't store the root, it may change from device to device
> -# XXX other variables to export?
> -if $handler $netif $netroot $NEWROOT; then
> +# Run the handler to mount/login into the root device
> +if $handler "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
>      # Network rootfs mount successful
>      for iface in $IFACES ; do
>  	[ -f /tmp/dhclient.$iface.lease ] &&    cp /tmp/dhclient.$iface.lease    /tmp/net.$iface.lease
> diff --git a/modules.d/95iscsi/iscsiroot b/modules.d/95iscsi/iscsiroot
> index 2b041cc..923141c 100755
> --- a/modules.d/95iscsi/iscsiroot
> +++ b/modules.d/95iscsi/iscsiroot
> @@ -11,14 +11,13 @@
>  PATH=$PATH:/sbin:/usr/sbin
>  
>  if getarg rdnetdebug; then
> -    exec > /tmp/iscsiroot.$1.$$.out
> -    exec 2>> /tmp/iscsiroot.$1.$$.out
> +    exec > /tmp/iscsiroot.$$.out
> +    exec 2>> /tmp/iscsiroot.$$.out
>      set -x
>  fi

Maybe this is just me nagging around, but do the changes above have
anything to do with removing netif?

>  
>  # root is in the form root=iscsi:[<servername>]:[<protocol>]:[<port>]:[<LUN>]:<targetname>
> -netif="$1"
> -root="$2"
> +root="$1"
>  
>  # read static conf settings
>  for conf in conf/conf.d/*; do
> diff --git a/modules.d/95nbd/nbdroot b/modules.d/95nbd/nbdroot
> index c7f24e8..f76211c 100755
> --- a/modules.d/95nbd/nbdroot
> +++ b/modules.d/95nbd/nbdroot
> @@ -5,15 +5,14 @@
>  PATH=$PATH:/sbin:/usr/sbin
>  
>  if getarg rdnetdebug; then
> -    exec > /tmp/nbdroot.$1.$$.out
> -    exec 2>> /tmp/nbdroot.$1.$$.out
> +    exec > /tmp/nbdroot.$$.out
> +    exec 2>> /tmp/nbdroot.$$.out
>      set -x
>  fi

Same here, is this netif related?

>  
>  # root is in the form root=nbd:srv:port[:fstype[:rootflags[:nbdopts]]]
> -netif="$1"
> -root="$2"
> -NEWROOT="$3"
> +root="$1"
> +NEWROOT="$4"
>  
>  # If it's not nbd we don't continue
>  [ "${root%%:*}" = "nbd" ] || return
> diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
> index f005378..847ec60 100755
> --- a/modules.d/95nfs/nfsroot
> +++ b/modules.d/95nfs/nfsroot
> @@ -43,16 +43,17 @@ root_to_var() {
>  PATH=$PATH:/sbin:/usr/sbin
>  
>  if getarg rdnetdebug ; then 
> -    exec > /tmp/nfsroot.$1.$$.out
> -    exec 2>> /tmp/nfsroot.$1.$$.out
> +    exec > /tmp/nfsroot.$$.out
> +    exec 2>> /tmp/nfsroot.$$.out
>      set -x
>  fi

And another one here. Is this Netif removal related?

>  
>  # root is in the form root=nfs[4]:[server:]path[:options], either from
>  # cmdline or dhcp root-path
> -netif="$1"
> -root="$2"
> -NEWROOT="$3"
> +root="$1"
> +server_id=$2
> +root_path=$3
> +NEWROOT="$4"
>  
>  # Continue if nfs prefix
>  case "${root%%:*}" in
> @@ -62,20 +63,15 @@ esac
>  
>  root_to_var $root
>  
> -#Load other data that might provide info
> -[ -f /tmp/net.$netif.override ] && . /tmp/net.$netif.override
> -[ -f /tmp/dhclient.$netif.dhcpopts ] && . /tmp/dhclient.$netif.dhcpopts
> -
>  #Empty path means try dhcp root-path, this is ok here since parse-nfsroot.sh
>  #already takes care of nfs:... formatted root-path
> -[ -z "$path" ] && root_to_var $nfs:$new_root_path
> +[ -z "$path" ] && root_to_var $nfs:$root_path
>  
>  #Empty path defaults to "/tftpboot/%s" only in nfsroot.txt legacy mode
>  [ -z "$path" ] && [ "$(getarg root=)" = "/dev/nfs" ] && path="/tftpboot/%s"
>  
>  if [ -z "$server" ] ; then
> -    # XXX new_dhcp_next_server is unconfirmed this is an assumption
> -    for var in $srv $new_dhcp_server_identifier $new_dhcp_next_server $new_root_path '' ; do
> +    for var in $server_id $root_path '' ; do
>  	[ -n "$var" ] && server=$var && break;
>      done
>  
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 3/5] netroot: remove netif from handler invocation
       [not found]         ` <4A520291.7010203-omB+W0Dpw2o@public.gmane.org>
@ 2009-07-06 14:43           ` David Dillow
       [not found]             ` <1246891382.24010.6.camel-FqX9LgGZnHWDB2HL1qBt2PIbXMQ5te18@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-06 14:43 UTC (permalink / raw)
  To: Seewer Philippe; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

On Mon, 2009-07-06 at 15:56 +0200, Seewer Philippe wrote:
> David Dillow wrote:
> > Now that netroot is serialized and only one interface can be configured
> > at a time, netif is almost completely unneeded by the netroot handlers.
> > The remaining hold out is nfsroot, which uses it to get the command line
> > overrides and the DHCP root information.
> 
> Loosing arguments is always good.
> 
> The only thing I don't like about this is that netroot needs to
> implement a few specialities just for NFS. I'd prefer netroot to be
> (mount-)protocol independent.

Well, it still is protocol independent, it just gives the handlers the
DHCP info instead of having them need to go find it themselves.

> On the other hand, I like the idea that network root handlers shouldn't
> care a thing about network configuration and options.
> 
> Question: Is it possible that other netroot handlers we haven't
> implemented or thought about might need to know about interfaces?

FCoE support comes to mind... had not thought about it too much, we need
to see what it really wants.

> Discussion: Instead of implementing the dhcp options server failover
> just for nfs, why not extend this to all protocols?

This does implement it for all protocols, if they choose to use it.

> > --- a/modules.d/95iscsi/iscsiroot
> > +++ b/modules.d/95iscsi/iscsiroot
> > @@ -11,14 +11,13 @@
> >  PATH=$PATH:/sbin:/usr/sbin
> >  
> >  if getarg rdnetdebug; then
> > -    exec > /tmp/iscsiroot.$1.$$.out
> > -    exec 2>> /tmp/iscsiroot.$1.$$.out
> > +    exec > /tmp/iscsiroot.$$.out
> > +    exec 2>> /tmp/iscsiroot.$$.out
> >      set -x
> >  fi
> 
> Maybe this is just me nagging around, but do the changes above have
> anything to do with removing netif?

$1 used to be $netif, so we'd get /tmp/iscsiroot.eth0.1234.out. Now we'd
get /tmp/iscsiroot.root=iscsi:..:blah.1234.out instead. Hence the
removal of $1.

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 3/5] netroot: remove netif from handler invocation
       [not found]             ` <1246891382.24010.6.camel-FqX9LgGZnHWDB2HL1qBt2PIbXMQ5te18@public.gmane.org>
@ 2009-07-06 15:02               ` Seewer Philippe
       [not found]                 ` <4A521208.2050605-omB+W0Dpw2o@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Seewer Philippe @ 2009-07-06 15:02 UTC (permalink / raw)
  To: David Dillow; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

David Dillow wrote:
> On Mon, 2009-07-06 at 15:56 +0200, Seewer Philippe wrote:
>> David Dillow wrote:
>>> Now that netroot is serialized and only one interface can be configured
>>> at a time, netif is almost completely unneeded by the netroot handlers.
>>> The remaining hold out is nfsroot, which uses it to get the command line
>>> overrides and the DHCP root information.
>> Loosing arguments is always good.
>>
>> The only thing I don't like about this is that netroot needs to
>> implement a few specialities just for NFS. I'd prefer netroot to be
>> (mount-)protocol independent.
> 
> Well, it still is protocol independent, it just gives the handlers the
> DHCP info instead of having them need to go find it themselves.
> 
>> On the other hand, I like the idea that network root handlers shouldn't
>> care a thing about network configuration and options.
>>
>> Question: Is it possible that other netroot handlers we haven't
>> implemented or thought about might need to know about interfaces?
> 
> FCoE support comes to mind... had not thought about it too much, we need
> to see what it really wants.
> 
>> Discussion: Instead of implementing the dhcp options server failover
>> just for nfs, why not extend this to all protocols?
> 
> This does implement it for all protocols, if they choose to use it.

Yes, that is correct. I meant it more like, why should only nfs choose 
to use it? We could insert the server into the root argument for each 
handler inside netroot instead of just "delegating" it.

>>> --- a/modules.d/95iscsi/iscsiroot
>>> +++ b/modules.d/95iscsi/iscsiroot
>>> @@ -11,14 +11,13 @@
>>>  PATH=$PATH:/sbin:/usr/sbin
>>>  
>>>  if getarg rdnetdebug; then
>>> -    exec > /tmp/iscsiroot.$1.$$.out
>>> -    exec 2>> /tmp/iscsiroot.$1.$$.out
>>> +    exec > /tmp/iscsiroot.$$.out
>>> +    exec 2>> /tmp/iscsiroot.$$.out
>>>      set -x
>>>  fi
>> Maybe this is just me nagging around, but do the changes above have
>> anything to do with removing netif?
> 
> $1 used to be $netif, so we'd get /tmp/iscsiroot.eth0.1234.out. Now we'd
> get /tmp/iscsiroot.root=iscsi:..:blah.1234.out instead. Hence the
> removal of $1.

Oh, didn't realize that. Thanks for claryfing.

> 
> --
> To unsubscribe from this list: send the line "unsubscribe initramfs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 3/5] netroot: remove netif from handler invocation
       [not found]                 ` <4A521208.2050605-omB+W0Dpw2o@public.gmane.org>
@ 2009-07-06 16:06                   ` David Dillow
       [not found]                     ` <1246896365.24010.14.camel-FqX9LgGZnHWDB2HL1qBt2PIbXMQ5te18@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: David Dillow @ 2009-07-06 16:06 UTC (permalink / raw)
  To: Seewer Philippe; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

On Mon, 2009-07-06 at 17:02 +0200, Seewer Philippe wrote:
> David Dillow wrote:
> > On Mon, 2009-07-06 at 15:56 +0200, Seewer Philippe wrote:
> >> Discussion: Instead of implementing the dhcp options server failover
> >> just for nfs, why not extend this to all protocols?
> > 
> > This does implement it for all protocols, if they choose to use it.
> 
> Yes, that is correct. I meant it more like, why should only nfs choose 
> to use it? We could insert the server into the root argument for each 
> handler inside netroot instead of just "delegating" it.

Sorry, I misunderstood.

I like the concept of other protocols using server_id as a fall back
when no server IP is given in their arguments (ie nbd::9000 would use
port 9000 on the DHCP server) as it makes it easier to share configs
among DHCP servers.

I suspect others will find it confusing, though.


> > $1 used to be $netif, so we'd get /tmp/iscsiroot.eth0.1234.out. Now we'd
> > get /tmp/iscsiroot.root=iscsi:..:blah.1234.out instead. Hence the
> > removal of $1.
> 
> Oh, didn't realize that. Thanks for claryfing.

If this series looks to be worth it, I'll try to do better on the commit
message regarding this point.
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 4/5] netroot: move dracut syntax validation to root handlers
       [not found]     ` <d1aed501ae771d769efcd370f47184f12b9800d6.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
@ 2009-07-07  7:47       ` Seewer Philippe
       [not found]         ` <4A52FD93.3000406-omB+W0Dpw2o@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Seewer Philippe @ 2009-07-07  7:47 UTC (permalink / raw)
  To: David Dillow; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

David Dillow wrote:
> This reduces the argument parsers in the cmdline hook to giving warnings
> about usage and translating legacy syntaxes to dracut's syntax. The root
> handlers themselves become responsible for validating the dracut syntax.
> This simplifies the cmdline parsers and centralizes the syntax parsers,
> which keeps them from multiplying as new functionality is added.

I don't think that centralizing code is that important. Dracut is
module based after all and the central point to find code is the module
directory. But reducing complexiting and eliminating duplicate code is
always good. And I like the approach of separating legacy cmdline
parseing and parsing dracut-style syntax.

But there's a two things about this approach that bug me:
- The netroot handlers become some sort of multi-call utilities, doing
  possibly entirely different things depending on arguments. From a
  functional point of view netroot handlers should only care about
  the actual mount/login.
- Arguments are constantly reparsed. This is unnecessary but applies to
  the current implementation as well

We need to refactor the netroot handlers, that is clear. Because only 
the netroot modules can give us a server IP for STP arping. And
second, you are correct that we should get rid of duplicate code in
the current implementation.

What hit me only after really reviewing your code is that the main reason
for duplicate code is that we are using the netroot argument as container 
for all parameters a netroot module needs. Hence the constant reparsing 
and splitting of that argument. 

In an object oriented language, I'd probably define an abstract
superclass which would define methods for the external interface and
concrete Implementations would have to provide. How the actual parameters
are stored internally I couldn't care less. While this isn't really
possible with shell code, a similar approach might simplify things 
even more.

Suggestion: We don't use the netroot argument inside netroot. Instead, the
different parsers should write out /tmp/netroot.info in a similar format
to net.ethX.override or dhclient.ethX.dhcpopts. Netroot modules would
have to provide three files: Legacy cmdline parser (optional),
dracut-syntax parser (for netroot= and dhcp root-path), mount/login handler.

This would work like this:
cmdline step 1: All legacy parsers run first, validating if necessary
                and/or reformat arguments into netroot= if possible
cmdline step 2: call proto-validate.sh which parses and validates
                dracut-syntax
==> After this step, if all parameters are correct and root!=dhcp,
    /tmp/netroot.info should be available.

netroot step 1: If root=dhcp do the same as cmdline step 2
==> After this step and if all parameters are correct /tmp/netroot.info
    should be available.

netroot step 2: Source /tmp/netroot.info and use "$server" for STP arping.
                If it's empty, issue a warning, use dhcp server and rewrite
                the file with the new server=.
netroot step 3: Just call the mount/login handler without any arguments.
                Everything it needs is in /tmp/netroot.info

What do you think?

Regards,
Philippe
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 5/5] PROOF-OF-CONCEPT: wait for spanning tree timeout via arping
       [not found]     ` <8cf3d034e4611fbe661d4585fbfd1a03a6fd094b.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
@ 2009-07-07  8:08       ` Seewer Philippe
       [not found]         ` <4A530297.1020906-omB+W0Dpw2o@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Seewer Philippe @ 2009-07-07  8:08 UTC (permalink / raw)
  To: David Dillow; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

David Dillow wrote:
> When operating on a switch running a spanning tree, it can take up to
> 100 seconds for our port to start forwarding traffic after comming up.
> Normally, this can be eaten by DHCP's request for an IP, but if we have
> a static configuration or need to down the interface to change the MTU, we
> will have to eat the timeout again.
> 
> Most of the network root protocols will timeout while waiting for traffic
> to flow again in this case, so we use ARP pings to determine when the
> link is available prior to trying the mount/disk login.

For the record: I've had a (much simpler, but not as generic as this
one) arping solution active in our production environment for a week and
the few cases that actually still use slow STP are solved.

So yes, this works.

(Some comments below)

> ---
>  modules.d/40network/dhclient-script |   11 ++++--
>  modules.d/40network/ifup            |    8 +++-
>  modules.d/40network/install         |    2 +-
>  modules.d/40network/netroot         |   67 +++++++++++++++++++++++++++++------
>  modules.d/95iscsi/iscsiroot         |   28 +++++++++-----
>  modules.d/95nbd/nbdroot             |   12 +++++--
>  modules.d/95nfs/nfsroot             |    9 +++--
>  7 files changed, 104 insertions(+), 33 deletions(-)
> 
> diff --git a/modules.d/40network/dhclient-script b/modules.d/40network/dhclient-script
> index 5b36a40..e440df9 100755
> --- a/modules.d/40network/dhclient-script
> +++ b/modules.d/40network/dhclient-script
> @@ -13,15 +13,20 @@ setup_interface() {
>  
>      [ -f /tmp/net.$netif.override ] && . /tmp/net.$netif.override
>  
> +    echo mynet=$ip/$mask > /tmp/net.$netif.up
> +

Please don't do this. $ip/$mask is in .override so we don't need
more variables.

>      if [ -n "$mtu" ] ; then
>  	echo ip link set $netif down
>  	echo ip link set $netif mtu $mtu
>  	echo ip link set $netif up
> -    fi > /tmp/net.$netif.up
> +    fi >> /tmp/net.$netif.up
>  
> -    echo ip addr add $ip${mask:+/$mask} ${bcast:+broadcast $bcast} dev $netif >> /tmp/net.$netif.up
> +    echo ip addr add $ip/$mask ${bcast:+broadcast $bcast} dev $netif >> /tmp/net.$netif.up
>  
> -    [ -n "$gw" ] && echo ip route add default via $gw dev $netif > /tmp/net.$netif.gw
> +    if [ -n "$gw" ]; then
> +	echo gw=$gw > /tmp/net.$netif.gw
> +	echo ip route add default via $gw dev $netif >> /tmp/net.$netif.gw
> +    fi

Same here, this information is in .dhcpopts There's no need to store 
them twice

>  
>      [ -n "${search}${domain}" ] && echo search $search $domain > /tmp/net.$netif.resolv.conf
>      if  [ -n "$namesrv" ] ; then
> diff --git a/modules.d/40network/ifup b/modules.d/40network/ifup
> index 89017bb..404d6d8 100755
> --- a/modules.d/40network/ifup
> +++ b/modules.d/40network/ifup
> @@ -32,13 +32,17 @@ do_dhcp() {
>  
>  # Handle static ip configuration
>  do_static() {
> -{
> +    {
> +	echo mynet=$ip/$mask

And again...

>  	echo ip link set $netif up 
>  	echo ip addr flush dev $netif
>  	echo ip addr add $ip/$mask dev $netif
>      } > /tmp/net.$netif.up
>  
> -    [ -n "$gw" ] && echo ip route add default via $gw dev $netif > /tmp/net.$netif.gw
> +    if [ -n "$gw" ]; then
> +	 echo gw=$gw > /tmp/net.$netif.gw
> +	 echo ip route add default via $gw dev $netif >> /tmp/net.$netif.gw
> +    fi
>      [ -n "$hostname" ] && echo hostname $hostname > /tmp/net.$netif.hostname
>  
>      echo online > /sys/class/net/$netif/uevent
> diff --git a/modules.d/40network/install b/modules.d/40network/install
> index 0b76cbd..20e8963 100755
> --- a/modules.d/40network/install
> +++ b/modules.d/40network/install
> @@ -1,5 +1,5 @@
>  #!/bin/bash
> -dracut_install ip dhclient hostname
> +dracut_install ip dhclient hostname arping date
>  # Include wired net drivers, excluding wireless
>  for modname in $(find "/lib/modules/$kernel/kernel/drivers" -name '*.ko'); do
>    if nm -uPA $modname | grep -q eth_type_trans; then
> diff --git a/modules.d/40network/netroot b/modules.d/40network/netroot
> index 2cf51fa..58bbd2e 100755
> --- a/modules.d/40network/netroot
> +++ b/modules.d/40network/netroot
> @@ -1,5 +1,29 @@
>  #!/bin/sh
>  
> +apply_mask() {
> +    local ip=$1
> +    local mask=$2
> +    local out i
> +
> +    for i in 1 2 3 4; do
> +	out=$out.$(( ${ip%%.*} & ${mask%%.*} ))
> +	ip=${ip#*.}
> +	mask=${mask#*.}
> +    done
> +    echo ${out#.}
> +}
> +
> +is_local() {
> +    local server=$1
> +    local mynet=$2
> +    local mask net
> +
> +    mask=${mynet#*/}
> +    mynet=$(apply_mask ${mynet%/*} $mask)
> +    net=$(apply_mask $server $mask)
> +    [ "$net" = "$mynet" ]
> +}
> +
>  PATH=$PATH:/sbin:/usr/sbin
>  
>  . /lib/dracut-lib.sh
> @@ -75,8 +99,10 @@ if [ -z "$netroot" ] || [ ! -e "$handler" ] ; then
>      die "No handler for netroot type '$netroot'"
>  fi
>  
> -# Now that we have DHCP information, we can fully validate netroot
> -$handler checkdhcp $netroot "$server_id" "$new_root_path" || exit 1
> +# Now that we have DHCP information, we can get our server
> +$handler server $netroot "$server_id" "$new_root_path" || exit 1
> +[ -s /tmp/server ] || die "Bug in $handler: did not create /tmp/server"
> +read target < /tmp/server
>  
>  # We're here, so we can assume that upping interfaces is now ok
>  [ -z "$IFACES" ] && IFACES="$netif"
> @@ -88,8 +114,21 @@ done
>  [ -e /tmp/net.$netif.hostname ]    && . /tmp/net.$netif.hostname
>  [ -e /tmp/net.$netif.resolv.conf ] && cp -f /tmp/net.$netif.resolv.conf /etc/resolv.conf
>  
> +# Wait for traffic to be passable before we continue
> +is_local $target $mynet || target=$gw
> +
> +# FIXME make 120 configurable
> +TIMEOUT=$(( $(date +%s) + 120 ))

Is 120 seconds really necessary? I've worked with 60 seconds and haven't
had any problems, even when specifically provoking STP with ip down/up 
before issuing arping.

Something else: If we really have to use 120 seconds and/or do this on 
multiple interface we might run into the default udev event timeout of 
180 seconds. Netroot should be moved into the new initqueue in that case.

> +while [ -z "$proceed" -a $(date +%s) -lt $TIMEOUT ]; do
> +    for iface in $IFACES; do
> +	arping -q -f -c 1 -I $iface $target && proceed=1 && break
> +    done
> +done

Just so that I understand you correctly: You're trying to arping on all
interfaces until one says OK? 

If we have multiple interface to up before mount/login, we should take care 
to try and enable all interfaces:

for iface in $IFACES; do
  is_local $gw $mask && local_target=$gw
  is_local $server $mask && local_target=$server

  [ -z "local_target" ] && continue;

  arping -q -f -w $TIMEOUT -I $iface $local_target || die "Unable to ARP ping $local_target via "$iface"
done

> +
>  # Run the handler to mount/login into the root device
> -if $handler mount "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
> +if [ -n "$proceed" ] &&
> +	$handler mount "$netroot" "$server_id" "$new_root_path" $NEWROOT;
> +then
>      # Network rootfs mount successful
>      for iface in $IFACES ; do
>  	[ -f /tmp/dhclient.$iface.lease ] &&    cp /tmp/dhclient.$iface.lease    /tmp/net.$iface.lease
> @@ -98,14 +137,20 @@ if $handler mount "$netroot" "$server_id" "$new_root_path" $NEWROOT; then
>  
>      # Save used netif for later use
>      [ ! -f /tmp/net.ifaces ] && echo $netif > /tmp/net.ifaces
> -else 
> +    exit 0
> +fi
> +
> +if [ -n "$proceed" ]; then
>      warn "Mounting root via '$netif' failed"
> -    # If we're trying with multiple interfaces, put that one down.
> -    # ip down/flush ensures that routeing info goes away as well
> -    if [ -z "$BOOTDEV" ] ; then
> -	ip link set $netif down
> -	ip addr flush dev $netif
> -	echo "#empty" > /etc/resolv.conf
> -    fi
> +else
> +    warn "Unable to ARP ping $target via $netif"
> +fi
> +
> +# If we're trying with multiple interfaces, put that one down.
> +# ip down/flush ensures that routeing info goes away as well
> +if [ -z "$BOOTDEV" ] ; then
> +    ip link set $netif down
> +    ip addr flush dev $netif
> +    echo "#empty" > /etc/resolv.conf
>  fi
>  exit 0

I'd suggest to move [ -n "$proceed" ] to before mount/login and just do 
[ -z "$proceed" ] && die "..."

that way loose the if/else.

> diff --git a/modules.d/95iscsi/iscsiroot b/modules.d/95iscsi/iscsiroot
> index d79b663..7f2efa2 100755
> --- a/modules.d/95iscsi/iscsiroot
> +++ b/modules.d/95iscsi/iscsiroot
> @@ -17,9 +17,10 @@ if getarg rdnetdebug; then
>  fi
>  
>  case "$1" in
> -    check|checkdhcp)	check_only=1 ;;
> -    mount)		;;
> -    *)			die "$0 called with invalid command '$1'" ;;
> +    check)	check_only=1 ;;
> +    server)	server_only=1 ;;
> +    mount)	;;
> +    *)		die "$0 called with invalid command '$1'" ;;
>  esac
>  
>  # root is in the form
> @@ -55,13 +56,6 @@ if [ ! -e /sys/devices/virtual/iscsi_transport ]; then
>      fi
>  fi
>  
> -[ -n "$check_only" ] && exit 0
> -
> -if getarg iscsi_firmware ; then
> -	iscsistart -b
> -	exit 0
> -fi
> -
>  # override conf settings by command line options
>  arg=$(getarg iscsi_initiator)
>  [ -n "$arg" ] && iscsi_initiator=$arg
> @@ -100,6 +94,20 @@ iscsi_lun=$1; shift
>  iscsi_target_name=$*
>  IFS="$OLDIFS"
>  
> +[ -n "$check_only" ] && exit 0
> +
> +# FIXME need support for service discovery if no server given, or
> +# parsing the firmware tables for an IP if iscsi_firmware is in effect
> +if [ -n "$server_only" ]; then
> +    echo $iscsi_target_ip > /tmp/server
> +    exit 0
> +fi
> +
> +if getarg iscsi_firmware ; then
> +	iscsistart -b
> +	exit 0
> +fi
> +
>  # XXX is this needed?
>  getarg ro && iscsirw=ro
>  getarg rw && iscsirw=rw
> diff --git a/modules.d/95nbd/nbdroot b/modules.d/95nbd/nbdroot
> index 55c1b88..9450b84 100755
> --- a/modules.d/95nbd/nbdroot
> +++ b/modules.d/95nbd/nbdroot
> @@ -11,9 +11,10 @@ if getarg rdnetdebug; then
>  fi
>  
>  case "$1" in
> -    check|checkdhcp)	check_only=1 ;;
> -    mount)		;;
> -    *)			die "$0 called with invalid command '$1'" ;;
> +    check)	check_only=1 ;;
> +    server)	server_only=1 ;;
> +    mount)	;;
> +    *)		die "$0 called with invalid command '$1'" ;;
>  esac
>  
>  # root is in the form root=nbd:srv:port[:fstype[:rootflags[:nbdopts]]]
> @@ -52,6 +53,11 @@ incol2 /proc/devices nbd || modprobe nbd ||
>  [ -z "$nbdport" ] && die "NBD root configuration missing port"
>  [ -n "$check_only" ] && exit 0
>  
> +if [ -n "$server_only" ]; then
> +    echo $nbdserver > /tmp/server
> +    exit 0
> +fi
> +
>  # look through the NBD options and pull out the ones that need to
>  # go before the host etc. Append a ',' so we know we terminate the loop
>  nbdopts=${nbdopts},
> diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
> index 8cbfbb8..acfd6b5 100755
> --- a/modules.d/95nfs/nfsroot
> +++ b/modules.d/95nfs/nfsroot
> @@ -12,7 +12,7 @@ fi
>  
>  case "$1" in
>      check)	basic_check=1 ;;
> -    checkdhcp)	full_check=1 ;;
> +    server)	server_only=1 ;;
>      mount)	;;
>      *)		die "$0 called with invalid command '$1'" ;;
>  esac
> @@ -101,8 +101,11 @@ fi
>  [ -z "$server" ] && die "Required parameter 'server' is missing"
>  [ -z "$path" ] && die "NFS root requires a path"
>  
> -# If we're just validating our options, we're done
> -[ -n "$full_check" ] && exit 0
> +# If we're just looking for the server...
> +if [ -n "$server_only" ]; then
> +    echo $server > /tmp/server
> +    exit 0
> +fi
>  
>  # Kernel replaces first %s with host name, and falls back to the ip address
>  # if it isn't set. Only the first %s is substituted.
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 3/5] netroot: remove netif from handler invocation
       [not found]                     ` <1246896365.24010.14.camel-FqX9LgGZnHWDB2HL1qBt2PIbXMQ5te18@public.gmane.org>
@ 2009-07-07  8:10                       ` Seewer Philippe
  0 siblings, 0 replies; 17+ messages in thread
From: Seewer Philippe @ 2009-07-07  8:10 UTC (permalink / raw)
  To: David Dillow; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

David Dillow wrote:
> On Mon, 2009-07-06 at 17:02 +0200, Seewer Philippe wrote:
>> David Dillow wrote:
>>> On Mon, 2009-07-06 at 15:56 +0200, Seewer Philippe wrote:
>>>> Discussion: Instead of implementing the dhcp options server failover
>>>> just for nfs, why not extend this to all protocols?
>>> This does implement it for all protocols, if they choose to use it.
>> Yes, that is correct. I meant it more like, why should only nfs choose 
>> to use it? We could insert the server into the root argument for each 
>> handler inside netroot instead of just "delegating" it.
> 
> Sorry, I misunderstood.
> 
> I like the concept of other protocols using server_id as a fall back
> when no server IP is given in their arguments (ie nbd::9000 would use
> port 9000 on the DHCP server) as it makes it easier to share configs
> among DHCP servers.
> 
> I suspect others will find it confusing, though.
> 
> 
>>> $1 used to be $netif, so we'd get /tmp/iscsiroot.eth0.1234.out. Now we'd
>>> get /tmp/iscsiroot.root=iscsi:..:blah.1234.out instead. Hence the
>>> removal of $1.
>> Oh, didn't realize that. Thanks for claryfing.
> 
> If this series looks to be worth it, I'll try to do better on the commit
> message regarding this point.

Bah. I think that's just me not thinking enough. And yes, I this series
is worth developing further.
--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 4/5] netroot: move dracut syntax validation to root handlers
       [not found]         ` <4A52FD93.3000406-omB+W0Dpw2o@public.gmane.org>
@ 2009-07-07 14:57           ` David Dillow
  0 siblings, 0 replies; 17+ messages in thread
From: David Dillow @ 2009-07-07 14:57 UTC (permalink / raw)
  To: Seewer Philippe; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

On Tue, 2009-07-07 at 09:47 +0200, Seewer Philippe wrote:
> David Dillow wrote:
> > This reduces the argument parsers in the cmdline hook to giving warnings
> > about usage and translating legacy syntaxes to dracut's syntax. The root
> > handlers themselves become responsible for validating the dracut syntax.
> > This simplifies the cmdline parsers and centralizes the syntax parsers,
> > which keeps them from multiplying as new functionality is added.
> 
> I don't think that centralizing code is that important. Dracut is
> module based after all and the central point to find code is the module
> directory. 

I'd rather centralize code into a single file as long as it doesn't get
too big -- and our scripts are no where close to that limit. When making
changes -- or just trying to understand how things work -- it is really
annoying to have to have umpteen different files open just to trace the
flow.

> But there's a two things about this approach that bug me:
> - The netroot handlers become some sort of multi-call utilities, doing
>   possibly entirely different things depending on arguments. From a
>   functional point of view netroot handlers should only care about
>   the actual mount/login.

I'm OK with the multicall handler, obviously. :) I like that all of the
knowledge about a protocol (modulo legacies) is in one file, and the
code to mount/login is usually a small portion of it -- the bulk is the
parsing.

> - Arguments are constantly reparsed. This is unnecessary but applies to
>   the current implementation as well

This doesn't bug me very much, as we're talking milliseconds. And the
alternative is yet more files.

> Suggestion: We don't use the netroot argument inside netroot. Instead, the
> different parsers should write out /tmp/netroot.info in a similar format
> to net.ethX.override or dhclient.ethX.dhcpopts. Netroot modules would
> have to provide three files: Legacy cmdline parser (optional),
> dracut-syntax parser (for netroot= and dhcp root-path), mount/login handler.
> 
> This would work like this:
> cmdline step 1: All legacy parsers run first, validating if necessary
>                 and/or reformat arguments into netroot= if possible
> cmdline step 2: call proto-validate.sh which parses and validates
>                 dracut-syntax
> ==> After this step, if all parameters are correct and root!=dhcp,
>     /tmp/netroot.info should be available.

Minor nit: Unless you're doing NFS and getting info from root-path, or
any netroot handler that supports defaulting the server the DHCP
server-id.

> netroot step 1: If root=dhcp do the same as cmdline step 2
> ==> After this step and if all parameters are correct /tmp/netroot.info
>     should be available.

Minor nit: you have to re-run it for the same reasons above.

> netroot step 2: Source /tmp/netroot.info and use "$server" for STP arping.
>                 If it's empty, issue a warning, use dhcp server and rewrite
>                 the file with the new server=.

Minor nit: you have to fall back to the default gateway if server isn't
available.

> netroot step 3: Just call the mount/login handler without any arguments.
>                 Everything it needs is in /tmp/netroot.info
> 
> What do you think?

It would work, but it isn't my preferred method due to spreading the
knowledge of a protocol over multiple files. I like the idea of making
the cmdline parsers only do legacy rather than the legacy + warnings and
invoke the netroot handler, but there may be some gothcas there that
make things uglier. I'd have to think about that.

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [RFC ONLY 5/5] PROOF-OF-CONCEPT: wait for spanning tree timeout via arping
       [not found]         ` <4A530297.1020906-omB+W0Dpw2o@public.gmane.org>
@ 2009-07-07 15:09           ` David Dillow
  0 siblings, 0 replies; 17+ messages in thread
From: David Dillow @ 2009-07-07 15:09 UTC (permalink / raw)
  To: Seewer Philippe; +Cc: initramfs-u79uwXL29TY76Z2rM5mHXA

On Tue, 2009-07-07 at 10:08 +0200, Seewer Philippe wrote:
> David Dillow wrote:
> > +    echo mynet=$ip/$mask > /tmp/net.$netif.up
> > +
> 
> Please don't do this. $ip/$mask is in .override so we don't need
> more variables.

This is why it is proof-of-concept, I don't like $mynet either. But, not
having it means that we have to parse both .override and .dhcpopts and
figure out which one applies in this case.

I think the real solution is to not use scriptlets in the net.$netif.up
files and move to a /tmp/net.$netif.cfg that has all the variables we
need for configuration, which we can then source. This gets rid of the
$netif.gw and $netif.hostname files as well, since we can pick the one
we want in netroot then.



> > +# FIXME make 120 configurable
> > +TIMEOUT=$(( $(date +%s) + 120 ))
> 
> Is 120 seconds really necessary? I've worked with 60 seconds and haven't
> had any problems, even when specifically provoking STP with ip down/up 
> before issuing arping.

Someone said that the STP timeout could be up to 100 seconds, so I added
a bit for safety. If I made it configurable, we could default to
something like 45-60 seconds and those that run a larger blocking time
could bump it up.

> Something else: If we really have to use 120 seconds and/or do this on 
> multiple interface we might run into the default udev event timeout of 
> 180 seconds. Netroot should be moved into the new initqueue in that case.
> 
> > +while [ -z "$proceed" -a $(date +%s) -lt $TIMEOUT ]; do
> > +    for iface in $IFACES; do
> > +	arping -q -f -c 1 -I $iface $target && proceed=1 && break
> > +    done
> > +done
> 
> Just so that I understand you correctly: You're trying to arping on all
> interfaces until one says OK? 
> 
> If we have multiple interface to up before mount/login, we should take care 
> to try and enable all interfaces:

Good point.

> > +if [ -n "$proceed" ]; then
> >      warn "Mounting root via '$netif' failed"
> > -    # If we're trying with multiple interfaces, put that one down.
> > -    # ip down/flush ensures that routeing info goes away as well
> > -    if [ -z "$BOOTDEV" ] ; then
> > -	ip link set $netif down
> > -	ip addr flush dev $netif
> > -	echo "#empty" > /etc/resolv.conf
> > -    fi
> > +else
> > +    warn "Unable to ARP ping $target via $netif"
> > +fi
> > +
> > +# If we're trying with multiple interfaces, put that one down.
> > +# ip down/flush ensures that routeing info goes away as well
> > +if [ -z "$BOOTDEV" ] ; then
> > +    ip link set $netif down
> > +    ip addr flush dev $netif
> > +    echo "#empty" > /etc/resolv.conf
> >  fi
> >  exit 0
> 
> I'd suggest to move [ -n "$proceed" ] to before mount/login and just do 
> [ -z "$proceed" ] && die "..."
> 
> that way loose the if/else.

But then you leave the interface configured.

Thanks for reviewing and the good comments on this series.

--
To unsubscribe from this list: send the line "unsubscribe initramfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2009-07-07 15:09 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-07-03 21:40 [RFC ONLY 0/5] Move argument validation to root handlers David Dillow
     [not found] ` <cover.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
2009-07-03 21:40   ` [RFC ONLY 1/5] netroot: remove unused hook David Dillow
     [not found]     ` <bd4d79bc048ad3944156f86eafb62cf4bc29b1f9.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
2009-07-06 12:56       ` Seewer Philippe
2009-07-03 21:40   ` [RFC ONLY 2/5] netroot: remove unhelpful argument checks David Dillow
     [not found]     ` <0535c73bd25fdcaa9bf4de0bc4b041b74d11e547.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
2009-07-06 13:00       ` Seewer Philippe
2009-07-03 21:40   ` [RFC ONLY 3/5] netroot: remove netif from handler invocation David Dillow
     [not found]     ` <44494af931ad839908cb4542f7c35ae59a4ef65e.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
2009-07-06 13:56       ` Seewer Philippe
     [not found]         ` <4A520291.7010203-omB+W0Dpw2o@public.gmane.org>
2009-07-06 14:43           ` David Dillow
     [not found]             ` <1246891382.24010.6.camel-FqX9LgGZnHWDB2HL1qBt2PIbXMQ5te18@public.gmane.org>
2009-07-06 15:02               ` Seewer Philippe
     [not found]                 ` <4A521208.2050605-omB+W0Dpw2o@public.gmane.org>
2009-07-06 16:06                   ` David Dillow
     [not found]                     ` <1246896365.24010.14.camel-FqX9LgGZnHWDB2HL1qBt2PIbXMQ5te18@public.gmane.org>
2009-07-07  8:10                       ` Seewer Philippe
2009-07-03 21:40   ` [RFC ONLY 4/5] netroot: move dracut syntax validation to root handlers David Dillow
     [not found]     ` <d1aed501ae771d769efcd370f47184f12b9800d6.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
2009-07-07  7:47       ` Seewer Philippe
     [not found]         ` <4A52FD93.3000406-omB+W0Dpw2o@public.gmane.org>
2009-07-07 14:57           ` David Dillow
2009-07-03 21:40   ` [RFC ONLY 5/5] PROOF-OF-CONCEPT: wait for spanning tree timeout via arping David Dillow
     [not found]     ` <8cf3d034e4611fbe661d4585fbfd1a03a6fd094b.1246656269.git.dave-i1Mk8JYDVaaSihdK6806/g@public.gmane.org>
2009-07-07  8:08       ` Seewer Philippe
     [not found]         ` <4A530297.1020906-omB+W0Dpw2o@public.gmane.org>
2009-07-07 15:09           ` David Dillow

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.