At this point the current nbd-server supports only 1 connection per client. For sequential reads kernel wont be able to create multiple connections and read in parallel.

It would be best if we support this at least for readonly servers, for the sake of performance.

Doubt: World is moving into a single socket based model and multiplexing, e.g. HTTP2, why can't we follow that?

Because NBD protocol do not have any alternative multiplexing mechanism. Some underlying storage layers only perform well when consumed in parallel. e.g. GCS, S3 or any blobstorage. For a large sequential IO, best performance can only be achieved if we start reading in parallel.

Probably should use a more liberally licensed library.

if Size() function of the FileBackend would be revised as below, standard block devices could be mounted successfully from client side.

func (b *FileBackend) Size() (int64, error) {
stat, err := b.file.Seek(0,2)
if err != nil {
return -1, err
}
return stat, nil
}

Just want to make you aware of the NBD URI specification. It's a simple extension of the basic NBD protocol, allowing an easy and cross-client way for NBD endpoints to be specified. eg:

nbd://localhost = NBD unencrypted over TCP to localhost:10809, default export name

nbds+unix:///default?socket=/tmp/sock = NBD encrypted over Unix domain socket /tmp/sock, export name default

Some other examples below, as well as more in the specification above.
https://libguestfs.org/nbd_connect_uri.3.html

To make go-nbd easier to use from libnbd or systemd, you should probably add support for systemd socket activation. This doesn't require or involve libnbd or systemd, it just means you parse a few environment variables in a standardized way. Many, many servers of all kinds support this protocol, eg. Apache.

In its simplest form:

• the parent process will set LISTEN_PID to your PID. Check that getpid() matches this, otherwise ignore the systemd socket activation
• the parent process will set LISTEN_FDS to some number, usually 1. Inherited file descriptors 3, 4, ... (up to the number of LISTEN_FDS) are listening sockets that you should accept on.

If this was enabled, then you'd be able to start up go-nbd either from a systemd unit or (more interesting to me) from this libnbd call: https://libguestfs.org/nbd_connect_systemd_socket_activation.3.html

Example of code that does this in nbdkit:

https://gitlab.com/nbdkit/nbdkit/-/blob/master/server/socket-activation.c?ref_type=heads

I am working on an application in which an nbd may be swapped out for another one, using a long-running client process (an agent running inside a VM).

The approach I'm thinking of using is to call client.Disconnect() then umount the nbd.

However, for my particular case, this repeated connect/disconnect approach would result in a goroutine leak, because client.Connect() never returns. It is getting stuck in the <-fatal channel recv at the end of the func, which never receives a fatal error even if I call conn.Close() and close the file descriptor with f.Close().

Is it possible to have this function return once the device is connected? (Maybe instead of the OnConnected callback, just always do the connection check, and have Connect() return a nil error if and only if the connection is established successfully. Or if it's desired to make the connection check optional, have a separate func to explicitly wait for the device to be ready. Maybe there could be separate funcs for Udev vs. polling /sys/block, like WaitForReadyUdev / WaitForReady.)

Considering the fixes of #4 (comment) is done, now the next step will be to support highly concurrent IO.

So far I have observed for any size of IO request 32MiB is allocated, which increases the memory usage unnecessarily.

As an alternative we can allocate the exact amount of the request size. (for both read and write)

---  b := make([]byte, maximumPacketSize) 
	for {
		var requestHeader protocol.TransmissionRequestHeader
...
		case protocol.TRANSMISSION_TYPE_REQUEST_READ:
			if err := binary.Write(conn, binary.BigEndian, protocol.TransmissionReplyHeader{
				ReplyMagic: protocol.TRANSMISSION_MAGIC_REPLY,
				Error:      0,
				Handle:     requestHeader.Handle,
			}); err != nil {
				return err
			}

+++			b := make([]byte, int64(requestHeader.Length))
			n, err := export.Backend.ReadAt(largeB, int64(requestHeader.Offset))

I have tested this and since linux dev mapper devices (if used on top of nbd device) makes very small io requests this has significantly reduced memory usage around 80-90% (since most IO requests do not exceed 4MiB).

The NBD protocol says ...

A special, "empty", name (i.e., the length field is zero and no name is specified), is reserved for a "default" export, to be used in cases where explicitly specifying an export name makes no sense.

I notice that this server doesn't seem to do this:

$ nbdinfo nbd://localhost/
nbdinfo: nbd_opt_go: server replied with error to opt_go request: No such file or directory for the default export
nbdinfo: suggestion: to list all exports on the server, use --list
protocol: newstyle-fixed without TLS, using simple packets

Using --list shows that there is one export literally called default:

$ nbdinfo --list nbd://localhost
protocol: newstyle-fixed without TLS, using simple packets
export="default":
	export-size: 1073741824 (1G)
	uri: nbd://localhost:10809/default
	is_rotational: false
	is_read_only: false
	can_block_status_payload: false
	can_cache: false
	can_df: false
	can_fast_zero: false
	can_flush: false
	can_fua: false
	can_multi_conn: true
	can_trim: false
	can_zero: false
	block_size_minimum: 1
	block_size_preferred: 4096 (4K)
	block_size_maximum: 4294967295

I would suggest mapping the empty string to default export as per the specification.

Currently, the server supports advertising multi-conn capabilities (see #4) , but the integrated client is not able to make use of this. If we call NBD_SET_SOCK more times than 1, we just get invalid argument when calling NBD_DO_IT.

• Hack/branch for go-ndb that starts implementing this: eb21896
• Hack/branch for r3map that starts implementing this: pojntfx/r3map@2529c4a

If we can get this to work, we'll probably be able to get very significant performance increases in related repos like r3map. Here is how the C version implements it: https://github.com/NetworkBlockDevice/nbd/blob/master/nbd-client.c#L1206-L1239

I basically ported the C version (if the whitespace diff is hidden in go-nbd's client its just +4 lines), it should be a simple enough change in theory.

I'm trying to write an automated interop test for libnbd & go-nbd.

However there seems to be know way to know when the server is ready and listening for requests. This is most obviously a problem if you have a test which does:

pid = os.fork()
if pid == 0:        # run go-nbd in the child process
    os.execvp(golang, argv)

# Connect to go-nbd in the parent process
h.connect_unix(sock)

Normally we would want to have the server write a PID file when it has finished starting up and is listening for requests (note: the PID file should be written after the listening socket is opened). In the client side of the test we would wait for this file to appear.

But as go-nbd doesn't seem to do that, there's no way to safely start the test in a race-free way.