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

From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; You can now add remote NBD drives using: (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-d... */ 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