Libguestfs - Mar 2013 - [PATCH] lib: Add direct support for the NBD (Network Block Device) protocol.

From: "Richard W.M. Jones" <rjones at redhat.com>

You can now add remote NBD drives using:

 ><fs> add-drive "" format:raw protocol:nbd server:localhost

(Note that you also need to add port:NNNN if the server is running on
a non-standard port).

The corresponding qemu-nbd service can be started by doing:

 qemu-nbd disk.img -t

This commit also adds a test.
---
 Makefile.am            |   1 +
 README                 |   2 +
 configure.ac           |   1 +
 generator/actions.ml   |  36 +++++-
 src/drives.c           | 302 ++++++++++++++++++++++++++++++++++++-------------
 src/guestfs-internal.h |  19 +++-
 src/guestfs.pod        |  35 ++++++
 src/launch-appliance.c |  81 +++++++------
 src/launch-libvirt.c   | 259 +++++++++++++++++++++++++++++-------------
 tests/nbd/Makefile.am  |  26 +++++
 tests/nbd/test-nbd.pl  |  78 +++++++++++++
 11 files changed, 644 insertions(+), 196 deletions(-)
 create mode 100644 tests/nbd/Makefile.am
 create mode 100755 tests/nbd/test-nbd.pl

diff --git a/Makefile.am b/Makefile.am
index 1bb5ce0..9c649a9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -56,6 +56,7 @@ SUBDIRS += tests/rsync
 SUBDIRS += tests/bigdirs
 SUBDIRS += tests/disk-labels
 SUBDIRS += tests/hotplug
+SUBDIRS += tests/nbd
 SUBDIRS += tests/regressions
 endif
 
diff --git a/README b/README
index 4f2fb7f..5284d58 100644
--- a/README
+++ b/README
@@ -155,6 +155,8 @@ The full requirements are described below.
 +--------------+-------------+---+-----------------------------------------+
 | static glibc |             | O | Used for testing only.                  |
 +--------------+-------------+---+-----------------------------------------+
+| qemu-nbd     |             | O | Used for testing only.                  |
++--------------+-------------+---+-----------------------------------------+
 | findlib      |             | O | For the OCaml bindings.                 |
 +--------------+-------------+---+-----------------------------------------+
 | ocaml-gettext|             | O | For localizing OCaml virt-* tools.      |
diff --git a/configure.ac b/configure.ac
index 7059867..64b4513 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1574,6 +1574,7 @@ AC_CONFIG_FILES([Makefile
                  tests/md/Makefile
                  tests/mount-local/Makefile
                  tests/mountable/Makefile
+                 tests/nbd/Makefile
                  tests/ntfsclone/Makefile
                  tests/parallel/Makefile
                  tests/protocol/Makefile
diff --git a/generator/actions.ml b/generator/actions.ml
index 958c606..30e2482 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -1247,7 +1247,7 @@ not all belong to a single logical operating system
 
   { defaults with
     name = "add_drive";
-    style = RErr, [String "filename"], [OBool "readonly";
OString "format"; OString "iface"; OString "name";
OString "label"];
+    style = RErr, [String "filename"], [OBool "readonly";
OString "format"; OString "iface"; OString "name";
OString "label"; OString "protocol"; OString
"server"; OInt "port"];
     once_had_no_optargs = true;
     blocking = false;
     fish_alias = ["add"];
@@ -1319,6 +1319,40 @@ the drive will also be named
C</dev/disk/guestfs/I<label>>.
 
 See L<guestfs(3)/DISK LABELS>.
 
+=item C<protocol>
+
+The optional protocol argument can be used to select an alternate
+source protocol:
+
+=over 4
+
+=item C<protocol = \"file\">
+
+C<filename> is interpreted as a local file or device.
+This is the default if the optional protocol parameter
+is omitted.
+
+=item C<protocol = \"nbd\">
+
+Connect to the Network Block Device server at C<server:port>.
+
+See: L<guestfs(3)/NETWORK BLOCK DEVICES>.
+
+=back
+
+=item C<server>
+
+For protocols which require access to a remote server, this
+is the name of the server.
+
+=item C<port>
+
+For protocols which require access to a remote server, this
+is the port number of the service.
+
+If not specified, this defaults to the standard port for
+the protocol, eg. 10809 when C<protocol> is C<\"nbd\">.
+
 =back" };
 
   { defaults with
diff --git a/src/drives.c b/src/drives.c
index d082193..c3199e8 100644
--- a/src/drives.c
+++ b/src/drives.c
@@ -30,6 +30,8 @@
 #include <inttypes.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <netdb.h>
+#include <arpa/inet.h>
 #include <assert.h>
 
 #include "c-ctype.h"
@@ -41,21 +43,51 @@
 
 /* Create and free the 'drive' struct. */
 static struct drive *
-create_drive_struct (guestfs_h *g, const char *path,
-                     bool readonly, const char *format,
-                     const char *iface, const char *name,
-                     const char *disk_label,
-                     bool use_cache_none)
+create_drive_file (guestfs_h *g, const char *path,
+                   bool readonly, const char *format,
+                   const char *iface, const char *name,
+                   const char *disk_label,
+                   bool use_cache_none)
 {
   struct drive *drv = safe_malloc (g, sizeof (struct drive));
 
-  drv->path = safe_strdup (g, path);
+  drv->protocol = drive_protocol_file;
+  drv->u.path = safe_strdup (g, path);
+
   drv->readonly = readonly;
   drv->format = format ? safe_strdup (g, format) : NULL;
   drv->iface = iface ? safe_strdup (g, iface) : NULL;
   drv->name = name ? safe_strdup (g, name) : NULL;
   drv->disk_label = disk_label ? safe_strdup (g, disk_label) : NULL;
   drv->use_cache_none = use_cache_none;
+
+  drv->priv = drv->free_priv = NULL;
+
+  return drv;
+}
+
+static struct drive *
+create_drive_nbd (guestfs_h *g,
+                  const char *server, int port, const char *exportname,
+                  bool readonly, const char *format,
+                  const char *iface, const char *name,
+                  const char *disk_label,
+                  bool use_cache_none)
+{
+  struct drive *drv = safe_malloc (g, sizeof (struct drive));
+
+  drv->protocol = drive_protocol_nbd;
+  drv->u.nbd.server = safe_strdup (g, server);
+  drv->u.nbd.port = port;
+  drv->u.nbd.exportname = safe_strdup (g, exportname);
+
+  drv->readonly = readonly;
+  drv->format = format ? safe_strdup (g, format) : NULL;
+  drv->iface = iface ? safe_strdup (g, iface) : NULL;
+  drv->name = name ? safe_strdup (g, name) : NULL;
+  drv->disk_label = disk_label ? safe_strdup (g, disk_label) : NULL;
+  drv->use_cache_none = use_cache_none;
+
   drv->priv = drv->free_priv = NULL;
 
   return drv;
@@ -70,9 +102,9 @@ create_drive_struct (guestfs_h *g, const char *path,
  * a null drive.
  */
 static struct drive *
-create_null_drive_struct (guestfs_h *g, bool readonly, const char *format,
-                          const char *iface, const char *name,
-                          const char *disk_label)
+create_drive_dev_null (guestfs_h *g, bool readonly, const char *format,
+                       const char *iface, const char *name,
+                       const char *disk_label)
 {
   CLEANUP_FREE char *tmpfile = NULL;
   int fd = -1;
@@ -107,30 +139,82 @@ create_null_drive_struct (guestfs_h *g, bool readonly,
const char *format,
     return NULL;
   }
 
-  return create_drive_struct (g, tmpfile, readonly, format, iface, name,
-                              disk_label, 0);
+  return create_drive_file (g, tmpfile, readonly, format, iface, name,
+                            disk_label, 0);
 }
 
 static struct drive *
-create_dummy_drive_struct (guestfs_h *g)
+create_drive_dummy (guestfs_h *g)
 {
   /* A special drive struct that is used as a dummy slot for the appliance. */
-  return create_drive_struct (g, "", 0, NULL, NULL, NULL, NULL, 0);
+  return create_drive_file (g, "", 0, NULL, NULL, NULL, NULL, 0);
 }
 
 static void
 free_drive_struct (struct drive *drv)
 {
-  free (drv->path);
+  switch (drv->protocol) {
+  case drive_protocol_file:
+    free (drv->u.path);
+    break;
+  case drive_protocol_nbd:
+    free (drv->u.nbd.server);
+    free (drv->u.nbd.exportname);
+    break;
+  default:
+    abort ();
+  }
+
   free (drv->format);
   free (drv->iface);
   free (drv->name);
   free (drv->disk_label);
+
   if (drv->priv && drv->free_priv)
     drv->free_priv (drv->priv);
+
   free (drv);
 }
 
+/* Convert a struct drive to a string for debugging.  The caller
+ * must free this string.
+ */
+static char *
+drive_to_string (guestfs_h *g, const struct drive *drv)
+{
+  CLEANUP_FREE char *p = NULL;
+
+  switch (drv->protocol) {
+  case drive_protocol_file:
+    p = safe_asprintf (g, "path=%s", drv->u.path);
+    break;
+  case drive_protocol_nbd:
+    if (STREQ (drv->u.nbd.exportname, ""))
+      p = safe_asprintf (g, "nbd=%s:%d", drv->u.nbd.server,
drv->u.nbd.port);
+    else
+      p = safe_asprintf (g, "nbd=%s:%d:exportname=%s",
+                         drv->u.nbd.server, drv->u.nbd.port,
+                         drv->u.nbd.exportname);
+    break;
+  default:
+    abort ();
+  }
+
+  return safe_asprintf
+    (g, "%s%s%s%s%s%s%s%s%s%s%s",
+     p,
+     drv->readonly ? " readonly" : "",
+     drv->format ? " format=" : "",
+     drv->format ? : "",
+     drv->iface ? " iface=" : "",
+     drv->iface ? : "",
+     drv->name ? " name=" : "",
+     drv->name ? : "",
+     drv->disk_label ? " label=" : "",
+     drv->disk_label ? : "",
+     drv->use_cache_none ? " cache=none" : "");
+}
+
 /* Add struct drive to the g->drives vector at the given index. */
 static void
 add_drive_to_handle_at (guestfs_h *g, struct drive *d, size_t drv_index)
@@ -162,7 +246,7 @@ guestfs___add_dummy_appliance_drive (guestfs_h *g)
 {
   struct drive *drv;
 
-  drv = create_dummy_drive_struct (g);
+  drv = create_drive_dummy (g);
   add_drive_to_handle (g, drv);
 }
 
@@ -183,6 +267,48 @@ guestfs___free_drives (guestfs_h *g)
   g->nr_drives = 0;
 }
 
+/* The low-level function that adds a drive. */
+static int
+add_drive (guestfs_h *g, struct drive *drv)
+{
+  size_t i, drv_index;
+
+  if (g->state == CONFIG) {
+    /* Not hotplugging, so just add it to the handle. */
+    add_drive_to_handle (g, drv);
+    return 0;
+  }
+
+  /* ... else, hotplugging case. */
+  if (!g->attach_ops || !g->attach_ops->hot_add_drive) {
+    error (g, _("the current attach-method does not support hotplugging
drives"));
+    return -1;
+  }
+
+  if (!drv->disk_label) {
+    error (g, _("'label' is required when hotplugging
drives"));
+    return -1;
+  }
+
+  /* Get the first free index, or add it at the end. */
+  drv_index = g->nr_drives;
+  for (i = 0; i < g->nr_drives; ++i)
+    if (g->drives[i] == NULL)
+      drv_index = i;
+
+  /* Hot-add the drive. */
+  if (g->attach_ops->hot_add_drive (g, drv, drv_index) == -1)
+    return -1;
+
+  add_drive_to_handle_at (g, drv, drv_index);
+
+  /* Call into the appliance to wait for the new drive to appear. */
+  if (guestfs_internal_hot_add_drive (g, drv->disk_label) == -1)
+    return -1;
+
+  return 0;
+}
+
 /* cache=none improves reliability in the event of a host crash.
  *
  * However this option causes qemu to try to open the file with
@@ -263,46 +389,35 @@ valid_disk_label (const char *str)
   return 1;
 }
 
-/* The low-level function that adds a drive. */
+/* Check the server (hostname) is reasonable. */
 static int
-add_drive (guestfs_h *g, struct drive *drv)
+valid_server (const char *str)
 {
-  size_t i, drv_index;
+  size_t len = strlen (str);
 
-  if (g->state == CONFIG) {
-    /* Not hotplugging, so just add it to the handle. */
-    add_drive_to_handle (g, drv);
+  if (len == 0 || len > 255)
     return 0;
-  }
-
-  /* ... else, hotplugging case. */
-  if (!g->attach_ops || !g->attach_ops->hot_add_drive) {
-    error (g, _("the current attach-method does not support hotplugging
drives"));
-    return -1;
-  }
 
-  if (!drv->disk_label) {
-    error (g, _("'label' is required when hotplugging
drives"));
-    return -1;
+  while (len > 0) {
+    char c = *str++;
+    len--;
+    if (!c_isalnum (c) &&
+        c != '-' && c != '.' && c !=
':' && c != '[' && c != ']')
+      return 0;
   }
+  return 1;
+}
 
-  /* Get the first free index, or add it at the end. */
-  drv_index = g->nr_drives;
-  for (i = 0; i < g->nr_drives; ++i)
-    if (g->drives[i] == NULL)
-      drv_index = i;
-
-  /* Hot-add the drive. */
-  if (g->attach_ops->hot_add_drive (g, drv, drv_index) == -1)
-    return -1;
-
-  add_drive_to_handle_at (g, drv, drv_index);
-
-  /* Call into the appliance to wait for the new drive to appear. */
-  if (guestfs_internal_hot_add_drive (g, drv->disk_label) == -1)
-    return -1;
+static int
+nbd_port (void)
+{
+  struct servent *servent;
 
-  return 0;
+  servent = getservbyname ("nbd", "tcp");
+  if (servent)
+    return ntohs (servent->s_port);
+  else
+    return 10809;
 }
 
 int
@@ -314,6 +429,9 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename,
   const char *iface;
   const char *name;
   const char *disk_label;
+  const char *protocol;
+  const char *server;
+  int port;
   int use_cache_none;
   struct drive *drv;
 
@@ -324,15 +442,27 @@ guestfs__add_drive_opts (guestfs_h *g, const char
*filename,
   }
 
   readonly = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_READONLY_BITMASK
-             ? optargs->readonly : false;
+    ? optargs->readonly : false;
   format = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_FORMAT_BITMASK
-           ? optargs->format : NULL;
+    ? optargs->format : NULL;
   iface = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_IFACE_BITMASK
-          ? optargs->iface : NULL;
+    ? optargs->iface : NULL;
   name = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_NAME_BITMASK
-         ? optargs->name : NULL;
+    ? optargs->name : NULL;
   disk_label = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_LABEL_BITMASK
-         ? optargs->label : NULL;
+    ? optargs->label : NULL;
+  protocol = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_PROTOCOL_BITMASK
+    ? optargs->protocol : "file";
+  server = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_SERVER_BITMASK
+    ? optargs->server : NULL;
+  if (optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_PORT_BITMASK) {
+    port = optargs->port;
+    if (port <= 0 || port > 65535) {
+      error (g, _("invalid port number"));
+      return -1;
+    }
+  } else
+    port = 0;
 
   if (format && !valid_format_iface (format)) {
     error (g, _("%s parameter is empty or contains disallowed
characters"),
@@ -348,28 +478,50 @@ guestfs__add_drive_opts (guestfs_h *g, const char
*filename,
     error (g, _("label parameter is empty, too long, or contains
disallowed characters"));
     return -1;
   }
+  if (server && !valid_server (server)) {
+    error (g, _("server parameter is empty, too long, or contains
disallowed characters"));
+    return -1;
+  }
 
-  if (STREQ (filename, "/dev/null"))
-    drv = create_null_drive_struct (g, readonly, format, iface, name,
-                                    disk_label);
-  else {
-    /* For writable files, see if we can use cache=none.  This also
-     * checks for the existence of the file.  For readonly we have
-     * to do the check explicitly.
-     */
-    use_cache_none = readonly ? false : test_cache_none (g, filename);
-    if (use_cache_none == -1)
-      return -1;
-
-    if (readonly) {
-      if (access (filename, R_OK) == -1) {
-        perrorf (g, "%s", filename);
+  if (STREQ (protocol, "file")) {
+    if (STREQ (filename, "/dev/null"))
+      drv = create_drive_dev_null (g, readonly, format, iface, name,
+                                   disk_label);
+    else {
+      /* For writable files, see if we can use cache=none.  This also
+       * checks for the existence of the file.  For readonly we have
+       * to do the check explicitly.
+       */
+      use_cache_none = readonly ? false : test_cache_none (g, filename);
+      if (use_cache_none == -1)
         return -1;
+
+      if (readonly) {
+        if (access (filename, R_OK) == -1) {
+          perrorf (g, "%s", filename);
+          return -1;
+        }
       }
-    }
 
-    drv = create_drive_struct (g, filename, readonly, format, iface, name,
+      drv = create_drive_file (g, filename, readonly, format, iface, name,
                                disk_label, use_cache_none);
+    }
+  }
+  else if (STREQ (protocol, "nbd")) {
+    if (!server) {
+      error (g, _("protocol nbd: missing server name"));
+      return -1;
+    }
+    if (port == 0)
+      port = nbd_port ();
+
+    drv = create_drive_nbd (g, server, port, filename,
+                            readonly, format, iface, name,
+                            disk_label, false);
+  }
+  else {
+    error (g, _("unknown protocol '%s'"), protocol);
+    return -1;
   }
 
   if (drv == NULL)
@@ -530,17 +682,7 @@ guestfs__debug_drives (guestfs_h *g)
 
   count = 0;
   ITER_DRIVES (g, i, drv) {
-    ret[count++] -      safe_asprintf (g, "path=%s%s%s%s%s%s%s%s%s",
-                     drv->path,
-                     drv->readonly ? " readonly" : "",
-                     drv->format ? " format=" : "",
-                     drv->format ? : "",
-                     drv->iface ? " iface=" : "",
-                     drv->iface ? : "",
-                     drv->name ? " name=" : "",
-                     drv->name ? : "",
-                     drv->use_cache_none ? " cache=none" :
"");
+    ret[count++] = drive_to_string (g, drv);
   }
 
   ret[count] = NULL;
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index f5db4cf..71f18a3 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -113,8 +113,22 @@ struct event {
 };
 
 /* Drives added to the handle. */
+enum drive_protocol {
+  drive_protocol_file,
+  drive_protocol_nbd,
+};
+union drive_source {
+  char *path;                   /* protocol = "file" */
+  struct {                      /* protocol = "nbd" */
+    char *server;
+    int port;
+    char *exportname;
+  } nbd;
+};
+
 struct drive {
-  char *path;
+  enum drive_protocol protocol;
+  union drive_source u;
 
   bool readonly;
   char *format;
@@ -123,7 +137,8 @@ struct drive {
   char *disk_label;
   bool use_cache_none;
 
-  void *priv;                   /* Data used by attach method. */
+  /* Data used by the attach method. */
+  void *priv;
   void (*free_priv) (void *);
 };
 
diff --git a/src/guestfs.pod b/src/guestfs.pod
index af7f276..6736f07 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -637,6 +637,41 @@ Backends that support hotplugging do not require that you
add
 E<ge> 1 disk before calling launch.  When hotplugging is supported
 you don't need to add any disks.
 
+=head2 NETWORK BLOCK DEVICES
+
+Libguestfs can access Network Block Device (NBD) disks remotely.
+
+To do this, set the optional C<protocol> and C<server> parameters
of
+L</guestfs_add_drive_opts> like this:
+
+ guestfs_add_drive_opts (g, "" /* export name - see below */,
+                         GUESTFS_ADD_DRIVE_OPTS_FORMAT, "raw",
+                         GUESTFS_ADD_DRIVE_OPTS_PROTOCOL, "nbd",
+                         GUESTFS_ADD_DRIVE_OPTS_SERVER, "localhost",
+                         -1);
+
+You can also specify the optional C<port> number if the NBD server is
+running on a non-default port.
+
+Notes:
+
+=over 4
+
+=item *
+
+The C<filename> parameter is the NBD export name.  Use an empty string
+to mean the default export.
+
+Not all NBD servers support this feature, and the libvirt
+attach-method also does not support it.
+
+=item *
+
+The libvirt backend requires that you set the C<format> parameter of
+L</guestfs_add_drive_opts> accurately when you use writable NBD disks.
+
+=back
+
 =head2 INSPECTION
 
 Libguestfs has APIs for inspecting an unknown disk image to find out
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index 122f143..4b70cc1 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -931,36 +931,46 @@ qemu_supports_virtio_scsi (guestfs_h *g)
   return g->app.virtio_scsi == 1;
 }
 
+/* Convert a struct drive into a qemu -drive parameter.  Note that if
+ * using virtio-scsi, then the code above adds a second -device
+ * parameter to connect this drive to the SCSI HBA, as is required by
+ * virtio-scsi.
+ */
 static char *
 qemu_drive_param (guestfs_h *g, const struct drive *drv, size_t index)
 {
-  size_t i;
-  size_t len = 128;
-  const char *p;
-  char *r;
+  CLEANUP_FREE char *file = NULL, *escaped_file = NULL;
+  size_t i, len;
   const char *iface;
+  char *p, *ret;
+
+  /* Make the file= parameter. */
+  switch (drv->protocol) {
+  case drive_protocol_file:
+    file = safe_strdup (g, drv->u.path);
+    break;
+  case drive_protocol_nbd:
+    if (STREQ (drv->u.nbd.exportname, ""))
+      file = safe_asprintf (g, "nbd:%s:%d", drv->u.nbd.server,
drv->u.nbd.port);
+    else
+      file = safe_asprintf (g, "nbd:%s:%d:exportname=%s",
+                            drv->u.nbd.server, drv->u.nbd.port,
+                            drv->u.nbd.exportname);
+    break;
+  default:
+    abort ();
+  }
 
-  len += strlen (drv->path) * 2; /* every "," could become
",," */
-  if (drv->iface)
-    len += strlen (drv->iface);
-  if (drv->format)
-    len += strlen (drv->format);
-  if (drv->disk_label)
-    len += strlen (drv->disk_label);
-
-  r = safe_malloc (g, len);
-
-  strcpy (r, "file=");
-  i = 5;
-
-  /* Copy the path in, escaping any "," as ",,". */
-  for (p = drv->path; *p; p++) {
-    if (*p == ',') {
-      r[i++] = ',';
-      r[i++] = ',';
-    } else
-      r[i++] = *p;
+  /* Escape the file= parameter.  Every ',' becomes ',,'. */
+  len = strlen (file);
+  len = len*2 + 1;              /* Maximum length of escaped filename + \0 */
+  p = escaped_file = safe_malloc (g, len);
+  for (i = 0; i < len; ++i) {
+    *p++ = file[i];
+    if (file[i] == ',')
+      *p++ = ',';
   }
+  *p = '\0';
 
   if (drv->iface)
     iface = drv->iface;
@@ -969,17 +979,18 @@ qemu_drive_param (guestfs_h *g, const struct drive *drv,
size_t index)
   else
     iface = "virtio";
 
-  snprintf (&r[i], len-i, "%s%s%s%s%s%s,id=hd%zu,if=%s",
-            drv->readonly ? ",snapshot=on" : "",
-            drv->use_cache_none ? ",cache=none" : "",
-            drv->format ? ",format=" : "",
-            drv->format ? drv->format : "",
-            drv->disk_label ? ",serial=" : "",
-            drv->disk_label ? drv->disk_label : "",
-            index,
-            iface);
-
-  return r;                     /* caller frees */
+  ret = safe_asprintf (g, "file=%s%s%s%s%s%s%s,id=hd%zu,if=%s",
+                       escaped_file,
+                       drv->readonly ? ",snapshot=on" :
"",
+                       drv->use_cache_none ? ",cache=none" :
"",
+                       drv->format ? ",format=" : "",
+                       drv->format ? drv->format : "",
+                       drv->disk_label ? ",serial=" :
"",
+                       drv->disk_label ? drv->disk_label : "",
+                       index,
+                       iface);
+
+  return ret;
 }
 
 /*
https://rwmj.wordpress.com/2011/01/09/how-are-linux-drives-named-beyond-drive-26-devsdz/
*/
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 58ccb9e..bef3910 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -101,12 +101,13 @@ xmlBufferDetach (xmlBufferPtr buf)
 
 /* Pointed to by 'struct drive *' -> priv field. */
 struct drive_libvirt {
-  /* This is either the original path, made absolute.  Or for readonly
-   * drives, it is an (absolute path to) the overlay file that we
-   * create.  This is always non-NULL.
+  /* The drive that we actually add.  If using an overlay, then this
+   * might be different from drive->protocol.
    */
-  char *path;
-  /* The format of priv->path. */
+  enum drive_protocol protocol;
+  union drive_source u;
+
+  /* The format of the drive we add. */
   char *format;
 };
 
@@ -135,8 +136,8 @@ static int is_custom_qemu (guestfs_h *g);
 static int is_blk (const char *path);
 static int random_chars (char *ret, size_t len);
 static void ignore_errors (void *ignore, virErrorPtr ignore2);
-static char *make_qcow2_overlay (guestfs_h *g, const char *path, const char
*format, const char *selinux_imagelabel);
-static int make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv, const
char *selinux_imagelabel);
+static char *make_qcow2_overlay (guestfs_h *g, const char *backing_device,
const char *format, const char *selinux_imagelabel);
+static int make_drive_priv (guestfs_h *g, struct drive *drv, const char
*selinux_imagelabel);
 static void drive_free_priv (void *);
 static void set_socket_create_context (guestfs_h *g);
 static void clear_socket_create_context (guestfs_h *g);
@@ -235,18 +236,18 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
   guestfs___launch_send_progress (g, 3);
   TRACE0 (launch_build_libvirt_appliance_end);
 
-  /* Create overlays for read-only drives and the appliance.  This
-   * works around lack of support for <transient/> disks in libvirt.
-   * Note that appliance can be NULL if using the old-style appliance.
-   */
+  /* Note that appliance can be NULL if using the old-style appliance. */
   if (appliance) {
     params.appliance_overlay = make_qcow2_overlay (g, appliance,
"raw", NULL);
     if (!params.appliance_overlay)
       goto cleanup;
   }
 
+  /* Set up the drv->priv part of the struct.  A side-effect of this
+   * may be that we create qcow2 overlays for drives.
+   */
   ITER_DRIVES (g, i, drv) {
-    if (make_qcow2_overlay_for_drive (g, drv, g->virt_selinux_imagelabel) ==
-1)
+    if (make_drive_priv (g, drv, g->virt_selinux_imagelabel) == -1)
       goto cleanup;
   }
 
@@ -1044,7 +1045,8 @@ construct_libvirt_xml_disk (guestfs_h *g,
 {
   char drive_name[64] = "sd";
   char scsi_target[64];
-  struct drive_libvirt *drv_priv;
+  char port_str[64];
+  struct drive_libvirt *drv_priv = (struct drive_libvirt *) drv->priv;
   CLEANUP_FREE char *format = NULL;
   int is_host_device;
 
@@ -1057,47 +1059,86 @@ construct_libvirt_xml_disk (guestfs_h *g,
   guestfs___drive_name (drv_index, &drive_name[2]);
   snprintf (scsi_target, sizeof scsi_target, "%zu", drv_index);
 
-  drv_priv = (struct drive_libvirt *) drv->priv;
-
-  /* Change the libvirt XML according to whether the host path is
-   * a device or a file.  For devices, use:
-   *   <disk type=block device=disk>
-   *     <source dev=[path]>
-   * For files, use:
-   *   <disk type=file device=disk>
-   *     <source file=[path]>
-   */
-  is_host_device = is_blk (drv_priv->path);
-
   XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "disk"));
   XMLERROR (-1,
             xmlTextWriterWriteAttribute (xo, BAD_CAST "device",
                                          BAD_CAST "disk"));
-  if (!is_host_device) {
-    XMLERROR (-1,
-              xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
-                                           BAD_CAST "file"));
 
-    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "source"));
-    XMLERROR (-1,
-              xmlTextWriterWriteAttribute (xo, BAD_CAST "file",
-                                           BAD_CAST drv_priv->path));
-    if (construct_libvirt_xml_disk_source_seclabel (g, xo) == -1)
+  switch (drv_priv->protocol) {
+  case drive_protocol_file:
+    /* Change the libvirt XML according to whether the host path is
+     * a device or a file.  For devices, use:
+     *   <disk type=block device=disk>
+     *     <source dev=[path]>
+     * For files, use:
+     *   <disk type=file device=disk>
+     *     <source file=[path]>
+     */
+    is_host_device = is_blk (drv_priv->u.path);
+
+    if (!is_host_device) {
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
+                                             BAD_CAST "file"));
+
+      XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"source"));
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "file",
+                                             BAD_CAST drv_priv->u.path));
+      if (construct_libvirt_xml_disk_source_seclabel (g, xo) == -1)
+        return -1;
+      XMLERROR (-1, xmlTextWriterEndElement (xo));
+    }
+    else {
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
+                                             BAD_CAST "block"));
+
+      XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"source"));
+      XMLERROR (-1,
+                xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
+                                             BAD_CAST drv_priv->u.path));
+      if (construct_libvirt_xml_disk_source_seclabel (g, xo) == -1)
+        return -1;
+      XMLERROR (-1, xmlTextWriterEndElement (xo));
+    }
+    break;
+
+  case drive_protocol_nbd:
+    /* For NBD:
+     *   <disk type=network device=disk>
+     *     <source protocol=nbd>
+     *       <host name='example.com' port='10809'/>
+     */
+    if (STRNEQ (drv_priv->u.nbd.exportname, "")) {
+      error (g, _("libvirt does not support nbd 'exportname'
feature"));
       return -1;
-    XMLERROR (-1, xmlTextWriterEndElement (xo));
-  }
-  else {
+    }
+
     XMLERROR (-1,
               xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
-                                           BAD_CAST "block"));
+                                           BAD_CAST "network"));
 
     XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "source"));
     XMLERROR (-1,
-              xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
-                                           BAD_CAST drv_priv->path));
+              xmlTextWriterWriteAttribute (xo, BAD_CAST "protocol",
+                                           BAD_CAST "nbd"));
+    XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "host"));
+    XMLERROR (-1,
+              xmlTextWriterWriteAttribute (xo, BAD_CAST "name",
+                                           BAD_CAST
drv_priv->u.nbd.server));
+    snprintf (port_str, sizeof port_str, "%d",
drv_priv->u.nbd.port);
+    XMLERROR (-1,
+              xmlTextWriterWriteAttribute (xo, BAD_CAST "port",
+                                           BAD_CAST port_str));
+    XMLERROR (-1, xmlTextWriterEndElement (xo));
     if (construct_libvirt_xml_disk_source_seclabel (g, xo) == -1)
       return -1;
     XMLERROR (-1, xmlTextWriterEndElement (xo));
+    break;
+
+  default:
+    abort ();
   }
 
   XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "target"));
@@ -1118,7 +1159,7 @@ construct_libvirt_xml_disk (guestfs_h *g,
               xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
                                            BAD_CAST drv_priv->format));
   }
-  else {
+  else if (drv_priv->protocol == drive_protocol_file) {
     /* libvirt has disabled the feature of detecting the disk format,
      * unless the administrator sets allow_disk_format_probing=1 in
      * qemu.conf.  There is no way to detect if this option is set, so we
@@ -1129,15 +1170,14 @@ construct_libvirt_xml_disk (guestfs_h *g,
      * the users pass the format to libguestfs which will faithfully pass
      * that to libvirt and this function won't be used.
      */
-    format = guestfs_disk_format (g, drv_priv->path);
+    format = guestfs_disk_format (g, drv_priv->u.path);
     if (!format)
       return -1;
 
     if (STREQ (format, "unknown")) {
-      error (g, _("could not auto-detect the format of
'%s'\n"
+      error (g, _("could not auto-detect the format.\n"
                   "If the format is known, pass the format to libguestfs,
eg. using the\n"
-                  "'--format' option, or via the optional
'format' argument to 'add-drive'."),
-             drv->path);
+                  "'--format' option, or via the optional
'format' argument to 'add-drive'."));
       return -1;
     }
 
@@ -1145,6 +1185,13 @@ construct_libvirt_xml_disk (guestfs_h *g,
               xmlTextWriterWriteAttribute (xo, BAD_CAST "type",
                                            BAD_CAST format));
   }
+  else {
+    error (g, _("could not auto-detect the format when using a non-file
protocol.\n"
+                "If the format is known, pass the format to libguestfs,
eg. using the\n"
+                "'--format' option, or via the optional
'format' argument to 'add-drive'."));
+    return -1;
+  }
+
   if (drv->use_cache_none) {
     XMLERROR (-1,
               xmlTextWriterWriteAttribute (xo, BAD_CAST "cache",
@@ -1265,7 +1312,7 @@ construct_libvirt_xml_appliance (guestfs_h *g,
   XMLERROR (-1, xmlTextWriterEndElement (xo));
 
   /* We'd like to do this, but it's not supported by libvirt.
-   * See construct_libvirt_xml_qemu_cmdline for the workaround.
+   * See make_drive_priv for the workaround.
    *
    * XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"transient"));
    * XMLERROR (-1, xmlTextWriterEndElement (xo));
@@ -1397,18 +1444,22 @@ ignore_errors (void *ignore, virErrorPtr ignore2)
   /* empty */
 }
 
-/* Create a temporary qcow2 overlay on top of 'path'. */
+/* Create a temporary qcow2 overlay on top of 'backing_device', which
is
+ * either an absolute path or a qemu device name.
+ */
 static char *
-make_qcow2_overlay (guestfs_h *g, const char *path, const char *format,
-                    const char *selinux_imagelabel)
+make_qcow2_overlay (guestfs_h *g, const char *backing_device,
+                    const char *format, const char *selinux_imagelabel)
 {
   char *tmpfile = NULL;
   CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g);
   int r;
 
-  /* Path must be absolute. */
-  assert (path);
-  assert (path[0] == '/');
+  /* Assert that backing_device is an absolute path, or it's an
+   * nbd device name.
+   */
+  assert (backing_device);
+  assert (backing_device[0] == '/' || STRPREFIX (backing_device,
"nbd:"));
 
   tmpfile = safe_asprintf (g, "%s/snapshot%d", g->tmpdir,
++g->unique);
 
@@ -1417,7 +1468,7 @@ make_qcow2_overlay (guestfs_h *g, const char *path, const
char *format,
   guestfs___cmd_add_arg (cmd, "-f");
   guestfs___cmd_add_arg (cmd, "qcow2");
   guestfs___cmd_add_arg (cmd, "-b");
-  guestfs___cmd_add_arg (cmd, path);
+  guestfs___cmd_add_arg (cmd, backing_device);
   if (format) {
     guestfs___cmd_add_arg (cmd, "-o");
     guestfs___cmd_add_arg_format (cmd, "backing_fmt=%s", format);
@@ -1427,7 +1478,7 @@ make_qcow2_overlay (guestfs_h *g, const char *path, const
char *format,
   if (r == -1)
     goto error;
   if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
-    guestfs___external_command_failed (g, r, "qemu-img create",
path);
+    guestfs___external_command_failed (g, r, "qemu-img create",
backing_device);
     goto error;
   }
 
@@ -1448,9 +1499,14 @@ make_qcow2_overlay (guestfs_h *g, const char *path, const
char *format,
   return NULL;
 }
 
+/* This sets up the drv->priv structure, which contains the drive
+ * that we're actually going to add.  If asked to make a drive readonly
+ * then because libvirt doesn't support <transient/> we have to add
+ * a qcow2 overlay here.
+ */
 static int
-make_qcow2_overlay_for_drive (guestfs_h *g, struct drive *drv,
-                              const char *selinux_imagelabel)
+make_drive_priv (guestfs_h *g, struct drive *drv,
+                 const char *selinux_imagelabel)
 {
   char *path;
   struct drive_libvirt *drv_priv;
@@ -1461,25 +1517,64 @@ make_qcow2_overlay_for_drive (guestfs_h *g, struct drive
*drv,
   drv->priv = drv_priv = safe_calloc (g, 1, sizeof (struct drive_libvirt));
   drv->free_priv = drive_free_priv;
 
-  /* Even for non-readonly paths, we need to make the paths absolute here. */
-  path = realpath (drv->path, NULL);
-  if (path == NULL) {
-    perrorf (g, _("realpath: could not convert '%s' to absolute
path"),
-             drv->path);
-    return -1;
-  }
+  switch (drv->protocol) {
+  case drive_protocol_file:
 
-  if (!drv->readonly) {
-    drv_priv->path = path;
-    drv_priv->format = drv->format ? safe_strdup (g, drv->format) :
NULL;
-  }
-  else {
-    drv_priv->path = make_qcow2_overlay (g, path, drv->format,
-                                         selinux_imagelabel);
-    free (path);
-    if (!drv_priv->path)
+    /* Even for non-readonly paths, we need to make the paths absolute here. */
+    path = realpath (drv->u.path, NULL);
+    if (path == NULL) {
+      perrorf (g, _("realpath: could not convert '%s' to absolute
path"),
+               drv->u.path);
       return -1;
-    drv_priv->format = safe_strdup (g, "qcow2");
+    }
+
+    drv_priv->protocol = drive_protocol_file;
+
+    if (!drv->readonly) {
+      drv_priv->u.path = path;
+      drv_priv->format = drv->format ? safe_strdup (g, drv->format) :
NULL;
+    }
+    else {
+      drv_priv->u.path = make_qcow2_overlay (g, path, drv->format,
+                                             selinux_imagelabel);
+      free (path);
+      if (!drv_priv->u.path)
+        return -1;
+      drv_priv->format = safe_strdup (g, "qcow2");
+    }
+    break;
+
+  case drive_protocol_nbd:
+    if (!drv->readonly) {
+      drv_priv->protocol = drive_protocol_nbd;
+      drv_priv->u.nbd.server = safe_strdup (g, drv->u.nbd.server);
+      drv_priv->u.nbd.port = drv->u.nbd.port;
+      drv_priv->u.nbd.exportname = safe_strdup (g,
drv->u.nbd.exportname);
+      drv_priv->format = drv->format ? safe_strdup (g, drv->format) :
NULL;
+    }
+    else {
+      CLEANUP_FREE char *nbd_device;
+
+      if (STREQ (drv->u.nbd.exportname, ""))
+        nbd_device +          safe_asprintf (g, "nbd:%s:%d",
drv->u.nbd.server, drv->u.nbd.port);
+      else
+        nbd_device +          safe_asprintf (g,
"nbd:%s:%d:exportname=%s",
+                         drv->u.nbd.server, drv->u.nbd.port,
+                         drv->u.nbd.exportname);
+
+      drv_priv->protocol = drive_protocol_file;
+      drv_priv->u.path = make_qcow2_overlay (g, nbd_device, drv->format,
+                                             selinux_imagelabel);
+      if (!drv_priv->u.path)
+        return -1;
+      drv_priv->format = safe_strdup (g, "qcow2");
+    }
+    break;
+
+  default:
+    abort ();
   }
 
   return 0;
@@ -1490,7 +1585,18 @@ drive_free_priv (void *priv)
 {
   struct drive_libvirt *drv_priv = priv;
 
-  free (drv_priv->path);
+  switch (drv_priv->protocol) {
+  case drive_protocol_file:
+    free (drv_priv->u.path);
+    break;
+  case drive_protocol_nbd:
+    free (drv_priv->u.nbd.server);
+    free (drv_priv->u.nbd.exportname);
+    break;
+  default:
+    abort ();
+  }
+
   free (drv_priv->format);
   free (drv_priv);
 }
@@ -1589,10 +1695,7 @@ hot_add_drive_libvirt (guestfs_h *g, struct drive *drv,
size_t drv_index)
     return -1;
   }
 
-  /* Create overlay for read-only drive.  This works around lack of
-   * support for <transient/> disks in libvirt.
-   */
-  if (make_qcow2_overlay_for_drive (g, drv, g->virt_selinux_imagelabel) ==
-1)
+  if (make_drive_priv (g, drv, g->virt_selinux_imagelabel) == -1)
     return -1;
 
   /* Create the XML for the new disk. */
diff --git a/tests/nbd/Makefile.am b/tests/nbd/Makefile.am
new file mode 100644
index 0000000..09d79c8
--- /dev/null
+++ b/tests/nbd/Makefile.am
@@ -0,0 +1,26 @@
+# libguestfs
+# Copyright (C) 2013 Red Hat Inc.
+#
+# 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 2 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+include $(top_srcdir)/subdir-rules.mk
+
+TESTS = \
+	test-nbd.pl
+
+TESTS_ENVIRONMENT = $(top_builddir)/run --test
+
+EXTRA_DIST = \
+	$(TESTS)
diff --git a/tests/nbd/test-nbd.pl b/tests/nbd/test-nbd.pl
new file mode 100755
index 0000000..4c14caf
--- /dev/null
+++ b/tests/nbd/test-nbd.pl
@@ -0,0 +1,78 @@
+#!/usr/bin/perl
+# Copyright (C) 2013 Red Hat Inc.
+#
+# 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 2 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, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+use strict;
+use warnings;
+
+use Sys::Guestfs;
+
+my $disk = "../guests/fedora.img";
+
+exit 77 if $ENV{SKIP_TEST_NBD_PL};
+
+# Check we have qemu-nbd.
+if (system ("qemu-nbd --help >/dev/null 2>&1") != 0) {
+    print "$0: test skipped because qemu-nbd program not found\n";
+    exit 77
+}
+
+if (! -r $disk || -z $disk) {
+    print "$0: test skipped because $disk is not found\n";
+    exit 77
+}
+
+# Since read-only and read-write paths are quite different, we have to
+# test both separately.
+my $readonly;
+for $readonly (1, 0) {
+    # Choose a random port number.  XXX Should check it is not in use.
+    my $port = int (60000 + rand (5000));
+
+    # Run the NBD server.
+    print "Starting qemu-nbd server on port $port ...\n";
+    my $pid = fork ();
+    if ($pid == 0) {
+        exec ("qemu-nbd", $disk, "-p", $port,
"-t");
+        die "qemu-nbd: $!";
+    }
+
+    my $g = Sys::Guestfs->new ();
+
+    # Add an NBD drive.
+    $g->add_drive ("", readonly => $readonly, format =>
"raw",
+                   protocol => "nbd", server =>
"localhost", port => $port);
+
+    # XXX qemu-nbd lacks any way to tell if it is awake and listening
+    # for connections.  It could write a pid file or something.  Could
+    # we check that the socket has been opened by looking in netstat?
+    sleep (2);
+
+    # This dies if qemu cannot connect to the NBD server.
+    $g->launch ();
+
+    # Inspection is quite a thorough test:
+    my $root = $g->inspect_os ();
+    die "$root != /dev/VG/Root" unless $root eq
"/dev/VG/Root";
+
+    # Note we have to close the handle (hence killing qemu), and we
+    # have to kill qemu-nbd.
+    $g->close ();
+    kill 15, $pid;
+    waitpid ($pid, 0) or die "waitpid: $pid: $!";
+}
+
+exit 0
-- 
1.8.1.4