Re: [Libguestfs] [nbdkit PATCH] nbd: Add new nbd forwarding plugin
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(a)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
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)