[Libguestfs] [libnbd PATCH 1/2] sockets: Add .shut_writes callback

Eric Blake eblake at redhat.com
Mon Mar 30 19:59:02 UTC 2020


The next commit wants to teach libnbd to ensure that a server will
read EOF from a bi-directional connection as soon as libnbd is done
writing, rather than making libnbd dependent on the server being the
connection endpoint to initiate closing the socket or waiting on the
user to eventually call nbd_close().  For that to work, we need to
expose the equivalent of shutdown(SHUT_WR) for our various connection
types.

Note that shutdown() is only applicable to sockets, since it only
really aids in bi-directional communication on a single fd.  For now,
libnbd only uses sockets (even for nbd_connect_command, we use
socketpair() to pass a single fd to both stdin and stdout, rather than
two pipe() calls to create a pair of one-way communication).  But if
we were to ever support NBD connections over a pair of pipe()s, the
obvious implementation would be to close() the write pipe during
.shut_writes, while leaving the read pipe open until .close.

Also note that shutdown() is effectively non-blocking, but that
gnutls_bye() can fail with GNUTLS_E_AGAIN if it is waiting for space
to send notification to the server.  But at the same time, while
socket read and write normally distinguish between success, EAGAIN,
and other failures, we don't need the tri-state return from our new
hook (we will only ever attempt to shut down writes after sending
NBD_CMD_DISC, and moving to CLOSED instead of DEAD at that point
doesn't change the results of any earlier requests).

Finally, note that the semantics of this new callback are specifically
only for SHUT_WR, and not generic enough for SHUT_RDWR.  This is
because gnutls_bye() handles SHUT_WR with only writes to the socket,
but when handling SHUT_RDWR it also waits for a subsequent read.
Getting the ping-pong I/O correct would require more states, for no
real gain when the user can just blindly call nbd_close() and close
the entire socket rather than having to massage their poll() loop
through more states.
---
 lib/internal.h |  3 ++-
 lib/crypto.c   | 21 +++++++++++++++++----
 lib/socket.c   | 12 +++++++++++-
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/lib/internal.h b/lib/internal.h
index 6eb50d8..0d02687 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -1,5 +1,5 @@
 /* nbd client library in userspace: internal definitions
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -267,6 +267,7 @@ struct socket_ops {
                    struct socket *sock, const void *buf, size_t len, int flags);
   bool (*pending) (struct socket *sock);
   int (*get_fd) (struct socket *sock);
+  bool (*shut_writes) (struct nbd_handle *h, struct socket *sock);
   int (*close) (struct socket *sock);
 };

diff --git a/lib/crypto.c b/lib/crypto.c
index 3e33af7..516651c 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -1,5 +1,5 @@
 /* NBD client library in userspace
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -230,9 +230,21 @@ tls_get_fd (struct socket *sock)
   return sock->u.tls.oldsock->ops->get_fd (sock->u.tls.oldsock);
 }

-/* XXX Calling gnutls_bye is possible, but it may send and receive
- * data over the wire which would require modifications to the state
- * machine.  So instead we abruptly drop the TLS session.
+static bool
+tls_shut_writes (struct nbd_handle *h, struct socket *sock)
+{
+  int r = gnutls_bye (sock->u.tls.session, GNUTLS_SHUT_WR);
+
+  if (r == GNUTLS_E_AGAIN || r == GNUTLS_E_INTERRUPTED)
+    return false;
+  if (r != 0)
+    debug (h, "ignoring gnutls_bye failure: %s", gnutls_strerror (r));
+  return sock->u.tls.oldsock->ops->shut_writes (h, sock->u.tls.oldsock);
+}
+
+/* XXX Calling gnutls_bye(GNUTLS_SHUT_RDWR) is possible, but it may send
+ * and receive data over the wire which would require further modifications
+ * to the state machine.  So instead we abruptly drop the TLS session.
  */
 static int
 tls_close (struct socket *sock)
@@ -254,6 +266,7 @@ static struct socket_ops crypto_ops = {
   .send = tls_send,
   .pending = tls_pending,
   .get_fd = tls_get_fd,
+  .shut_writes = tls_shut_writes,
   .close = tls_close,
 };

diff --git a/lib/socket.c b/lib/socket.c
index ac60286..6698941 100644
--- a/lib/socket.c
+++ b/lib/socket.c
@@ -1,5 +1,5 @@
 /* NBD client library in userspace
- * Copyright (C) 2013-2019 Red Hat Inc.
+ * Copyright (C) 2013-2020 Red Hat Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -63,6 +63,15 @@ socket_get_fd (struct socket *sock)
   return sock->u.fd;
 }

+static bool
+socket_shut_writes (struct nbd_handle *h, struct socket *sock)
+{
+  if (shutdown (sock->u.fd, SHUT_WR) == -1)
+    debug (h, "ignoring shutdown failure: %s", strerror (errno));
+  /* Regardless of any errors, we don't need to retry. */
+  return true;
+}
+
 static int
 socket_close (struct socket *sock)
 {
@@ -76,6 +85,7 @@ static struct socket_ops socket_ops = {
   .recv = socket_recv,
   .send = socket_send,
   .get_fd = socket_get_fd,
+  .shut_writes = socket_shut_writes,
   .close = socket_close,
 };

-- 
2.26.0.rc2




More information about the Libguestfs mailing list