[PATCH] Add optional nbd:// io space support

[Eric Blake]

This uses libnbd to access any NBD server as an io space.  Tests will
come in the next patch.

* configure.ac (PKG_CHECK_MODULES): Check for libnbd.
* src/ios-dev-nbd.c: New file.
* src/Makefile.am (poke_SOURCES): Optionally build it.
* src/ios.c (ios_dev_ifs): Expose it through 'open'.
* src/pk-ios.c (pk_cmd_nbd): Add .nbd command.
* src/pk-cmd.c (dot_cmds): Load it.
* doc/poke.texi (nbd command): New section.
(open): Document nbd handler.
* HACKING (libnbd): Mention it.
---

I'm still working on implementing a custom dg-nbd command plus a
capability in the testsuite to skip tests that utilize NBD if libnbd
was not compiled into poke and nbdkit is not available (hmm, should
poke have some way to introspect what optional features were compiled
in?), otherwise to use nbdkit to spawn a temporary NBD server on a
Unix socket for use in testing this.  In the meantime, I'm
publishing work prone to rewinds on the eblake/nbd branch.

To test this, you can do things like:
$ qemu-nbd -k /tmp/socket -f qcow2 image.qcow2
$ ./run poke
(poke) open("nbd:///?socket=/tmp/socket")

or:
$ ./run nbdkit memory 1M --run poke
(poke) open("nbd://localhost:10809")

as samples of connecting to an NBD io space.  Noticed while writing
this patch: poke could really benefit from using pread/pwrite rather
than lseek/read or lseek/write (fewer syscalls is better performance,
plus NBD doesn't natively track a current offset, but you don't need a
current offset with pread).  Also, doing I/O one byte at a time has a
lot of overhead (28 bytes sent and 21 received plus TCP overhead per
byte when using NBD one byte at a time), we really ought to improve
the IOS layer to support buffer reads.

 ChangeLog         |  13 +++
 HACKING           |  33 +++++---
 configure.ac      |  11 +++
 doc/poke.texi     |  45 +++++++++-
 src/Makefile.am   |   6 +-
 src/ios-dev-nbd.c | 207 ++++++++++++++++++++++++++++++++++++++++++++++
 src/ios.c         |   7 ++
 src/pk-cmd.c      |   2 +
 src/pk-ios.c      |  42 ++++++++++
 9 files changed, 352 insertions(+), 14 deletions(-)
 create mode 100644 src/ios-dev-nbd.c

diff --git a/ChangeLog b/ChangeLog
index 44c45f35..2888db84 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2020-02-25  Eric Blake  <address@hidden>
+
+       Add optional nbd:// io space support
+       * configure.ac (PKG_CHECK_MODULES): Check for libnbd.
+       * src/ios-dev-nbd.c: New file.
+       * src/Makefile.am (poke_SOURCES): Optionally build it.
+       * src/ios.c (ios_dev_ifs): Expose it through 'open'.
+       * src/pk-ios.c (pk_cmd_nbd): Add .nbd command.
+       * src/pk-cmd.c (dot_cmds): Load it.
+       * doc/poke.texi (nbd command): New section.
+       (open): Document nbd handler.
+       * HACKING (libnbd): Mention it.
+
 2020-02-25  Eric Blake  <address@hidden>

        * .gitignore: Ignore more testsuite leftovers.
diff --git a/HACKING b/HACKING
index ed8f46ab..2a62a8ec 100644
--- a/HACKING
+++ b/HACKING
@@ -44,22 +44,24 @@ along with GNU poke.  If not, see 
<https://www.gnu.org/licenses/>.
        3. 5  Boehm GC
        3. 6  Jitter
        3. 7  libtextstyle
-       3. 8  Building
-       3. 9  Building a 32-bit poke
-       3.10  Gettext
-       3.11  Running an Uninstalled Poke
-       3.12  Continuous Integration
+       3. 8  libnbd
+       3. 9  Building
+       3.10  Building a 32-bit poke
+       3.11  Gettext
+       3.12  Running an Uninstalled Poke
+       3.13  Continuous Integration
      4  Coding Style and Conventions
        4.1  Writing C
        4.2  Writing Poke
        4.3  Writing RAS
      5  Writing Tests
-       5.1  Naming Tests
-       5.2  Always set obase
-       5.3  Put each test in its own file
-       5.4  dg-output may require a newline
-       5.5  Using data files in tests
-       5.6  Writing tests that depend on a certain capability
+       5.1  Test framework
+       5.2  Naming Tests
+       5.3  Always set obase
+       5.4  Put each test in its own file
+       5.5  dg-output may require a newline
+       5.6  Using data files in tests
+       5.7  Writing tests that depend on a certain capability
      6  Fuzzing poke
        6.1  Grammarinator
      7  Deciding on What to Work on
@@ -265,6 +267,15 @@ instead... that does not do any styling!
 At the moment libtextstyle lives in a subdirectory of GNU gettext.
 See http://www.gnu.org/s/gettext for more information.

+libnbd
+~~~~~~
+
+GNU poke optionally uses libnbd to expose an io space for data served
+by an arbitrary NBD (Network Block Device) server.  The package names are:
+  - On Red Hat distributions: libnbd-devel
+
+See http://libguestfs.org/libnbd.3.html for more information.
+
 Building
 ~~~~~~~~

diff --git a/configure.ac b/configure.ac
index 5e27d01a..7fd21f39 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,6 +82,17 @@ dnl Jitter

 AC_JITTER_SUBPACKAGE([jitter])

+dnl libnbd for nbd:// io spaces (optional)
+PKG_CHECK_MODULES([LIBNBD], [libnbd], [
+  AC_SUBST([LIBNBD_CFLAGS])
+  AC_SUBST([LIBNBD_LIBS])
+  AC_DEFINE([HAVE_LIBNBD], [1], [libnbd found at compile time])
+  libnbd_enabled=yes
+], [libnbd_enabled=no
+  AC_MSG_WARN([libnbd not found, nbd:// io spaces will be disabled.])
+])
+AM_CONDITIONAL([NBD], [test "x$libnbd_enabled" = "xyes"])
+
 dnl Used in Makefile.am.  See the note there.
 WITH_JITTER=$with_jitter
 AC_SUBST([WITH_JITTER])
diff --git a/doc/poke.texi b/doc/poke.texi
index 6951db15..43279c54 100644
--- a/doc/poke.texi
+++ b/doc/poke.texi
@@ -68,6 +68,7 @@ Top
 * load command::               Loading pickles.
 * file command::               Opening and selecting file IO spaces.
 * mem command::                        Opening and selecting memory IO spaces.
+* nbd command::                        Opening and selecting NBD IO spaces.
 * ios command::                        Switching between IO spaces.
 * close command::              Closing IO spaces.
 * editor command::             Using an external editor for input.
@@ -553,6 +554,40 @@ mem command
 When a new memory buffer IOS is opened it becomes the current IO
 space.  @xref{file command}.

+@node nbd command
+@chapter @code{.nbd}
+@cindex @code{.nbd}
+@cindex opening NBD buffers
+@cindex IO space
+The @command{.nbd} command opens a new IO space backed by an external
+NBD server.  The syntax is:
+
+@example
+.nbd @var{uri}
+@end example
+
+Where @var{uri} is the name of the newly created buffer.  @var{uri}
+should match the
+@url{https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md,
+NBD URI specification}.
+
+When a new NBD IOS is opened, it becomes the current IO
+space.  @xref{file command}.
+
+NBD support in GNU poke is optional, depending on whether poke was
+compiled against @url{http://libguestfs.org/libnbd.3.html,, libnbd}.
+
+For an example of connecting to the guest-visible content of a qcow2
+image, with the default export name as exposed by using qemu as an NBD
+server:
+
+@example
+$ qemu-nbd --socket=/tmp/mysock -f qcow2 image.qcow2
+$ poke
+(poke) .nbd nbd:///socket=?/tmp/mysock
+The current file is now `nbd:///socket=?/tmp/mysock'.
+@end example
+
 @node ios command
 @chapter @code{.ios}
 @cindex @code{.ios}
@@ -3364,6 +3401,9 @@ open
 The process ID of some process.
 @item /path/to/file
 An either absolute or relative path to a file.
+@item nbd://@var{host:port}/@var{export}
+@itemx nbd+unix:///@var{export}?socket=@var{/path/to/socket}
+A connection to an NBD server. @xref{nbd command}
 @end table

 @var{flags} is a bitmask that specifies several aspects of the
@@ -3384,8 +3424,9 @@ open

 @noindent
 Note that the specific meanings of these flags depend on the on the
-nature of the IO space that is opened: for example, a file can be
-truncated, but a memory buffer is truncated by default.
+nature of the IO space that is opened: for example, it is optional
+whether a file is truncated, but a memory buffer is truncated by
+default, and an NBD iospace does not support truncation.

 In order to ease the usage of @code{open}, a few pre-made bitmaps are
 provided to specify opening @dfn{modes}:
diff --git a/src/Makefile.am b/src/Makefile.am
index 07db253b..a276278e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -53,6 +53,9 @@ poke_SOURCES = poke.c poke.h \
                pkl-gen.pkc pkl-asm.pkc \
                pkl-insn.def pkl-ops.def pkl-attrs.def

+if NBD
+poke_SOURCES += ios-dev-nbd.c
+endif NBD

 if HSERVER
   poke_SOURCES += pk-hserver.h pk-hserver.c
@@ -75,8 +78,9 @@ poke_CPPFLAGS = -I$(top_builddir)/lib -I$(top_srcdir)/lib \
                 -DPKGDATADIR=\"$(pkgdatadir)\" \
                 -DJITTER_VERSION=\"$(JITTER_VERSION)\" \
                 -DLOCALEDIR=\"$(localedir)\"
-poke_CFLAGS = -Wall $(BDW_GC_CFLAGS)
+poke_CFLAGS = -Wall $(BDW_GC_CFLAGS) $(LIBNBD_CFLAGS)
 poke_LDADD = $(top_builddir)/lib/libpoke.la \
+             $(LIBNBD_LIBS) \
              $(LTLIBREADLINE) $(BDW_GC_LIBS) $(LIBTEXTSTYLE)
 poke_LDFLAGS =

diff --git a/src/ios-dev-nbd.c b/src/ios-dev-nbd.c
new file mode 100644
index 00000000..d1c0f7e0
--- /dev/null
+++ b/src/ios-dev-nbd.c
@@ -0,0 +1,207 @@
+/* ios-dev-nbd.c - NBD IO devices.  */
+
+/* Copyright (C) 2020 Eric Blake */
+
+/* This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <config.h>
+
+#include <assert.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <xalloc.h>
+
+#include <libnbd.h>
+
+#include "ios.h"
+#include "ios-dev.h"
+
+/* State associated with an NBD device.  */
+
+struct ios_dev_nbd
+{
+  struct nbd_handle *nbd;
+  char *uri;
+  ios_dev_off loc;
+  ios_dev_off size;
+  uint64_t flags;
+};
+
+static bool
+startswith (const char *str, const char *prefix)
+{
+  return strncmp (str, prefix, strlen (prefix)) == 0;
+}
+
+static int
+ios_dev_nbd_handler_p (const char *handler)
+{
+  return (startswith (handler, "nbd://")
+         || startswith (handler, "nbd+unix://"));
+}
+
+static void *
+ios_dev_nbd_open (const char *handler, uint64_t flags, int *error)
+{
+  struct ios_dev_nbd *nio = NULL;
+  struct nbd_handle *nbd = NULL;
+  uint8_t flags_mode = flags & IOS_FLAGS_MODE;
+  int err = IOD_ERROR;
+  int64_t size;
+
+  /* We don't permit truncation. */
+  if (flags_mode & IOS_F_TRUNCATE)
+    {
+      err = IOD_EINVAL;
+      goto err;
+    }
+
+  /* We have to connect before we know if server permits writes */
+  nbd = nbd_create ();
+  if (nbd == NULL)
+    goto err;
+
+  if (nbd_connect_uri (nbd, handler) == -1)
+    goto err;
+
+  if (flags_mode & IOS_F_WRITE && nbd_is_read_only (nbd))
+    {
+      err = IOD_EINVAL;
+      goto err;
+    }
+  else if (flags_mode == 0)
+    {
+      flags |= IOS_F_READ;
+      if (nbd_is_read_only (nbd) == 0)
+       flags |= IOS_F_WRITE;
+    }
+
+  size = nbd_get_size (nbd);
+  if (size < 0)
+    goto err;
+
+  nio = xmalloc (sizeof *nio);
+  nio->nbd = nbd;
+  nio->uri = xstrdup (handler);
+  nio->loc = 0;
+  nio->size = size;
+  nio->flags = flags;
+
+  return nio;
+
+ err:
+  /* Worth logging nbd_get_error () ? */
+  if (error)
+    *error = err;
+  nbd_close (nbd);
+  return NULL;
+}
+
+static int
+ios_dev_nbd_close (void *iod)
+{
+  struct ios_dev_nbd *nio = iod;
+
+  /* Should this flush when possible? */
+  nbd_close (nio->nbd);
+  free (nio->uri);
+  free (nio);
+
+  return 1;
+}
+
+static uint64_t
+ios_dev_nbd_get_flags (void *iod)
+{
+  struct ios_dev_nbd *nio = iod;
+
+  return nio->flags;
+}
+
+static int
+ios_dev_nbd_getc (void *iod)
+{
+  struct ios_dev_nbd *nio = iod;
+  uint8_t buf;
+  int ret = nbd_pread (nio->nbd, &buf, 1, nio->loc, 0);
+
+  if (ret == -1)
+    return IOD_EOF;
+  nio->loc++;
+  return buf;
+}
+
+static int
+ios_dev_nbd_putc (void *iod, int c)
+{
+  struct ios_dev_nbd *nio = iod;
+  uint8_t buf = c;
+  int ret = nbd_pwrite (nio->nbd, &buf, 1, nio->loc, 0);
+
+  if (ret == -1)
+    return IOD_EOF;
+  nio->loc++;
+  return buf;
+}
+
+static ios_dev_off
+ios_dev_nbd_tell (void *iod)
+{
+  struct ios_dev_nbd *nio = iod;
+  return nio->loc;
+}
+
+static int
+ios_dev_nbd_seek (void *iod, ios_dev_off offset, int whence)
+{
+  struct ios_dev_nbd *nio = iod;
+
+  switch (whence)
+    {
+    case IOD_SEEK_SET: break;
+    case IOD_SEEK_CUR: offset += nio->loc; break;
+    case IOD_SEEK_END: offset = nio->size - offset; break;
+    default:
+      assert (0);
+    }
+
+  if (offset >= nio->size)
+    return -1;
+  nio->loc = offset;
+  return 0;
+}
+
+static ios_dev_off
+ios_dev_nbd_size (void *iod)
+{
+  struct ios_dev_nbd *nio = iod;
+
+  return nio->size;
+}
+
+struct ios_dev_if ios_dev_nbd =
+  {
+   .handler_p = ios_dev_nbd_handler_p,
+   .open = ios_dev_nbd_open,
+   .close = ios_dev_nbd_close,
+   .tell = ios_dev_nbd_tell,
+   .seek = ios_dev_nbd_seek,
+   .get_c = ios_dev_nbd_getc,
+   .put_c = ios_dev_nbd_putc,
+   .get_flags = ios_dev_nbd_get_flags,
+   .size = ios_dev_nbd_size,
+  };
diff --git a/src/ios.c b/src/ios.c
index 2340bc88..c8696720 100644
--- a/src/ios.c
+++ b/src/ios.c
@@ -139,10 +139,17 @@ static struct ios *cur_io;

 extern struct ios_dev_if ios_dev_mem; /* ios-dev-mem.c */
 extern struct ios_dev_if ios_dev_file; /* ios-dev-file.c */
+#ifdef HAVE_LIBNBD
+extern struct ios_dev_if ios_dev_nbd; /* ios-dev-nbd.c */
+#endif

 static struct ios_dev_if *ios_dev_ifs[] =
   {
    &ios_dev_mem,
+#ifdef HAVE_LIBNBD
+   &ios_dev_nbd,
+#endif
+   /* File must be last */
    &ios_dev_file,
    NULL,
   };
diff --git a/src/pk-cmd.c b/src/pk-cmd.c
index 21227535..5b4a755a 100644
--- a/src/pk-cmd.c
+++ b/src/pk-cmd.c
@@ -44,6 +44,7 @@
 extern struct pk_cmd ios_cmd; /* pk-ios.c */
 extern struct pk_cmd file_cmd; /* pk-ios.c  */
 extern struct pk_cmd mem_cmd; /* pk-ios.c */
+extern struct pk_cmd nbd_cmd; /* pk-ios.c */
 extern struct pk_cmd close_cmd; /* pk-file.c */
 extern struct pk_cmd load_cmd; /* pk-file.c */
 extern struct pk_cmd info_cmd; /* pk-info.c  */
@@ -72,6 +73,7 @@ static struct pk_cmd *dot_cmds[] =
     &set_cmd,
     &editor_cmd,
     &mem_cmd,
+    &nbd_cmd,
     &null_cmd
   };

diff --git a/src/pk-ios.c b/src/pk-ios.c
index 08ca89f8..361fd366 100644
--- a/src/pk-ios.c
+++ b/src/pk-ios.c
@@ -330,6 +330,45 @@ pk_cmd_mem (int argc, struct pk_cmd_arg argv[], uint64_t 
uflags)
     return 1;
 }

+static int
+pk_cmd_nbd (int argc, struct pk_cmd_arg argv[], uint64_t uflags)
+{
+#ifdef HAVE_LIBNBD
+  /* nbd URI */
+
+  assert (argc == 1);
+  assert (PK_CMD_ARG_TYPE (argv[0]) == PK_CMD_ARG_STR);
+
+  /* Create a new NBD IO space.  */
+  const char *arg_str = PK_CMD_ARG_STR (argv[0]);
+  char *nbd_name = xstrdup (arg_str);
+
+  if (ios_search (nbd_name) != NULL)
+    {
+      printf (_("Buffer %s already opened.  Use `.ios #N' to switch.\n"),
+              nbd_name);
+      free (nbd_name);
+      return 0;
+    }
+
+  if (IOS_ERROR == ios_open (nbd_name, 0, 1))
+    {
+      pk_printf (_("Error creating NBD IOS %s\n"), nbd_name);
+      free (nbd_name);
+      return 0;
+    }
+
+  if (poke_interactive_p && !poke_quiet_p)
+    pk_printf (_("The current IOS is now `%s'.\n"),
+               ios_handler (ios_cur ()));
+
+  return 1;
+#else /* ! HAVE_LIBNBD */
+  pk_printf (_("This poke was compiled without NBD support\n"));
+  return 0;
+#endif
+}
+
 struct pk_cmd ios_cmd =
   {"ios", "t", "", 0, NULL, pk_cmd_ios, "ios #ID", ios_completion_function};

@@ -339,5 +378,8 @@ struct pk_cmd file_cmd =
 struct pk_cmd mem_cmd =
   {"mem", "s", "", 0, NULL, pk_cmd_mem, "mem NAME", NULL};

+struct pk_cmd nbd_cmd =
+  {"nbd", "s", "", 0, NULL, pk_cmd_nbd, "nbd URI", NULL};
+
 struct pk_cmd close_cmd =
   {"close", "?t", "", PK_CMD_F_REQ_IO, NULL, pk_cmd_close, "close [#ID]", 
ios_completion_function};

-- 
2.24.1