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

Re: [Nbd] nbd-server working easily in cygwin in XP



Wouter Verhelst <w@...112...> wrote on Thu, 24 Jul 2008 13:50:01 +0200:
" On Sun, Jul 20, 2008 at 11:56:46AM -0700, ulmo@...205... wrote:

[Before we fixed it, when I tried nbd-server with cygwin:]
" > 3.  I found out nbd-server disconnected whenever nbd-client
" >     connected to it.
" 
" On cygwin, too? We hit that on Solaris as well...
[...]
" On Linux, apparently the default is to make sockets blocking, even
" if they're coming from an accept() call from a socket that is
" blocking.  Apparently this is not POSIX, which I presumed it to
" be. Of course making incorrect assumptions is never a good idea :-)
" 
" I've incorporated this patch now (though I've added it right after
" the accept() call, instead of at the beginning of the negotiate()
" function), so it should now work on systems which inherit socket
" flags through accept(). This should also fix the issues on Solaris
" that I promised ages ago to debug; Daniel (CC), could you check?

Thank you.  The commits you made for svn checkout from sourceforge at
branches/release_2_9/nbd/ work well for cygwin, and nbd-server now
works out of the box in cygwin with the following:

" >  #define NBD_SET_SOCK   _IO( 0xab, 0 )
" >  #define NBD_SET_BLKSIZE        _IO( 0xab, 1 )
" > @@ -39,8 +39,8 @@
" >  /* userspace doesn't need the nbd_device structure */
" >  #ifdef __KERNEL__
" > 
" > -#include <linux/wait.h>
" > -#include <linux/mutex.h>
" > +/*#include <linux/wait.h>*/
" > +/*#include <linux/mutex.h>*/
" 
" Note that these are inside an #ifdef __KERNEL__, so they won't hurt
" even if they're left in there.

I copied my nbd.h out (which itself was a modified copy of a recent
kernel tarball's nbd.h with the #include's commented out, and which
you commented on above), and then uncommented the comments in the
"#ifdef __LINUX__" piece like you mentioned, leaving the only comment
"#include <linux/types.h>" as different from the original.  So, to
clean that up, I searched for another comment I could make to figure
out what type of system it is that should work for all systems, and
found:

__CYGWIN__
__linux__

I decided to use __linux__, so the patch from the original nbd.h from
the kernel becomes:

--- nbd.h.orig	2008-08-12 06:38:02.906250000 -0700
+++ nbd.h	2008-08-12 06:37:36.500000000 -0700
@@ -15,7 +15,9 @@
 #ifndef LINUX_NBD_H
 #define LINUX_NBD_H
 
+#ifdef __linux__
 #include <linux/types.h>
+#endif /* __linux__ */
 
 #define NBD_SET_SOCK	_IO( 0xab, 0 )
 #define NBD_SET_BLKSIZE	_IO( 0xab, 1 )
@@ -70,7 +72,7 @@
 	int xmit_timeout;
 };
 
-#endif
+#endif /* __KERNEL__ */
 
 /* These are sent over the network in the request/reply magic fields */
 
./configure should work now.

With an autoconf-cooked release, none of the autoreconf stuff would be
necessary.  If someone had a Linux system with nbd-client, they could
copy the new source to it and autoconf-cook it there or something
(copy old ./configure ane Makefile.in if cooking is too hard).
autoreconf is a bastard in my current cygwin.  Here's what I did that
did work, so I didn't have to go get it cooked from Linux or a
distribution src ball:

  #(install docbook2x from sourceforge (0.8.8 went in no problem) -- not
  #necessary because it doesn't work anyway and worked despite the failure)
  touch nbd-server.5.in nbd-server.1.in
  autoreconf
  automake --add-missing
  autoreconf
  ./configure --prefix=/usr/local
  make

("make nbd-server" skips some unnecessary stuff and produces fewer
errors.)

Note that I use /usr/local since it is not a distribution-versioned
package (in this case distribution=cygwin, otherwise could mean gentoo
or openbsd or redhat or debian or whatever), my criteria for putting
it in /usr/local.  If it were a cygwin native package, the cygwin
maintainer should put it into the various correct non-local
directories (I'm assuming /etc, /usr/bin, etc.), testing that the
manual page points to the correct config file for nbd-server.
  
Ok, and now for the test:

  cp README /usr/local/etc/nbd-server/config
  vi /usr/local/etc/nbd-server/config
  cp -a nbd-server.exe /usr/local/bin/
  nbd-server

Boot remote system that wants it so I can test it ... and it now works
fine!  (Don't forget to poke a hole in Windows Firewall; clicking
"Unblock" on the popup is insufficient.)

Thank you.

Ok, for the other fixes:

I use a local INIT_PASSWD that isn't 8 bytes, so here are the
necessary modifications for that:

--- nbd-client.c~	2008-08-12 06:44:12.578125000 -0700
+++ nbd-client.c	2008-08-12 07:57:16.446750000 -0700
@@ -96,7 +96,7 @@
 	char buf[256] = "\0\0\0\0\0\0\0\0\0";
 
 	printf("Negotiation: ");
-	if (read(sock, buf, 8) < 0)
+	if (read(sock, buf, strlen(INIT_PASSWD)) < 0)
 		err("Failed/1: %m");
 	if (strlen(buf)==0)
 		err("Server closed connection");
--- nbd-server.c~	2008-08-12 06:44:12.421875000 -0700
+++ nbd-server.c	2008-08-12 07:56:49.337375000 -0700
@@ -1061,7 +1061,7 @@
 	u32 flags = NBD_FLAG_HAS_FLAGS;
 
 	memset(zeros, '\0', sizeof(zeros));
-	if (write(client->net, INIT_PASSWD, 8) < 0)
+	if (write(client->net, INIT_PASSWD, strlen(INIT_PASSWD)) < 0)
 		err("Negotiation failed: %m");
 	cliserv_magic = htonll(cliserv_magic);
 	if (write(client->net, &cliserv_magic, sizeof(cliserv_magic)) < 0)
--- nbd-client.c~	2008-08-12 07:57:16.446750000 -0700
+++ nbd-client.c	2008-08-12 08:01:24.616625000 -0700
@@ -100,7 +100,7 @@
 		err("Failed/1: %m");
 	if (strlen(buf)==0)
 		err("Server closed connection");
-	if (strcmp(buf, INIT_PASSWD))
+	if (strncmp(buf, INIT_PASSWD, strlen(INIT_PASSWD)))
 		err("INIT_PASSWD bad");
 	printf(".");
 	if (read(sock, &magic, sizeof(magic)) < 0)

I probably forgot something in the above (I've re-done that so many
times, I forget).


" > 5.  In cygwin in XP, a big file I had of 7.5G worked fine, but
" >     when I had DODBG [and NOFORK] defined to 1 (in config.h) and
" >     it printed the size, the size was minus 2^32, so I assume it
" >     just wraps at 2^32.  So to isolate that that wasn't the
" >     problem, besides testing smaller pieces and stuff, I put in a
" >     simple test to check if the variable was at least correct.

That debugging bug with the 8 octect integer printing as a 4 octect
number where I wrote the little function that prints decimal numbers
can be redone like this much cleaner (and locale-clean as well):

In Linux, the manual page says L is for floating point numbers; ll is
the one to use for integers, and I'm assuming Cygwin is the same
(which apparently it is).  I replaced all the "%L" with "%ll" (that
is, %Lu with %llu and %Ld with %lld), and as you can see in the
following debugging output, it works fine (in cygwin, with DODBG and
NOFORK #defined to 1 in config.h for debugging purposes):

Script started on Tue Aug 12 10:15:18 2008
$ ./nbd-server -C /dev/null 1111 /dev/sdd
** (process:892): WARNING **: Could not parse config file: Could not open config file.
Waiting for connections... bind, listen, accept, ** Message: connect from 10.0.0.51, assigned file is /dev/sdd
** Message: Can't open authorization file /usr/local/etc/nbd-server/allow (No such file or directory).
** Message: Authorized client
** Message: Starting to serve
Opening /dev/sdd
looking for fhandle size with fstat
looking for fhandle size with lseek SEEK_END
** Message: Size of exported file/device is 320209333776
[                                           ^^^^^^^^^^^^ correct size]
Entering request loop!
1: *** Message: Disconnect request received.
$ exit
Script done on Tue Aug 12 10:15:46 2008
[Note: I changed some values in debugging output for security.]

Small bug, easy fix.  Here's the patch for that:

--- nbd-server.c~	2008-08-12 07:56:49.337375000 -0700
+++ nbd-server.c	2008-08-12 10:06:44.109500000 -0700
@@ -892,7 +892,7 @@
 	if(maxbytes && len > maxbytes)
 		len = maxbytes;
 
-	DEBUG4("(WRITE to fd %d offset %Lu len %u), ", fhandle, foffset, len);
+	DEBUG4("(WRITE to fd %d offset %llu len %u), ", fhandle, foffset, len);
 
 	myseek(fhandle, foffset);
 	return write(fhandle, buf, len);
@@ -934,7 +934,7 @@
 	if(maxbytes && len > maxbytes)
 		len = maxbytes;
 
-	DEBUG4("(READ from fd %d offset %Lu len %u), ", fhandle, foffset, len);
+	DEBUG4("(READ from fd %d offset %llu len %u), ", fhandle, foffset, len);
 
 	myseek(fhandle, foffset);
 	return read(fhandle, buf, len);
@@ -971,7 +971,7 @@
 
 	if (!(client->server->flags & F_COPYONWRITE))
 		return(rawexpread_fully(a, buf, len, client));
-	DEBUG3("Asked to read %d bytes at %Lu.\n", len, (unsigned long long)a);
+	DEBUG3("Asked to read %d bytes at %llu.\n", len, (unsigned long long)a);
 
 	mapl=a/DIFFPAGESIZE; maph=(a+len-1)/DIFFPAGESIZE;
 
@@ -981,12 +981,12 @@
 		rdlen=(0<DIFFPAGESIZE-offset && len<(size_t)(DIFFPAGESIZE-offset)) ?
 			len : (size_t)DIFFPAGESIZE-offset;
 		if (client->difmap[mapcnt]!=(u32)(-1)) { /* the block is already there */
-			DEBUG3("Page %Lu is at %lu\n", (unsigned long long)mapcnt,
+			DEBUG3("Page %llu is at %lu\n", (unsigned long long)mapcnt,
 			       (unsigned long)(client->difmap[mapcnt]));
 			myseek(client->difffile, client->difmap[mapcnt]*DIFFPAGESIZE+offset);
 			if (read(client->difffile, buf, rdlen) != rdlen) return -1;
 		} else { /* the block is not there */
-			DEBUG2("Page %Lu is not here, we read the original one\n",
+			DEBUG2("Page %llu is not here, we read the original one\n",
 			       (unsigned long long)mapcnt);
 			if(rawexpread_fully(a, buf, rdlen, client)) return -1;
 		}
@@ -1015,7 +1015,7 @@
 
 	if (!(client->server->flags & F_COPYONWRITE))
 		return(rawexpwrite_fully(a, buf, len, client)); 
-	DEBUG3("Asked to write %d bytes at %Lu.\n", len, (unsigned long long)a);
+	DEBUG3("Asked to write %d bytes at %llu.\n", len, (unsigned long long)a);
 
 	mapl=a/DIFFPAGESIZE ; maph=(a+len-1)/DIFFPAGESIZE ;
 
@@ -1026,7 +1026,7 @@
 			len : (size_t)DIFFPAGESIZE-offset;
 
 		if (client->difmap[mapcnt]!=(u32)(-1)) { /* the block is already there */
-			DEBUG3("Page %Lu is at %lu\n", (unsigned long long)mapcnt,
+			DEBUG3("Page %llu is at %lu\n", (unsigned long long)mapcnt,
 			       (unsigned long)(client->difmap[mapcnt])) ;
 			myseek(client->difffile,
 					client->difmap[mapcnt]*DIFFPAGESIZE+offset);
@@ -1034,7 +1034,7 @@
 		} else { /* the block is not there */
 			myseek(client->difffile,client->difffilelen*DIFFPAGESIZE) ;
 			client->difmap[mapcnt]=(client->server->flags&F_SPARSE)?mapcnt:client->difffilelen++;
-			DEBUG3("Page %Lu is not here, we put it at %lu\n",
+			DEBUG3("Page %llu is not here, we put it at %lu\n",
 			       (unsigned long long)mapcnt,
 			       (unsigned long)(client->difmap[mapcnt]));
 			rdlen=DIFFPAGESIZE ;
@@ -1134,7 +1134,7 @@
 		if (len > BUFSIZE + sizeof(struct nbd_reply))
 			err("Request too big!");
 #ifdef DODBG
-		printf("%s from %Lu (%Lu) len %d, ", request.type ? "WRITE" :
+		printf("%s from %llu (%llu) len %d, ", request.type ? "WRITE" :
 				"READ", (unsigned long long)request.from,
 				(unsigned long long)request.from / 512, len);
 #endif
@@ -1257,7 +1257,7 @@
 		client->exportsize = client->server->expected_size;
 	}
 
-	msg3(LOG_INFO, "Size of exported file/device is %Lu", (unsigned long long)client->exportsize);
+	msg3(LOG_INFO, "Size of exported file/device is %llu", (unsigned long long)client->exportsize);
 	if(multifile) {
 		msg3(LOG_INFO, "Total number of files: %d", i);
 	}

===================

Modifying my previous email's (and tarball's) cygwin instructions for
your above improved version, assuming all above patches applied and
instructions followed (i.e., use nbd.h as I described and fight the
muddy fight with autoconf):

Basically, all that's left is hand-holding:

0.  Obviously, every time ./configure says something that is necessary
    is missing, go get it!  I have a healthy cygwin installation (that
    means a lot of stuff is present).  If you don't know enough to
    select the right cygwin packages, then you are brave indeed!  To
    such a brave soul, go and install EVERY cygwin package that might
    be used, and you'll be just fine.  (I don't; no room, and no time
    for all the insane conflicts, which I've never seen so maybe there
    aren't any.)  (I've since installed much more, and found that
    cygwin actually says it's OK to install everything; you can just
    do that.)

    Be sure to include glib2-devel.  I have no idea what it is, but
    they say nbd needs it, and indeed it does (I tried without it,
    since I have a no-Gnome policy).

1.  Please remember to read the README for configuration file options.  
    You can copy the sample config file inside the README as
    "CONFDIR/nbd-server/config.sample", and copy that to config and
    edit it, where CONFDIR is /etc or /usr/local/etc or whereever
    (do "strings nbd-server.exe|egrep /config" to find out where it
    goes).

2.  During my tests, my file to serve was actually a normal file
    inside an NTFS filesystem, and then since I got it to work like
    that, I then gave it its own partition (which was my goal all
    along, using the kindness of "lvm pvmove").  Note that cygwin
    assigns primary and logical/extended partitions all to
    /dev/sd[a-l][0-15] or such, and they all work fine, except they
    won't work with partition type 8e; I had to change the partition
    type to 83 to make it work (originally I thought maybe cygwin only
    looked at "primary" partitions, but that's not true; they can
    definately be "logical"; partition TYPE codes are all that matter,
    probably to XP, which then passes its problems on to cygwin).
    Since Linux doesn't pay attention to partition type, this was a
    trivial decision for me.  (It actually contains LUKS data, so I
    don't care at all anyway.)  Note that cygwin also allows whole
    discs, such as /dev/sd[a-z], so with the partition handling
    patches for the Linux kernel's NBD that I just saw in the mailing
    list, you could bypass that partition type setting step that I
    did, and avoid another XP reboot (although you'd have to patch and
    reboot your client kernel).  Life is wonderful again.

3.  As I mentioned above, don't forget to poke a hole in Windows
    Firewall; clicking "Unblock" on the popup is insufficient.  Then,
    put appropriate firewall settings in your firewalls to not allow
    illicit uses of those ports.  (Otherwise, everybody has instant
    root access to all of your nbd-accessible data, without any
    password at all.)  One thing I do, as I mentioned, is use LUKS
    partitions.  Yes, if they got past the firewall, they could WRITE
    to my partitions, but they couldn't make it work as a LUKS
    partition without my password (which they'd be closer to, since
    they could read the encrypted data and then start brute-forcing
    it, but that's it).  So, even though breaching the firewall would
    allow a DOS (denial or service -- clobbering my data), they
    couldn't as easily steal or modify runnable data.  To make it
    really secure, nbd-server ought to have a password settable in the
    config file that nbd-client must use, which it requires upon
    negotiation.  That should be trivial for anybody to add that knows
    a small modicum of C -- it's a splendidly simple program.  I
    already put a password in for nbd-client to make sure no one is
    masquerading as an illicit nbd-server, which also ought to be done
    (nbd-server isn't to be trusted, either, right?).

Brad Allen
<ulmo@...205...>



Reply to: