Re: [PATCH v2 1/2] Add optional nbd:// io space support

[Jose E. Marchesi]

Hi Eric.
This part if OK for master.
Thanks!

    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.
    ---
     ChangeLog         |  13 +++
     doc/poke.texi     |  45 +++++++++-
     HACKING           |  33 +++++---
     configure.ac      |  14 ++++
     src/ios-dev-nbd.c | 209 ++++++++++++++++++++++++++++++++++++++++++++++
     src/ios.c         |   7 ++
     src/pk-cmd.c      |   6 ++
     src/pk-ios.c      |  41 +++++++++
     src/Makefile.am   |   6 +-
     9 files changed, 359 insertions(+), 15 deletions(-)
     create mode 100644 src/ios-dev-nbd.c
    
    diff --git a/ChangeLog b/ChangeLog
    index c894606e..5caff08e 100644
    --- a/ChangeLog
    +++ b/ChangeLog
    @@ -1,3 +1,16 @@
    +2020-02-27  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-27  Jose E. Marchesi  <address@hidden>
    
        * src/pkl-lex.l: Set errno to 0 before calling strtoull.
    diff --git a/doc/poke.texi b/doc/poke.texi
    index 6951db15..880dd3ad 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.
    @@ -539,7 +540,7 @@ mem command
     @cindex opening memory buffers
     @cindex IO space
     The @command{.mem} command opens a new IO space backed by a memory
    -buffer, or switches to a previously opened IOS.  The syntax is:
    +buffer.  The syntax is:
    
     @example
     .mem @var{name}
    @@ -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
    +
    +@cindex tags, file ID tags
    +Where @var{uri} is the name of the newly created buffer, matching 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+unix:///socket=?/tmp/mysock
    +The current file is now `nbd+unix:///socket=?/tmp/mysock'.
    +@end example
    +
     @node ios command
     @chapter @code{.ios}
     @cindex @code{.ios}
    @@ -3364,6 +3399,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 +3422,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/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..9e296b49 100644
    --- a/configure.ac
    +++ b/configure.ac
    @@ -82,6 +82,15 @@ 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])
    +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])
    @@ -142,6 +151,11 @@ if test "x$hserver_enabled" = "xno"; then
        echo "warning: --enable-hserver to activate it."
     fi
    
    +if test "x$libnbd_enabled" != "xyes"; then
    +  echo "warning: building poke without NBD io space support."
    +  echo "warning: install libnbd to use it."
    +fi
    +
     dnl Report errors
    
     if test "x$gl_cv_lib_readline" = "xno"; then
    diff --git a/src/ios-dev-nbd.c b/src/ios-dev-nbd.c
    new file mode 100644
    index 00000000..42b6f187
    --- /dev/null
    +++ b/src/ios-dev-nbd.c
    @@ -0,0 +1,209 @@
    +/* 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 char *
    +ios_dev_nbd_handler_normalize (const char *handler)
    +{
    +  if (startswith (handler, "nbd://")
    +      || startswith (handler, "nbd+unix://"))
    +    return xstrdup (handler);
    +  return NULL;
    +}
    +
    +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_normalize = ios_dev_nbd_handler_normalize,
    +   .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 bb760161..35507e5e 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..254c17e8 100644
    --- a/src/pk-cmd.c
    +++ b/src/pk-cmd.c
    @@ -44,6 +44,9 @@
     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 */
    +#ifdef HAVE_LIBNBD
    +extern struct pk_cmd nbd_cmd; /* pk-ios.c */
    +#endif
     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 +75,9 @@ static struct pk_cmd *dot_cmds[] =
         &set_cmd,
         &editor_cmd,
         &mem_cmd,
    +#ifdef HAVE_LIBNBD
    +    &nbd_cmd,
    +#endif
         &null_cmd
       };
    
    diff --git a/src/pk-ios.c b/src/pk-ios.c
    index 08ca89f8..6b91444d 100644
    --- a/src/pk-ios.c
    +++ b/src/pk-ios.c
    @@ -330,6 +330,42 @@ pk_cmd_mem (int argc, struct pk_cmd_arg argv[], 
uint64_t uflags)
         return 1;
     }
    
    +#ifdef HAVE_LIBNBD
    +static int
    +pk_cmd_nbd (int argc, struct pk_cmd_arg argv[], uint64_t uflags)
    +{
    +  /* 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;
    +}
    +#endif /* HAVE_LIBNBD */
    +
     struct pk_cmd ios_cmd =
       {"ios", "t", "", 0, NULL, pk_cmd_ios, "ios #ID", 
ios_completion_function};
    
    @@ -339,6 +375,11 @@ struct pk_cmd file_cmd =
     struct pk_cmd mem_cmd =
       {"mem", "s", "", 0, NULL, pk_cmd_mem, "mem NAME", NULL};
    
    +#ifdef HAVE_LIBNBD
    +struct pk_cmd nbd_cmd =
    +  {"nbd", "s", "", 0, NULL, pk_cmd_nbd, "nbd URI", NULL};
    +#endif
    +
     struct pk_cmd close_cmd =
       {"close", "?t", "", PK_CMD_F_REQ_IO, NULL, pk_cmd_close, "close [#ID]", 
ios_completion_function};
    
    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 =