feature request: support colons in nbdroot kernel parameter

[Alkis Georgopoulos]

Continuing from https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/593227/comments/5

In LTSP the following possible syntaxes are used for nbdroot:
 * nbdroot=server_ip:port
 * nbdroot=server_ip
   So nbdroot here actually means nbdsrv=xxx.
   The port is assumed to be 2000 in this case.
   (yup unfortunately that's not a good default number :-()
 * nbdport=port
   If nbdroot is not provided, the server is
   assumed to be the same as the DHCP server.

ltsp_nbd doesn't require nor it supports a root=/dev/nbd* parameter.
The first /dev/nbd* device to be available is used.

So I guess that the attached *untested* patch would take nbd and ltsp_nbd a little closer.

Link to the 2 nbd-related LTSP scripts:
http://bazaar.launchpad.net/~ltsp-upstream/ltsp/ltsp-trunk/annotate/head:/client/initramfs/scripts/ltsp_nbd
http://bazaar.launchpad.net/~ltsp-upstream/ltsp/ltsp-trunk/annotate/head:/client/initscripts/ltsp-init-common

[Wouter Verhelst (wouter-debian)]

Hi,

For some reason, I didn't see this until now. Since Debian is now in
freeze for squeeze, it's too late to incorporate this patch.

I'll look at it after the release of squeeze.

--
The biometric identification system at the gates of the CIA headquarters
works because there's a guard with a large gun making sure no one is
trying to fool the system.
  http://www.schneier.com/blog/archives/2009/01/biometrics.html

[Alkis Georgopoulos (alkisg)]

Hi Wouter,

I think that with the new name-based exports and the 10809 port, we'll need to change the pxelinux.cfg/default file provided by LTSP anyway, so I'm not sure that supporting colons will be necessary, maybe we can just switch to the commas syntax in LTSP.

So I think this feature request should be postponed until nbd 2.9.18 or later lands in Debian & Ubuntu and until LTSP is updated to support the new name-based exports.

Thanks!

[Alkis Georgopoulos (alkisg)]

Solved by adding support for an NBDROOT environment variable:
http://anonscm.debian.org/gitweb/?p=users/wouter/nbd.git;a=commit;h=a68fec1f04c8c95d2af704eb9629ec59d077fd2a

Thanks Wouter :)

[Alkis Georgopoulos (alkisg)]

While the NBDROOT environment variable would allow for some flexibility, it still wouldn't allow LTSP to use the nbd-client.initrd script, as some LTSP users need to use the nbd-proxy hack.
Also, LTSP would need to initialize networking before nbd-client.initrd, to get the necessary info from DHCP, so I think that providing a way to launch an NBDCLIENT wrapper would be more flexible than the NBDROOT env var.
Furthermore, I believe it's best to have a common syntax for NBD, and the host:port/path syntax that NFS uses (http://www.rfc-editor.org/rfc/rfc2224.txt) is a good candidate.

I'm attaching a proposed nbd-client.initrd script that should handled all the use cases I've heard, while still being rather simple and while maintaining the commas-based syntax. I also did some minor optimizations, like replacing sed with case etc.

Wouter, could you please consider it for inclusion? Thanks!

[Alkis Georgopoulos (alkisg)]

Newer version that replaces `case "$x"` with `case "$nbdroot"`.

[Wouter Verhelst (wouter-debian)]

Hi Alkis,

On Thu, Dec 01, 2011 at 06:14:42AM -0000, Alkis Georgopoulos wrote:
> While the NBDROOT environment variable would allow for some flexibility, it
> still wouldn't allow LTSP to use the nbd-client.initrd script, as some LTSP
> users need to use the nbd-proxy hack.
> Also, LTSP would need to initialize networking before nbd-client.initrd, to
> get the necessary info from DHCP, so I think that providing a way to launch
> an NBDCLIENT wrapper would be more flexible than the NBDROOT env var.

Right.

> Furthermore, I believe it's best to have a common syntax for NBD, and the
> host:port/path syntax that NFS uses
> (http://www.rfc-editor.org/rfc/rfc2224.txt) is a good candidate.
>
> I'm attaching a proposed nbd-client.initrd script that should handled
> all the use cases I've heard, while still being rather simple and while
> maintaining the commas-based syntax. I also did some minor
> optimizations, like replacing sed with case etc.

It doesn't look too intrusive, and I would accept it if it worked, but
there's a bug. Try running:

a='[2001:6f8:32f::39]:1234'
echo ${a%%:*}

the result is:

[2001

rather than

[2001:6f8:32f::39]

I'm not entirely sure how to fix this. In fact, the fact that IPv6 uses
colons is the reason I chose commas rather than colons as hostname-port
separator -- that way I didn't have to deal with this kind of mess :-)

> Wouter, could you please consider it for inclusion? Thanks!
>
> ** Attachment added: "nbd-client.initrd"
> https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/594595/+attachment/2614365/+files/nbd-client.initrd
>
> ** Patch removed: "Patch for debian/nbd-client.initrd"
> https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/594595/+attachment/1427143/+files/nbd.patch
>
> ** Changed in: nbd (Ubuntu)
> Status: Invalid => Triaged
>
> ** Changed in: nbd (Ubuntu)
> Importance: Undecided => Wishlist
>
> --
> You received this bug notification because you are subscribed to nbd in
> Ubuntu.
> https://bugs.launchpad.net/bugs/594595
>
> Title:
> feature request: support colons in nbdroot kernel parameter
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/ubuntu/+source/nbd/+bug/594595/+subscriptions
>

--
The volume of a pizza of thickness a and radius z can be described by
the following formula:

pi zz a

[Alkis Georgopoulos (alkisg)]

Yup sorry in line 61 I had nbdsrv=${nbdsrv%%:*} instead of the correct nbdsrv=${nbdsrv%:*}.

Here's a snippet to verify it should be ok now:
nbdroot='[2001:6f8:32f::39]:1234/some/nbd/path'
nbdsrv=${nbdroot%%/*}
nbdpath="/${nbdroot#*/}"
case "$nbdsrv" in
    *:*)
        nbdport=${nbdsrv##*:}
        nbdsrv=${nbdsrv%:*}
        # Square brackets are used for IPv6 hosts. Strip them.
        nbdsrv=${nbdsrv#[}
        nbdsrv=${nbdsrv%]}
esac
echo "nbdroot=$nbdroot, nbdsrv=$nbdsrv, nbdport=$nbdport, nbdpath=$nbdpath"

Attaching the corrected nbd-client.initrd, along with 2 other minor enchancements, to get it under 100 lines. :)

[Alkis Georgopoulos (alkisg)]

...and yet another revised version. I should have linked a bzr branch instead. :D

[Alkis Georgopoulos (alkisg)]

'morning Wouter, attaching my last proposal.
I did some more testing in the ip:port/path parsing, it should cover all cases now. Also, up to 2 of these parameters can be omitted.

[Wouter Verhelst (wouter-debian)]

On Fri, Dec 02, 2011 at 04:20:25AM -0000, Alkis Georgopoulos wrote:
> 'morning Wouter, attaching my last proposal.
> I did some more testing in the ip:port/path parsing, it should cover all
> cases now. Also, up to 2 of these parameters can be omitted.

Wonderful.

I'll give it some testing and do an upload "soonishly". Unfortunately
that probably won't be this weekend, since I've got plenty of other
things to do; but I'll try to remember doing this by the end of next
week, or some such. Don't hesitate to remind me if I do forget.

--
The volume of a pizza of thickness a and radius z can be described by
the following formula:

pi zz a

[Alkis Georgopoulos (alkisg)]

Fix committed in http://anonscm.debian.org/gitweb/?p=users/wouter/nbd.git;a=commit;h=68340a2b17cd1f14f5bf1d52c7ab8c4ca8fa89d3, thank you Wouter.
But we also need to add support for the new boot options in the sendsigs protection code, so I'll leave the bug open until I find some time to propose another patch for /etc/init.d/nbd-client, unless of course you do it yourself before that. :)

[Launchpad Janitor (janitor)]

This bug was fixed in the package nbd - 1:2.9.25-2ubuntu1

---------------
nbd (1:2.9.25-2ubuntu1) precise; urgency=low

  * Merge from Debian unstable. Remaining changes (LP: #594595):
    - Drop /etc/modprobe.d/nbd-client; the kernel default is already
      appropriate. Ubuntu-specific change.
      + Modify debian/nbd-client.dirs
      + Remove debian/nbd-client.modprobe
      + Add debian/nbd-client.preinst

nbd (1:2.9.25-2) unstable; urgency=low

  * Add support for yet more ways to netboot on NBD, this time to help
    our friends of LTSP.
  * Update documentation to reflect new possibilities, and refactor it a
    bit so it's more clear now.
  * Update standards-version to 3.9.2 (no changes applicable to nbd)
  * use #if, not #ifdef. Closes: #651116

nbd (1:2.9.25-1) unstable; urgency=low

  * New upstream release
 -- Stephane Graber <email address hidden> Sat, 11 Feb 2012 12:35:53 -0500