On 11/11/2017 08:33 PM, Eric Blake wrote:
This is a minimal implementation of an NBD forwarder; it lets us convert between old and newstyle connections (great if a client expects one style but the real server only provides the other), or add TLS safety on top of a server without having to rewrite that server. Right now, the real server is expected to live on a named Unix socket, and the transactions are serialized rather than interleaved; further enhancements could be made to also permit TCP servers or more efficient transmission.
I also envision the possibility of enhancing our testsuite to use NBD forwarding as a great test of our server.
Signed-off-by: Eric Blake <eblake@redhat.com>
+/* Read an entire buffer, returning 0 on success or -1 with errno set */ +static int read_full (int fd, void *buf, size_t len) +{ + ssize_t r; + + while (len) { + r = read (fd, buf, len); + if (r < 0) { + if (errno == EINTR || errno == EAGAIN) + continue; + return -1; + } + if (!r) { + /* Unexpected EOF */ + errno = EBADMSG;
This errno value does not travel over for the wire in NBD during transmission phase, and results in the client seeing EINVAL; however, based on how this function is sometimes called... [1]
+/* Write an entire buffer, returning 0 on success or -1 with errno set */ +static int write_full (int fd, const void *buf, size_t len) +{ + ssize_t r; + + while (len) { + r = write (fd, buf, len); + if (r < 0) { + if (errno == EINTR || errno == EAGAIN) + continue; + return -1;
Likewise when this function fails with EPIPE... [2]
+static int nbd_request (struct handle *h, uint32_t type, uint64_t offset, + uint32_t count, uint64_t *cookie) +{ + struct request req = { + .magic = htobe32 (NBD_REQUEST_MAGIC), + /* TODO nbdkit should have a way to pass flags, separate from cmd type */ + .type = htobe32 (type), + .handle = htobe64 (h->cookie), + .offset = htobe64 (offset), + .count = htobe32 (count), + }; + int r; + + /* TODO full parallel support requires tracking cookies for handling + out-of-order responses */ + *cookie = h->cookie++; + if (h->dead) { + nbdkit_set_error (ESHUTDOWN); + return -1; + } + nbdkit_debug ("sending request with type %d and cookie %" PRIu64, type, + *cookie); + r = write_full (h->fd, &req, sizeof req); + if (r < 0) + h->dead = true;
[2] ...failing with ESHUTDOWN instead of EPIPE (which turns into EINVAL)
+ return r; +} + +static int nbd_reply (struct handle *h, uint64_t cookie) +{ + struct reply rep; + + if (read_full (h->fd, &rep, sizeof rep) < 0) { + h->dead = true; + return -1;
[1] ...and failing with ESHUTDOWN instead of EBADMSG (which turns into EINVAL) make more sense from the client's point of view. I'm going to submit a v2 of this patch; I also have an idea up my sleeve for how to make things interleave (by having a dedicated reader pthread, as well as a pipe() pair per transaction to manage the handoffs between transactions issuing the request and the reader thread handling the reply) that I hope to include in my v2 series if I can get it further in my testing. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org