13 NFS syntax variations

13 NFS syntax variations

[Warren Togami]

I ask the contributors again, please consider that we should reduce 
redundant ways of configuring dracut as much as possible.  Please 
consider that this code will ship in various products for the next 
decade.  Is it really a good idea to confuse people by having 
documentation and examples floating around be in several different 
formats?  Sure we developers can understand 13 different syntax 
variations, but this is a lot more complicated to users trying to 
understand examples they find.

Here seems to be every possible dracut NFS syntax variation.

Group A
root=[<server-ip>:]<root-dir>[:<nfs-options>]
root=nfs:[<server-ip>:]<root-dir>[:<nfs-options>]
root=nfs4:[<server-ip>:]<root-dir>[:<nfs-options>]

Group B
netroot=[<server-ip>:]<root-dir>[:<nfs-options>]
netroot=nfs:[<server-ip>:]<root-dir>[:<nfs-options>]
netroot=nfs4:[<server-ip>:]<root-dir>[:<nfs-options>]

Group C
root=dhcp
	DHCP root-path=[<server-ip>:]<root-dir>[:<nfs-options>]
	DHCP root-path=nfs:[<server-ip>:]<root-dir>[:<nfs-options>]
	DHCP root-path=nfs4:[<server-ip>:]<root-dir>[:<nfs-options>]

Legacy nfsroot.txt
root=/dev/nfs nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]

Legacy Variations
root=/dev/nfs4 nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]
root=nfs nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]
root=nfs4 nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]

DHCP Fallbacks
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/filesystems/nfsroot.txt
Seewer's code seems to implement various DHCP-derived fallbacks as 
documented from this ancient method.  I suppose this is OK, except:

* The fallbacks should kick-in only if server-ip is blank from root= or 
DHCP root-path.

> 54   If the `nfsroot' parameter is NOT given on the command line,
> 55   the default "/tftpboot/%s" will be used.

* We should do this only in the case where root=/dev/nfs, or not do it 
at all.  A hard-coded "/tftpboot" root-dir override in the modern 
syntaxes makes no sense.

* We should however support the %s substitution in exactly the same way 
as the kernel.  Looking at our current code though, it seems we don't. 
The docs don't do hostname.

What should we cut?

1) The most obvious to cut are the Legacy Variations.  Why should
dracut invent three new syntaxes and immediately declare them Legacy? 
Removing these would be the simplest.

2) Harald and I think we should cut Group B.  The netroot= syntax is 
necessary only for remote block device protocols like iscsci and nbd 
when you want to use LVM or crypto on those block devices.  For NFS it 
is only redundant.

Removing these would be a bit complex surgery because netroot= is 
tightly ingrained into the current code.

3) I personally think we should cut the "nfs:" prefixed syntaxes from 
Group A, B and C because they have no precedent and we're better off 
with fewer variations.  Others have disagreed though.

Any further thoughts on this?

Warren Togami
wtogami-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
--
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

Re: 13 NFS syntax variations

[Seewer Philippe]

Warren Togami wrote:
> I ask the contributors again, please consider that we should reduce 
> redundant ways of configuring dracut as much as possible.  Please 
> consider that this code will ship in various products for the next 
> decade.  Is it really a good idea to confuse people by having 
> documentation and examples floating around be in several different 
> formats?  Sure we developers can understand 13 different syntax 
> variations, but this is a lot more complicated to users trying to 
> understand examples they find.
> 
> Here seems to be every possible dracut NFS syntax variation.
> 
> Group A
> root=[<server-ip>:]<root-dir>[:<nfs-options>]
> root=nfs:[<server-ip>:]<root-dir>[:<nfs-options>]
> root=nfs4:[<server-ip>:]<root-dir>[:<nfs-options>]
> 
> Group B
> netroot=[<server-ip>:]<root-dir>[:<nfs-options>]
> netroot=nfs:[<server-ip>:]<root-dir>[:<nfs-options>]
> netroot=nfs4:[<server-ip>:]<root-dir>[:<nfs-options>]
> 
> Group C
> root=dhcp
>     DHCP root-path=[<server-ip>:]<root-dir>[:<nfs-options>]
>     DHCP root-path=nfs:[<server-ip>:]<root-dir>[:<nfs-options>]
>     DHCP root-path=nfs4:[<server-ip>:]<root-dir>[:<nfs-options>]
> 
> Legacy nfsroot.txt
> root=/dev/nfs nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]
> 
> Legacy Variations
> root=/dev/nfs4 nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]
> root=nfs nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]
> root=nfs4 nfsroot=[<server-ip>:]<root-dir>[,<nfs-options>]
> 
> DHCP Fallbacks
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/filesystems/nfsroot.txt 
> 
> Seewer's code seems to implement various DHCP-derived fallbacks as 
> documented from this ancient method.  I suppose this is OK, except:
> 
> * The fallbacks should kick-in only if server-ip is blank from root= or 
> DHCP root-path.

Yes. But (sorry if I seem to be slow) I don't see the difference between
my code and your statement. It does exactly that. If server-ip is missing
from root, nfsroot or dhcp root-path it does the fallback.  

>> 54   If the `nfsroot' parameter is NOT given on the command line,
>> 55   the default "/tftpboot/%s" will be used.
> 
> * We should do this only in the case where root=/dev/nfs, or not do it 
> at all.  A hard-coded "/tftpboot" root-dir override in the modern 
> syntaxes makes no sense.

Good point, agreed.

> 
> * We should however support the %s substitution in exactly the same way 
> as the kernel.  Looking at our current code though, it seems we don't. 
> The docs don't do hostname.

Removing hostname isn't a problem, patch below
 
> What should we cut?
> 
> 1) The most obvious to cut are the Legacy Variations.  Why should
> dracut invent three new syntaxes and immediately declare them Legacy? 
> Removing these would be the simplest.

If I understand David Dillow correctly they were meant as shortcuts.
Removing them doesn't break anything, so I agree.

> 
> 2) Harald and I think we should cut Group B.  The netroot= syntax is 
> necessary only for remote block device protocols like iscsci and nbd 
> when you want to use LVM or crypto on those block devices.  For NFS it 
> is only redundant.

Reduntant: Yes. But it keeps the argument scheme consistent. And dracut
is allowed to introduce new syntaxes if they make sense.

> Removing these would be a bit complex surgery because netroot= is 
> tightly ingrained into the current code.

Not true, see patch below.

> 3) I personally think we should cut the "nfs:" prefixed syntaxes from 
> Group A, B and C because they have no precedent and we're better off 
> with fewer variations.  Others have disagreed though.

See above, I like consistency. But that's just my opinion.

Regards,
Philippe

diff --git a/modules.d/95nfs/nfsroot b/modules.d/95nfs/nfsroot
index 46c2b42..5f52457 100755
--- a/modules.d/95nfs/nfsroot
+++ b/modules.d/95nfs/nfsroot
@@ -109,9 +109,7 @@ if [ "${path#*%s}" != "$path" ]; then
     ip=$(ip -o -f inet addr show $netif)
     ip=${ip%%/*}
     ip=${ip##* }
-    read node < /proc/sys/kernel/hostname
-    [ "$node" = "(none)" ] && node=$ip
-    path=${path%%%s*}$node${path#*%s}
+    path=${path%%%s*}$ip${path#*%s}
 fi
 
 # Look through the options and remove rw/locking options
diff --git a/modules.d/95nfs/parse-nfsroot.sh b/modules.d/95nfs/parse-nfsroot.sh
index 5413ad5..55fbc50 100755
--- a/modules.d/95nfs/parse-nfsroot.sh
+++ b/modules.d/95nfs/parse-nfsroot.sh
@@ -71,23 +71,23 @@ netroot_to_var() {
 [ -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
+
 # Handle old style <server-ip>:/<path
 case "$netroot" in
     [0-9]*:/*|[0-9]*\.[0-9]*\.[0-9]*[!:]|/*)
        netroot=nfs:$netroot;;
 esac
 
-# Root takes precedence over netroot
-case "${root%%:*}" in
-    nfs|nfs4|/dev/nfs|/dev/nfs4)
-    if [ -n "$netroot" ] ; then
-	warn "root takes precedence over netroot. Ignoring netroot"
-
-    fi
-    netroot=$root
-    ;;
-esac
-
 # If it's not empty or nfs we don't continue
 case "${netroot%%:*}" in
     ''|nfs|nfs4|/dev/nfs|/dev/nfs4);;
--
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

%s substitution

[Warren Togami]

On 06/18/2009 06:15 AM, Seewer Philippe wrote:
>>
>> * We should however support the %s substitution in exactly the same
>> way as the kernel. Looking at our current code though, it seems we
>> don't. The docs don't do hostname.
>
> Removing hostname isn't a problem, patch below

I noticed another potential problem...

> 62   <root-dir>    Name of the directory on the server to mount as root.
> 63                 If there is a "%s" token in the string, it will be
> 64                 replaced by the ASCII-representation of the client's
> 65                 IP address.

> # 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.
> if [ "${path#*%s}" != "$path" ]; then

Wont this only match if the $path ends with %s?  The nfsroot.txt seems 
to indicate it should be substituted if the %s appears anywhere in the 
string.

I'll wait until you have an updated patch fixing both issues.

Also, did you verify that the ip format you generate is identical to the 
"ASCII-representation of the client's IP address" as generated by the 
kernel?

Warren Togami
wtogami-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
--
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

Re: %s substitution

[David Dillow]

On Thu, 2009-06-18 at 13:48 -0400, Warren Togami wrote:
> > # 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.
> > if [ "${path#*%s}" != "$path" ]; then
> 
> Wont this only match if the $path ends with %s?  The nfsroot.txt seems 
> to indicate it should be substituted if the %s appears anywhere in the 
> string.

It will match if %s is anywhere in the string, it will strip it out and
it will no longer match the original path. Then we go into the code that
does the actual substitution.

The nice part about working in shell scripts is that it takes 3 seconds
to test out something you don't understand.

> Also, did you verify that the ip format you generate is identical to the 
> "ASCII-representation of the client's IP address" as generated by the 
> kernel?

The kernel docs say IP address, but the actual kernel code uses the host
name if set and falls back to the IP address.
--
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

Re: %s substitution

[Seewer Philippe]

David Dillow wrote:
[snip]
>> Also, did you verify that the ip format you generate is identical to the 
>> "ASCII-representation of the client's IP address" as generated by the 
>> kernel?
> 
> The kernel docs say IP address, but the actual kernel code uses the host
> name if set and falls back to the IP address.

That is indeed the case (from vanilla 2.6.29/fs/nfs/nfsroot.c):

[quote]
*      Martin Mares    :       Default path now contains host name instead of
*                              host IP address (but host name defaults to IP
*                              address anyway).
[/quote]

[quote]
	cp = utsname()->nodename;
        if (strlen(buf) + strlen(cp) > NFS_MAXPATHLEN) {
                printk(KERN_ERR "Root-NFS: Pathname for remote directory too long.\n");
                return -1;
        }
        sprintf(nfs_path, buf, cp);
[/quote]
--
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

Re: 13 NFS syntax variations

[Warren Togami]

On 06/18/2009 06:15 AM, Seewer Philippe wrote:
>> What should we cut?
>>
>> 1) The most obvious to cut are the Legacy Variations. Why should
>> dracut invent three new syntaxes and immediately declare them Legacy?
>> Removing these would be the simplest.
>
> If I understand David Dillow correctly they were meant as shortcuts.
> Removing them doesn't break anything, so I agree.

Soon pushing this.  This removes 3 variations of the nfsroot.txt syntax 
that are new inventions of dracut.

>>
>> 2) Harald and I think we should cut Group B. The netroot= syntax is
>> necessary only for remote block device protocols like iscsci and nbd
>> when you want to use LVM or crypto on those block devices. For NFS it
>> is only redundant.
>
> Reduntant: Yes. But it keeps the argument scheme consistent. And dracut
> is allowed to introduce new syntaxes if they make sense.

root= and netroot= redundant syntaxes with redundant tests make sense how?

* netroot= NFS variation adds to confusion understanding the 
documentation and examples.
* Complaints about the tests taking too long?  Remove all the redundant 
tests that do identical root= and netroot=.
* Separate netroot= only exists for remote block devices, useless for NFS.

This would remove 3 redundant new syntaxes.

>> 3) I personally think we should cut the "nfs:" prefixed syntaxes from
>> Group A, B and C because they have no precedent and we're better off
>> with fewer variations. Others have disagreed though.

Alternatively, how do people feel about removing the NFS syntax that is 
missing the nfs: prefix?  It was poorly supported (did not support 
nfs-options) in mkinitrd, lacked DHCP support, and non-existent in 
Debian's initramfs-tools.

root=[<server-ip>:]<root-dir>[:<nfs-options>]
root=dhcp
	root-path=[<server-ip>:]<root-dir>[:<nfs-options>]

RHEL5 supported the former without nfs-options, but without the ability 
to load network drivers it was almost useless.
Fedora 10+ supported the latter with root=dhcp but without nfs-options.

Options:
1) Leave it as-is.
2) Remove it.
3) Support it but print deprecation warnings like Legacy nfsroot.txt.

Opinions?

Warren Togami
wtogami-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
--
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

Re: 13 NFS syntax variations

[David Dillow]

On Mon, 2009-06-22 at 16:56 -0400, Warren Togami wrote:
> On 06/18/2009 06:15 AM, Seewer Philippe wrote:
> >> 2) Harald and I think we should cut Group B. The netroot= syntax is
> >> necessary only for remote block device protocols like iscsci and nbd
> >> when you want to use LVM or crypto on those block devices. For NFS it
> >> is only redundant.
> >
> > Reduntant: Yes. But it keeps the argument scheme consistent. And dracut
> > is allowed to introduce new syntaxes if they make sense.
> 
> root= and netroot= redundant syntaxes with redundant tests make sense how?

The redundant tests could be removed yes, but they are currently
redundant solely because they try to not make any assumptions about the
implementation.

> * netroot= NFS variation adds to confusion understanding the 
> documentation and examples.

I don't see this, but perhaps I'm too close to it. To me it would make
more sense to say that netroot= is the proper way to get a NFS root or
network device for root, and root=dhcp/root=nfs/root=nbd/root=iscsi etc
are the legacy ways.

> * Complaints about the tests taking too long?  Remove all the redundant 
> tests that do identical root= and netroot=.

Sure, but you open up possibilities of missing unintended changes when
you make assumptions in the test code.

> * Separate netroot= only exists for remote block devices, useless for NFS.

Maybe, in your definition. But since all of the network root code
translates existing root sources requiring the network into a netroot=
argument, it would actually take more code to remove support for
netroot=nfs:... than it takes to support it.

> >> 3) I personally think we should cut the "nfs:" prefixed syntaxes from
> >> Group A, B and C because they have no precedent and we're better off
> >> with fewer variations. Others have disagreed though.
> 
> Alternatively, how do people feel about removing the NFS syntax that is 
> missing the nfs: prefix?  It was poorly supported (did not support 
> nfs-options) in mkinitrd, lacked DHCP support, and non-existent in 
> Debian's initramfs-tools.

> 3) Support it but print deprecation warnings like Legacy nfsroot.txt.

This would be my choice; the DHCP (BOOTP) root-path=[server:]/root-path
formats have been around for decades. I seem to recall network booting a
SunOS 4.1.2 box using that back in 1993, and I'll bet it wasn't a new
feature then.

Some changes I'm thinking about would benefit from dropping the non nfs:
formats, as they wouldn't have a clean home otherwise, but I don't think
that is a good reason to drop them -- they are too easy to support.

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

Re: 13 NFS syntax variations

[Warren Togami]

On 06/22/2009 06:29 PM, David Dillow wrote:
>> * netroot= NFS variation adds to confusion understanding the
>> documentation and examples.
>
> I don't see this, but perhaps I'm too close to it. To me it would make
> more sense to say that netroot= is the proper way to get a NFS root or
> network device for root, and root=dhcp/root=nfs/root=nbd/root=iscsi etc
> are the legacy ways.

I'm willing to consider dropping root=* and supporting only netroot= for 
NFS.

OTOH, Harald was thinking about extending the iscsi root-path syntax 
specification to allow for specifying the root device (by partition 
name, LABEL, UUID, etc.) within the root-path itself.  This might 
eliminate the need for a separate root=.

There may however be value in allowing an optional separate root=.  This 
might allow a mechanism for a bootloader menu to select a different root 
device during netboot.

Harald have you put further thought into the iscsi root-path syntax 
extension?

>> 3) Support it but print deprecation warnings like Legacy nfsroot.txt.
>
> This would be my choice; the DHCP (BOOTP) root-path=[server:]/root-path
> formats have been around for decades. I seem to recall network booting a
> SunOS 4.1.2 box using that back in 1993, and I'll bet it wasn't a new
> feature then.
>
> Some changes I'm thinking about would benefit from dropping the non nfs:
> formats, as they wouldn't have a clean home otherwise, but I don't think
> that is a good reason to drop them -- they are too easy to support.

"Too easy to support" does not necessarily mean we should support.

Warren Togami
wtogami-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
--
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

Re: 13 NFS syntax variations

[Warren Togami]

On 06/22/2009 06:29 PM, David Dillow wrote:
>> 3) Support it but print deprecation warnings like Legacy nfsroot.txt.
>
> This would be my choice; the DHCP (BOOTP) root-path=[server:]/root-path
> formats have been around for decades. I seem to recall network booting a
> SunOS 4.1.2 box using that back in 1993, and I'll bet it wasn't a new
> feature then.
>
> Some changes I'm thinking about would benefit from dropping the non nfs:
> formats, as they wouldn't have a clean home otherwise, but I don't think
> that is a good reason to drop them -- they are too easy to support.

root=<server-ip>:/path:nfs-options

Yes, I am aware it has existed for decades.  However neither Red Hat nor 
Debian's tools supported this method.[1]  This seems to make it a good 
candidate for chopping.

Harald, what do you think?

[1]
mkinitrd supports it only partially, without nfs-options, and without a 
method to auto-load network drivers prior to NFS, meaning nobody used it.

Warren Togami
wtogami-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
--
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

Re: 13 NFS syntax variations

[David Dillow]

On Tue, 2009-06-23 at 00:15 -0400, Warren Togami wrote:
> root=<server-ip>:/path:nfs-options
> 
> Yes, I am aware it has existed for decades.  However neither Red Hat nor 
> Debian's tools supported this method.[1]  This seems to make it a good 
> candidate for chopping.

Industry practice outside of Red Hat or Debian doesn't count? Then I
guess it is a good thing that Debian's initramfs-tools supports it,
albeit without options. 

> Harald, what do you think?

I'm sure he'll speak his own mind, but I'll point out he re-added the
syntax in commit 1e885ccd


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

Re: 13 NFS syntax variations

[Harald Hoyer]

On 06/23/2009 06:15 AM, Warren Togami wrote:
> On 06/22/2009 06:29 PM, David Dillow wrote:
>>> 3) Support it but print deprecation warnings like Legacy nfsroot.txt.
>>
>> This would be my choice; the DHCP (BOOTP) root-path=[server:]/root-path
>> formats have been around for decades. I seem to recall network booting a
>> SunOS 4.1.2 box using that back in 1993, and I'll bet it wasn't a new
>> feature then.
>>
>> Some changes I'm thinking about would benefit from dropping the non nfs:
>> formats, as they wouldn't have a clean home otherwise, but I don't think
>> that is a good reason to drop them -- they are too easy to support.
>
> root=<server-ip>:/path:nfs-options
>
> Yes, I am aware it has existed for decades. However neither Red Hat nor
> Debian's tools supported this method.[1] This seems to make it a good
> candidate for chopping.
>
> Harald, what do you think?

root=<server-ip>:/path

should stay.
Adding the options was just the next logical step, be it with ":" or ","

>
> [1]
> mkinitrd supports it only partially, without nfs-options, and without a
> method to auto-load network drivers prior to NFS, meaning nobody used it.
>
--
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