feature request: support colons in nbdroot kernel parameter

Bug #594595 reported by Alkis Georgopoulos on 2010-06-15
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
nbd (Ubuntu)
Wishlist
Unassigned

Bug Description

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

Related branches

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) wrote :

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!

tags: added: patch
Alkis Georgopoulos (alkisg) wrote :

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

Changed in nbd (Ubuntu):
status: New → Invalid
Alkis Georgopoulos (alkisg) 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.
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!

Changed in nbd (Ubuntu):
status: Invalid → Triaged
importance: Undecided → Wishlist
Alkis Georgopoulos (alkisg) wrote :

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

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) wrote :

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) wrote :

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

Alkis Georgopoulos (alkisg) 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.

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) wrote :

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) wrote :

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

Changed in nbd (Ubuntu):
status: Triaged → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers