[Libguestfs] [PATCH 2/2] drives: Centrally create overlays for read-only drives.

qemu has broken snapshot=on ... again. Change the way that drives are created so that the backend no longer has to use snapshot=on, <transient/> (which never worked), or UML's corresponding COW-creation feature (also broken). Instead of that, the src/drives.c code will create overlays when required by calling into a new backend operation 'create_cow_overlay'. This operation runs 'qemu-img create -b' or 'uml_mkcow' as determined by the backend, and returns the name of the overlay. The format of the overlay is still backend-specific because qemu needs to use qcow2 and UML needs to use COW. This patch also includes some factorization of the libvirt XML code. This also drops the drv->priv (private per-drive data) field, since it is no longer used by any backend. This also moves the guestfs___drive_source_qemu_param utility function, used & shared by the direct & libvirt backends only, into src/launch-direct.c (from src/drives.c). --- src/drives.c | 286 ++++++---------------- src/guestfs-internal.h | 33 ++- src/launch-direct.c | 275 +++++++++++++++++++-- src/launch-libvirt.c | 653 ++++++++++++++++++++++--------------------------- src/launch-uml.c | 87 ++++--- src/libvirt-domain.c | 5 +- 6 files changed, 703 insertions(+), 636 deletions(-) diff --git a/src/drives.c b/src/drives.c index cddde4f..4911e73 100644 --- a/src/drives.c +++ b/src/drives.c @@ -37,8 +37,6 @@ #include <pcre.h> -#include <libxml/uri.h> - #include "c-ctype.h" #include "ignore-value.h" @@ -81,6 +79,38 @@ free_regexps (void) pcre_free (re_hostname_port); } +static void free_drive_struct (struct drive *drv); +static void free_drive_source (struct drive_source *src); + +/* For readonly drives, create an overlay to protect the original + * drive content. Note we never need to clean up these overlays since + * they are created in the temporary directory and deleted when the + * handle is closed. + */ +static int +create_overlay (guestfs_h *g, struct drive *drv) +{ + char *overlay; + + assert (g->backend_ops != NULL); + + if (g->backend_ops->create_cow_overlay == NULL) { + error (g, _("this backend does not support adding read-only drives")); + return -1; + } + + debug (g, "creating COW overlay to protect original drive content"); + overlay = g->backend_ops->create_cow_overlay (g, g->backend_data, drv); + if (overlay == NULL) + return -1; + + if (drv->overlay) + free (drv->overlay); + drv->overlay = overlay; + + return 0; +} + /* Create and free the 'drive' struct. */ static struct drive * create_drive_file (guestfs_h *g, const char *path, @@ -92,15 +122,20 @@ create_drive_file (guestfs_h *g, const char *path, drv->src.protocol = drive_protocol_file; drv->src.u.path = safe_strdup (g, path); + drv->src.format = format ? safe_strdup (g, format) : NULL; 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->cachemode = cachemode ? safe_strdup (g, cachemode) : NULL; - drv->priv = drv->free_priv = NULL; + if (readonly) { + if (create_overlay (g, drv) == -1) { + free_drive_struct (drv); + return NULL; + } + } return drv; } @@ -123,15 +158,20 @@ create_drive_non_file (guestfs_h *g, drv->src.u.exportname = safe_strdup (g, exportname); drv->src.username = username ? safe_strdup (g, username) : NULL; drv->src.secret = secret ? safe_strdup (g, secret) : NULL; + drv->src.format = format ? safe_strdup (g, format) : NULL; 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->cachemode = cachemode ? safe_strdup (g, cachemode) : NULL; - drv->priv = drv->free_priv = NULL; + if (readonly) { + if (create_overlay (g, drv) == -1) { + free_drive_struct (drv); + return NULL; + } + } return drv; } @@ -523,35 +563,49 @@ free_drive_servers (struct drive_server *servers, size_t nr_servers) static void free_drive_struct (struct drive *drv) { - guestfs___free_drive_source (&drv->src); - free (drv->format); + free_drive_source (&drv->src); + free (drv->overlay); free (drv->iface); free (drv->name); free (drv->disk_label); free (drv->cachemode); - if (drv->priv && drv->free_priv) - drv->free_priv (drv->priv); - free (drv); } +static const char * +protocol_to_string (enum drive_protocol protocol) +{ + switch (protocol) { + case drive_protocol_file: return "file"; + case drive_protocol_ftp: return "ftp"; + case drive_protocol_ftps: return "ftps"; + case drive_protocol_gluster: return "gluster"; + case drive_protocol_http: return "http"; + case drive_protocol_https: return "https"; + case drive_protocol_iscsi: return "iscsi"; + case drive_protocol_nbd: return "nbd"; + case drive_protocol_rbd: return "rbd"; + case drive_protocol_sheepdog: return "sheepdog"; + case drive_protocol_ssh: return "ssh"; + case drive_protocol_tftp: return "tftp"; + } + abort (); +} + /* 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; - - p = guestfs___drive_source_qemu_param (g, &drv->src); - return safe_asprintf - (g, "%s%s%s%s%s%s%s%s%s%s%s%s", - p, + (g, "%s%s%s%s protocol=%s%s%s%s%s%s%s%s%s", + drv->src.u.path, drv->readonly ? " readonly" : "", - drv->format ? " format=" : "", - drv->format ? : "", + drv->src.format ? " format=" : "", + drv->src.format ? : "", + protocol_to_string (drv->src.protocol), drv->iface ? " iface=" : "", drv->iface ? : "", drv->name ? " name=" : "", @@ -1176,199 +1230,11 @@ guestfs__debug_drives (guestfs_h *g) return ret.argv; /* caller frees */ } -/* The drive_source struct is also used in the backends, so we - * also have these utility functions. - */ -void -guestfs___copy_drive_source (guestfs_h *g, - const struct drive_source *src, - struct drive_source *dest) -{ - size_t i; - - dest->protocol = src->protocol; - dest->u.path = safe_strdup (g, src->u.path); - dest->nr_servers = src->nr_servers; - dest->servers = safe_calloc (g, src->nr_servers, - sizeof (struct drive_server)); - for (i = 0; i < src->nr_servers; ++i) { - dest->servers[i].transport = src->servers[i].transport; - if (src->servers[i].u.hostname) - dest->servers[i].u.hostname = safe_strdup (g, src->servers[i].u.hostname); - dest->servers[i].port = src->servers[i].port; - } -} - -static char * -make_uri (guestfs_h *g, const char *scheme, const char *user, - struct drive_server *server, const char *path) -{ - xmlURI uri = { .scheme = (char *) scheme, - .path = (char *) path, - .user = (char *) user }; - CLEANUP_FREE char *query = NULL; - - switch (server->transport) { - case drive_transport_none: - case drive_transport_tcp: - uri.server = server->u.hostname; - uri.port = server->port; - break; - case drive_transport_unix: - query = safe_asprintf (g, "socket=%s", server->u.socket); - uri.query_raw = query; - break; - } - - return (char *) xmlSaveUri (&uri); -} - -char * -guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) -{ - /* Note that the qemu parameter is the bit after "file=". It is not - * escaped here, but would usually be escaped if passed to qemu as - * part of a full -drive parameter (but not for qemu-img). - */ - switch (src->protocol) { - case drive_protocol_file: - /* We might need to rewrite the path if it contains a ':' character. */ - if (src->u.path[0] == '/' || strchr (src->u.path, ':') == NULL) - return safe_strdup (g, src->u.path); - else - return safe_asprintf (g, "./%s", src->u.path); - - case drive_protocol_ftp: - return make_uri (g, "ftp", src->username, - &src->servers[0], src->u.exportname); - - case drive_protocol_ftps: - return make_uri (g, "ftps", src->username, - &src->servers[0], src->u.exportname); - - case drive_protocol_gluster: - switch (src->servers[0].transport) { - case drive_transport_none: - return make_uri (g, "gluster", NULL, &src->servers[0], src->u.exportname); - case drive_transport_tcp: - return make_uri (g, "gluster+tcp", - NULL, &src->servers[0], src->u.exportname); - case drive_transport_unix: - return make_uri (g, "gluster+unix", NULL, &src->servers[0], NULL); - } - - case drive_protocol_http: - return make_uri (g, "http", src->username, - &src->servers[0], src->u.exportname); - - case drive_protocol_https: - return make_uri (g, "https", src->username, - &src->servers[0], src->u.exportname); - - case drive_protocol_iscsi: - return make_uri (g, "iscsi", NULL, &src->servers[0], src->u.exportname); - - case drive_protocol_nbd: { - CLEANUP_FREE char *p = NULL; - char *ret; - - switch (src->servers[0].transport) { - case drive_transport_none: - case drive_transport_tcp: - p = safe_asprintf (g, "nbd:%s:%d", - src->servers[0].u.hostname, src->servers[0].port); - break; - case drive_transport_unix: - p = safe_asprintf (g, "nbd:unix:%s", src->servers[0].u.socket); - break; - } - assert (p); - - if (STREQ (src->u.exportname, "")) - ret = safe_strdup (g, p); - else - /* Skip the mandatory leading '/' character. */ - ret = safe_asprintf (g, "%s:exportname=%s", p, &src->u.exportname[1]); - - return ret; - } - - case drive_protocol_rbd: { - /* build the list of all the mon hosts */ - CLEANUP_FREE char *mon_host = NULL, *username = NULL, *secret = NULL; - const char *auth; - size_t n = 0; - size_t i, j; - - for (i = 0; i < src->nr_servers; i++) { - n += strlen (src->servers[i].u.hostname); - n += 8; /* for slashes, colons, & port numbers */ - } - n++; /* for \0 */ - mon_host = safe_malloc (g, n); - n = 0; - for (i = 0; i < src->nr_servers; i++) { - CLEANUP_FREE char *port = NULL; - - for (j = 0; j < strlen (src->servers[i].u.hostname); j++) - mon_host[n++] = src->servers[i].u.hostname[j]; - mon_host[n++] = '\\'; - mon_host[n++] = ':'; - port = safe_asprintf (g, "%d", src->servers[i].port); - for (j = 0; j < strlen (port); j++) - mon_host[n++] = port[j]; - - /* join each host with \; */ - if (i != src->nr_servers - 1) { - mon_host[n++] = '\\'; - mon_host[n++] = ';'; - } - } - mon_host[n] = '\0'; - - if (src->username) - username = safe_asprintf (g, ":id=%s", src->username); - if (src->secret) - secret = safe_asprintf (g, ":key=%s", src->secret); - if (username || secret) - auth = ":auth_supported=cephx\\;none"; - else - auth = ":auth_supported=none"; - - /* Skip the mandatory leading '/' character on exportname. */ - return safe_asprintf (g, "rbd:%s:mon_host=%s%s%s%s", - &src->u.exportname[1], - mon_host, - username ? username : "", - auth, - secret ? secret : ""); - } - - case drive_protocol_sheepdog: - /* Skip the mandatory leading '/' character on exportname. */ - if (src->nr_servers == 0) - return safe_asprintf (g, "sheepdog:%s", &src->u.exportname[1]); - else /* XXX How to pass multiple hosts? */ - return safe_asprintf (g, "sheepdog:%s:%d:%s", - src->servers[0].u.hostname, src->servers[0].port, - &src->u.exportname[1]); - - case drive_protocol_ssh: - return make_uri (g, "ssh", src->username, - &src->servers[0], src->u.exportname); - - case drive_protocol_tftp: - return make_uri (g, "tftp", src->username, - &src->servers[0], src->u.exportname); - } - - abort (); -} - -void -guestfs___free_drive_source (struct drive_source *src) +static void +free_drive_source (struct drive_source *src) { if (src) { + free (src->format); free (src->u.path); free (src->username); free (src->secret); diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index a1f3f02..61f41a1 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -209,6 +209,9 @@ struct drive_server { struct drive_source { enum drive_protocol protocol; + /* Format (eg. raw, qcow2). NULL = autodetect. */ + char *format; + /* This field is always non-NULL. It may be an empty string. */ union { char *path; /* path to file (file) */ @@ -228,19 +231,27 @@ struct drive_source { char *secret; }; +/* There is one 'struct drive' per drive, including hot-plugged drives. */ struct drive { + /* Original source of the drive, eg. file:..., http:... */ struct drive_source src; + /* If the drive is readonly, then an overlay [a local file] is + * created before launch to protect the original drive content, and + * the filename is stored here. Backends should open this file if + * it is non-NULL, else consult the original source above. + * + * Note that the overlay is in a backend-specific format, probably + * different from the source format. eg. qcow2, UML COW. + */ + char *overlay; + + /* Various per-drive flags. */ bool readonly; - char *format; char *iface; char *name; char *disk_label; char *cachemode; - - /* Data used by the backend. */ - void *priv; - void (*free_priv) (void *); }; /* Extra hv parameters (from guestfs_config). */ @@ -262,6 +273,12 @@ struct backend_ops { */ size_t data_size; + /* Create a COW overlay on top of a drive. This must be a local + * file, created in the temporary directory. This is called when + * the drive is added to the handle. + */ + char *(*create_cow_overlay) (guestfs_h *g, void *data, struct drive *drv); + /* Launch and shut down. */ int (*launch) (guestfs_h *g, void *data, const char *arg); int (*shutdown) (guestfs_h *g, void *data, int check_for_errors); @@ -695,9 +712,6 @@ extern size_t guestfs___checkpoint_drives (guestfs_h *g); extern void guestfs___rollback_drives (guestfs_h *g, size_t); extern void guestfs___add_dummy_appliance_drive (guestfs_h *g); extern void guestfs___free_drives (guestfs_h *g); -extern void guestfs___copy_drive_source (guestfs_h *g, const struct drive_source *src, struct drive_source *dest); -extern char *guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src); -extern void guestfs___free_drive_source (struct drive_source *src); /* appliance.c */ extern int guestfs___build_appliance (guestfs_h *g, char **kernel, char **dtb, char **initrd, char **appliance); @@ -813,4 +827,7 @@ extern void guestfs___cmd_close (struct command *); #endif extern void guestfs___cleanup_cmd_close (struct command **); +/* launch-direct.c */ +extern char *guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src); + #endif /* GUESTFS_INTERNAL_H_ */ diff --git a/src/launch-direct.c b/src/launch-direct.c index 532740a..9ad5cf2 100644 --- a/src/launch-direct.c +++ b/src/launch-direct.c @@ -33,9 +33,12 @@ #include <sys/socket.h> #include <sys/un.h> #include <grp.h> +#include <assert.h> #include <pcre.h> +#include <libxml/uri.h> + #include "cloexec.h" #include "ignore-value.h" @@ -101,6 +104,53 @@ static int qemu_supports_device (guestfs_h *g, struct backend_direct_data *, con static int qemu_supports_virtio_scsi (guestfs_h *g, struct backend_direct_data *); static char *qemu_escape_param (guestfs_h *g, const char *param); +static char * +create_cow_overlay_direct (guestfs_h *g, void *datav, struct drive *drv) +{ + char *overlay = NULL; + CLEANUP_FREE char *backing_drive = NULL; + CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); + int r; + + backing_drive = guestfs___drive_source_qemu_param (g, &drv->src); + if (!backing_drive) + goto error; + + if (guestfs___lazy_make_tmpdir (g) == -1) + goto error; + + overlay = safe_asprintf (g, "%s/overlay%d", g->tmpdir, ++g->unique); + + guestfs___cmd_add_arg (cmd, "qemu-img"); + guestfs___cmd_add_arg (cmd, "create"); + guestfs___cmd_add_arg (cmd, "-f"); + guestfs___cmd_add_arg (cmd, "qcow2"); + guestfs___cmd_add_arg (cmd, "-b"); + guestfs___cmd_add_arg (cmd, backing_drive); + if (drv->src.format) { + guestfs___cmd_add_arg (cmd, "-o"); + guestfs___cmd_add_arg_format (cmd, "backing_fmt=%s", drv->src.format); + } + guestfs___cmd_add_arg (cmd, overlay); + r = guestfs___cmd_run (cmd); + if (r == -1) + goto error; + if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { + guestfs___external_command_failed (g, r, "qemu-img create", backing_drive); + goto error; + } + + /* Caller sets g->overlay in the handle to this, and then manages + * the memory. + */ + return overlay; + + error: + free (overlay); + + return NULL; +} + #ifdef QEMU_OPTIONS /* Like 'add_cmdline' but allowing a shell-quoted string of zero or * more options. XXX The unquoting is not very clever. @@ -470,23 +520,35 @@ launch_direct (guestfs_h *g, void *datav, const char *arg) ITER_DRIVES (g, i, drv) { CLEANUP_FREE char *file = NULL, *escaped_file = NULL, *param = NULL; - /* Make the file= parameter. */ - file = guestfs___drive_source_qemu_param (g, &drv->src); - escaped_file = qemu_escape_param (g, file); + if (!drv->overlay) { + /* Make the file= parameter. */ + file = guestfs___drive_source_qemu_param (g, &drv->src); + escaped_file = qemu_escape_param (g, file); - /* Make the first part of the -drive parameter, everything up to - * the if=... at the end. - */ - param = safe_asprintf - (g, "file=%s%s,cache=%s%s%s%s%s,id=hd%zu", - escaped_file, - drv->readonly ? ",snapshot=on" : "", - drv->cachemode ? drv->cachemode : "writeback", - drv->format ? ",format=" : "", - drv->format ? drv->format : "", - drv->disk_label ? ",serial=" : "", - drv->disk_label ? drv->disk_label : "", - i); + /* Make the first part of the -drive parameter, everything up to + * the if=... at the end. + */ + param = safe_asprintf + (g, "file=%s%s,cache=%s%s%s%s%s,id=hd%zu", + escaped_file, + drv->readonly ? ",snapshot=on" : "", + drv->cachemode ? drv->cachemode : "writeback", + drv->src.format ? ",format=" : "", + drv->src.format ? drv->src.format : "", + drv->disk_label ? ",serial=" : "", + drv->disk_label ? drv->disk_label : "", + i); + } + else { + /* Writable qcow2 overlay on top of read-only drive. */ + escaped_file = qemu_escape_param (g, drv->overlay); + param = safe_asprintf + (g, "file=%s,cache=unsafe,format=qcow2%s%s,id=hd%zu", + escaped_file, + drv->disk_label ? ",serial=" : "", + drv->disk_label ? drv->disk_label : "", + i); + } /* If there's an explicit 'iface', use it. Otherwise default to * virtio-scsi if available. Otherwise default to virtio-blk. @@ -1130,6 +1192,186 @@ qemu_escape_param (guestfs_h *g, const char *param) return ret; } +static char * +make_uri (guestfs_h *g, const char *scheme, const char *user, + struct drive_server *server, const char *path) +{ + xmlURI uri = { .scheme = (char *) scheme, + .path = (char *) path, + .user = (char *) user }; + CLEANUP_FREE char *query = NULL; + + switch (server->transport) { + case drive_transport_none: + case drive_transport_tcp: + uri.server = server->u.hostname; + uri.port = server->port; + break; + case drive_transport_unix: + query = safe_asprintf (g, "socket=%s", server->u.socket); + uri.query_raw = query; + break; + } + + return (char *) xmlSaveUri (&uri); +} + +/* Useful function to format a drive + protocol for qemu. Also shared + * with launch-libvirt.c. + * + * Note that the qemu parameter is the bit after "file=". It is not + * escaped here, but would usually be escaped if passed to qemu as + * part of a full -drive parameter (but not for qemu-img). + */ +char * +guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) +{ + char *path; + + switch (src->protocol) { + case drive_protocol_file: + /* We have to convert the path to an absolute path, since + * otherwise qemu will look for the backing file relative to the + * overlay (which is located in g->tmpdir). + * + * As a side-effect this deals with paths that contain ':' since + * qemu will not process the ':' if the path begins with '/'. + */ + path = realpath (src->u.path, NULL); + if (path == NULL) { + perrorf (g, _("realpath: could not convert '%s' to absolute path"), + src->u.path); + return NULL; + } + return path; + + case drive_protocol_ftp: + return make_uri (g, "ftp", src->username, + &src->servers[0], src->u.exportname); + + case drive_protocol_ftps: + return make_uri (g, "ftps", src->username, + &src->servers[0], src->u.exportname); + + case drive_protocol_gluster: + switch (src->servers[0].transport) { + case drive_transport_none: + return make_uri (g, "gluster", NULL, &src->servers[0], src->u.exportname); + case drive_transport_tcp: + return make_uri (g, "gluster+tcp", + NULL, &src->servers[0], src->u.exportname); + case drive_transport_unix: + return make_uri (g, "gluster+unix", NULL, &src->servers[0], NULL); + } + + case drive_protocol_http: + return make_uri (g, "http", src->username, + &src->servers[0], src->u.exportname); + + case drive_protocol_https: + return make_uri (g, "https", src->username, + &src->servers[0], src->u.exportname); + + case drive_protocol_iscsi: + return make_uri (g, "iscsi", NULL, &src->servers[0], src->u.exportname); + + case drive_protocol_nbd: { + CLEANUP_FREE char *p = NULL; + char *ret; + + switch (src->servers[0].transport) { + case drive_transport_none: + case drive_transport_tcp: + p = safe_asprintf (g, "nbd:%s:%d", + src->servers[0].u.hostname, src->servers[0].port); + break; + case drive_transport_unix: + p = safe_asprintf (g, "nbd:unix:%s", src->servers[0].u.socket); + break; + } + assert (p); + + if (STREQ (src->u.exportname, "")) + ret = safe_strdup (g, p); + else + /* Skip the mandatory leading '/' character. */ + ret = safe_asprintf (g, "%s:exportname=%s", p, &src->u.exportname[1]); + + return ret; + } + + case drive_protocol_rbd: { + /* build the list of all the mon hosts */ + CLEANUP_FREE char *mon_host = NULL, *username = NULL, *secret = NULL; + const char *auth; + size_t n = 0; + size_t i, j; + + for (i = 0; i < src->nr_servers; i++) { + n += strlen (src->servers[i].u.hostname); + n += 8; /* for slashes, colons, & port numbers */ + } + n++; /* for \0 */ + mon_host = safe_malloc (g, n); + n = 0; + for (i = 0; i < src->nr_servers; i++) { + CLEANUP_FREE char *port = NULL; + + for (j = 0; j < strlen (src->servers[i].u.hostname); j++) + mon_host[n++] = src->servers[i].u.hostname[j]; + mon_host[n++] = '\\'; + mon_host[n++] = ':'; + port = safe_asprintf (g, "%d", src->servers[i].port); + for (j = 0; j < strlen (port); j++) + mon_host[n++] = port[j]; + + /* join each host with \; */ + if (i != src->nr_servers - 1) { + mon_host[n++] = '\\'; + mon_host[n++] = ';'; + } + } + mon_host[n] = '\0'; + + if (src->username) + username = safe_asprintf (g, ":id=%s", src->username); + if (src->secret) + secret = safe_asprintf (g, ":key=%s", src->secret); + if (username || secret) + auth = ":auth_supported=cephx\\;none"; + else + auth = ":auth_supported=none"; + + /* Skip the mandatory leading '/' character on exportname. */ + return safe_asprintf (g, "rbd:%s:mon_host=%s%s%s%s", + &src->u.exportname[1], + mon_host, + username ? username : "", + auth, + secret ? secret : ""); + } + + case drive_protocol_sheepdog: + /* Skip the mandatory leading '/' character on exportname. */ + if (src->nr_servers == 0) + return safe_asprintf (g, "sheepdog:%s", &src->u.exportname[1]); + else /* XXX How to pass multiple hosts? */ + return safe_asprintf (g, "sheepdog:%s:%d:%s", + src->servers[0].u.hostname, src->servers[0].port, + &src->u.exportname[1]); + + case drive_protocol_ssh: + return make_uri (g, "ssh", src->username, + &src->servers[0], src->u.exportname); + + case drive_protocol_tftp: + return make_uri (g, "tftp", src->username, + &src->servers[0], src->u.exportname); + } + + abort (); +} + static int shutdown_direct (guestfs_h *g, void *datav, int check_for_errors) { @@ -1196,6 +1438,7 @@ max_disks_direct (guestfs_h *g, void *datav) static struct backend_ops backend_direct_ops = { .data_size = sizeof (struct backend_direct_data), + .create_cow_overlay = create_cow_overlay_direct, .launch = launch_direct, .shutdown = shutdown_direct, .get_pid = get_pid_direct, diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index d900b6d..08e2a5d 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -108,18 +108,6 @@ struct backend_libvirt_data { char name[DOMAIN_NAME_LEN]; /* random name */ }; -/* Pointed to by 'struct drive *' -> priv field. */ -struct drive_libvirt { - /* The drive that we actually add. If using an overlay, then this - * might be different from drive->src. Call it 'real_src' so we - * don't confuse accesses to this with accesses to 'drive->src'. - */ - struct drive_source real_src; - - /* The format of the drive we add. */ - char *format; -}; - /* Parameters passed to construct_libvirt_xml and subfunctions. We * keep them all in a structure for convenience! */ @@ -146,9 +134,6 @@ static void libvirt_error (guestfs_h *g, const char *fs, ...) __attribute__((for static int is_custom_hv (guestfs_h *g); static int is_blk (const char *path); static void ignore_errors (void *ignore, virErrorPtr ignore2); -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); @@ -156,6 +141,80 @@ static void clear_socket_create_context (guestfs_h *g); static void selinux_warning (guestfs_h *g, const char *func, const char *selinux_op, const char *data); #endif +static char * +make_qcow2_overlay (guestfs_h *g, const char *backing_drive, + const char *format) +{ + CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); + char *overlay = NULL; + int r; + + if (guestfs___lazy_make_tmpdir (g) == -1) + return NULL; + + overlay = safe_asprintf (g, "%s/overlay%d", g->tmpdir, ++g->unique); + + guestfs___cmd_add_arg (cmd, "qemu-img"); + guestfs___cmd_add_arg (cmd, "create"); + guestfs___cmd_add_arg (cmd, "-f"); + guestfs___cmd_add_arg (cmd, "qcow2"); + guestfs___cmd_add_arg (cmd, "-b"); + guestfs___cmd_add_arg (cmd, backing_drive); + if (format) { + guestfs___cmd_add_arg (cmd, "-o"); + guestfs___cmd_add_arg_format (cmd, "backing_fmt=%s", format); + } + guestfs___cmd_add_arg (cmd, overlay); + r = guestfs___cmd_run (cmd); + if (r == -1) { + free (overlay); + return NULL; + } + if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { + guestfs___external_command_failed (g, r, "qemu-img create", backing_drive); + free (overlay); + return NULL; + } + + return overlay; +} + +static char * +create_cow_overlay_libvirt (guestfs_h *g, void *datav, struct drive *drv) +{ + struct backend_libvirt_data *data = datav; + CLEANUP_FREE char *backing_drive = NULL; + char *overlay = NULL; + + backing_drive = guestfs___drive_source_qemu_param (g, &drv->src); + if (!backing_drive) + goto error; + + overlay = make_qcow2_overlay (g, backing_drive, drv->src.format); + if (!overlay) + goto error; + +#if HAVE_LIBSELINUX + if (data->selinux_imagelabel) { + debug (g, "setting SELinux label on %s to %s", + overlay, data->selinux_imagelabel); + if (setfilecon (overlay, + (security_context_t) data->selinux_imagelabel) == -1) + selinux_warning (g, __func__, "setfilecon", overlay); + } +#endif + + /* Caller sets g->overlay in the handle to this, and then manages + * the memory. + */ + return overlay; + + error: + free (overlay); + + return NULL; +} + static int launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) { @@ -178,8 +237,6 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) int r; uint32_t size; CLEANUP_FREE void *buf = NULL; - struct drive *drv; - size_t i; params.current_proc_is_root = geteuid () == 0; @@ -274,19 +331,11 @@ launch_libvirt (guestfs_h *g, void *datav, const char *libvirt_uri) /* Note that appliance can be NULL if using the old-style appliance. */ if (appliance) { - params.appliance_overlay = make_qcow2_overlay (g, appliance, "raw", NULL); + params.appliance_overlay = make_qcow2_overlay (g, appliance, "raw"); 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_drive_priv (g, drv, data->selinux_imagelabel) == -1) - goto cleanup; - } - TRACE0 (launch_build_libvirt_qcow2_overlay_end); /* Using virtio-serial, we need to create a local Unix domain socket @@ -746,6 +795,9 @@ static int construct_libvirt_xml_lifecycle (guestfs_h *g, const struct libvirt_x static int construct_libvirt_xml_devices (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo); static int construct_libvirt_xml_qemu_cmdline (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo); static int construct_libvirt_xml_disk (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo, struct drive *drv, size_t drv_index); +static int construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); +static int construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo, const char *format, const char *cachemode); +static int construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, size_t drv_index); static int construct_libvirt_xml_disk_source_hosts (guestfs_h *g, xmlTextWriterPtr xo, const struct drive_source *src); static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const struct backend_libvirt_data *data, xmlTextWriterPtr xo); static int construct_libvirt_xml_appliance (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo); @@ -756,7 +808,8 @@ static int construct_libvirt_xml_appliance (guestfs_h *g, const struct libvirt_x */ #define XMLERROR_RET(code,e,ret) do { \ if ((e) == (code)) { \ - perrorf (g, _("error constructing libvirt XML at \"%s\""), \ + perrorf (g, _("%s:%d: error constructing libvirt XML at \"%s\""), \ + __FILE__, __LINE__, \ #e); \ return (ret); \ } \ @@ -1111,12 +1164,10 @@ construct_libvirt_xml_disk (guestfs_h *g, xmlTextWriterPtr xo, struct drive *drv, size_t drv_index) { - char drive_name[64] = "sd"; const char *protocol_str; - char scsi_target[64]; - struct drive_libvirt *drv_priv = (struct drive_libvirt *) drv->priv; - CLEANUP_FREE char *format = NULL; + CLEANUP_FREE char *path = NULL; int is_host_device; + CLEANUP_FREE char *format = NULL; /* XXX We probably could support this if we thought about it some more. */ if (drv->iface) { @@ -1124,175 +1175,185 @@ construct_libvirt_xml_disk (guestfs_h *g, return -1; } - guestfs___drive_name (drv_index, &drive_name[2]); - snprintf (scsi_target, sizeof scsi_target, "%zu", drv_index); - XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "disk")); XMLERROR (-1, xmlTextWriterWriteAttribute (xo, BAD_CAST "device", BAD_CAST "disk")); - switch (drv_priv->real_src.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]> + if (drv->overlay) { + /* Overlay to protect read-only backing disk. The format of the + * overlay is always qcow2. */ - is_host_device = is_blk (drv_priv->real_src.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->real_src.u.path)); - if (construct_libvirt_xml_disk_source_seclabel (g, data, 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->real_src.u.path)); - if (construct_libvirt_xml_disk_source_seclabel (g, data, xo) == -1) - return -1; - XMLERROR (-1, xmlTextWriterEndElement (xo)); - } - break; - - /* For network protocols: - * <disk type=network device=disk> - * <source protocol=[protocol] [name=exportname]> - * and then zero or more of: - * <host name='example.com' port='10809'/> - * or: - * <host transport='unix' socket='/path/to/socket'/> - */ - case drive_protocol_gluster: - protocol_str = "gluster"; goto network_protocols; - case drive_protocol_iscsi: - protocol_str = "iscsi"; goto network_protocols; - case drive_protocol_nbd: - protocol_str = "nbd"; goto network_protocols; - case drive_protocol_rbd: - protocol_str = "rbd"; goto network_protocols; - case drive_protocol_sheepdog: - protocol_str = "sheepdog"; goto network_protocols; - case drive_protocol_ssh: - protocol_str = "ssh"; - /*FALLTHROUGH*/ - network_protocols: XMLERROR (-1, xmlTextWriterWriteAttribute (xo, BAD_CAST "type", - BAD_CAST "network")); + BAD_CAST "file")); + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "source")); XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "protocol", - BAD_CAST protocol_str)); - if (STRNEQ (drv_priv->real_src.u.exportname, "")) - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "name", - BAD_CAST drv_priv->real_src.u.exportname)); - if (construct_libvirt_xml_disk_source_hosts (g, xo, - &drv_priv->real_src) == -1) - return -1; + xmlTextWriterWriteAttribute (xo, BAD_CAST "file", + BAD_CAST drv->overlay)); if (construct_libvirt_xml_disk_source_seclabel (g, data, xo) == -1) return -1; XMLERROR (-1, xmlTextWriterEndElement (xo)); - if (drv_priv->real_src.username != NULL) { - XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "auth")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "username", - BAD_CAST drv_priv->real_src.username)); - /* TODO: write the drive secret, after first storing it separately - * in libvirt + + if (construct_libvirt_xml_disk_target (g, xo, drv_index) == -1) + return -1; + + if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2", "unsafe") == -1) + return -1; + } + else { + /* Not an overlay, a writable disk. */ + + switch (drv->src.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->src.u.path); + + if (!is_host_device) { + path = realpath (drv->src.u.path, NULL); + if (path == NULL) { + perrorf (g, _("realpath: could not convert '%s' to absolute path"), + drv->src.u.path); + return -1; + } + + 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 path)); + if (construct_libvirt_xml_disk_source_seclabel (g, data, 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->src.u.path)); + if (construct_libvirt_xml_disk_source_seclabel (g, data, xo) == -1) + return -1; + XMLERROR (-1, xmlTextWriterEndElement (xo)); + } + break; + + /* For network protocols: + * <disk type=network device=disk> + * <source protocol=[protocol] [name=exportname]> + * and then zero or more of: + * <host name='example.com' port='10809'/> + * or: + * <host transport='unix' socket='/path/to/socket'/> */ + case drive_protocol_gluster: + protocol_str = "gluster"; goto network_protocols; + case drive_protocol_iscsi: + protocol_str = "iscsi"; goto network_protocols; + case drive_protocol_nbd: + protocol_str = "nbd"; goto network_protocols; + case drive_protocol_rbd: + protocol_str = "rbd"; goto network_protocols; + case drive_protocol_sheepdog: + protocol_str = "sheepdog"; goto network_protocols; + case drive_protocol_ssh: + protocol_str = "ssh"; + /*FALLTHROUGH*/ + network_protocols: + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "type", + BAD_CAST "network")); + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "source")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "protocol", + BAD_CAST protocol_str)); + if (STRNEQ (drv->src.u.exportname, "")) + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "name", + BAD_CAST drv->src.u.exportname)); + if (construct_libvirt_xml_disk_source_hosts (g, xo, + &drv->src) == -1) + return -1; + if (construct_libvirt_xml_disk_source_seclabel (g, data, xo) == -1) + return -1; XMLERROR (-1, xmlTextWriterEndElement (xo)); - } - break; - - /* libvirt doesn't support the qemu curl driver yet. Give a - * reasonable error message instead of trying and failing. - */ - case drive_protocol_ftp: - case drive_protocol_ftps: - case drive_protocol_http: - case drive_protocol_https: - case drive_protocol_tftp: - error (g, _("libvirt does not support the qemu curl driver protocols (ftp, http, etc.); try setting LIBGUESTFS_BACKEND=direct")); - return -1; - } + if (drv->src.username != NULL) { + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "auth")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "username", + BAD_CAST drv->src.username)); + /* TODO: write the drive secret, after first storing it separately + * in libvirt + */ + XMLERROR (-1, xmlTextWriterEndElement (xo)); + } + break; - XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "target")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "dev", - BAD_CAST drive_name)); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "bus", - BAD_CAST "scsi")); - XMLERROR (-1, xmlTextWriterEndElement (xo)); + /* libvirt doesn't support the qemu curl driver yet. Give a + * reasonable error message instead of trying and failing. + */ + case drive_protocol_ftp: + case drive_protocol_ftps: + case drive_protocol_http: + case drive_protocol_https: + case drive_protocol_tftp: + error (g, _("libvirt does not support the qemu curl driver protocols (ftp, http, etc.); try setting LIBGUESTFS_BACKEND=direct")); + return -1; + } - XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "driver")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "name", - BAD_CAST "qemu")); - if (drv_priv->format) { - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "type", - BAD_CAST drv_priv->format)); - } - else if (drv_priv->real_src.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 - * have to do format detection here using qemu-img and pass that to - * libvirt. - * - * This is still a security issue, so in most cases it is recommended - * 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->real_src.u.path); - if (!format) + if (construct_libvirt_xml_disk_target (g, xo, drv_index) == -1) return -1; - if (STREQ (format, "unknown")) { - error (g, _("could not auto-detect the format.\n" + if (drv->src.format) + format = safe_strdup (g, drv->src.format); + else if (drv->src.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 + * have to do format detection here using qemu-img and pass that to + * libvirt. + * + * This is still a security issue, so in most cases it is recommended + * 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->src.u.path); + if (!format) + return -1; + + if (STREQ (format, "unknown")) { + 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'.")); + return -1; + } + } + 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; } - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "type", - BAD_CAST format)); + if (construct_libvirt_xml_disk_driver_qemu (g, xo, format, + drv->cachemode ? : "writeback") + == -1) + return -1; } - 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; - } - - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "cache", - BAD_CAST (drv->cachemode ? - drv->cachemode : - "writeback"))); - - XMLERROR (-1, xmlTextWriterEndElement (xo)); if (drv->disk_label) { XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "serial")); @@ -1300,6 +1361,62 @@ construct_libvirt_xml_disk (guestfs_h *g, XMLERROR (-1, xmlTextWriterEndElement (xo)); } + if (construct_libvirt_xml_disk_address (g, xo, drv_index) == -1) + return -1; + + XMLERROR (-1, xmlTextWriterEndElement (xo)); /* </disk> */ + + return 0; +} + +static int +construct_libvirt_xml_disk_target (guestfs_h *g, xmlTextWriterPtr xo, + size_t drv_index) +{ + char drive_name[64] = "sd"; + + guestfs___drive_name (drv_index, &drive_name[2]); + + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "target")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "dev", + BAD_CAST drive_name)); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "bus", + BAD_CAST "scsi")); + XMLERROR (-1, xmlTextWriterEndElement (xo)); + + return 0; +} + +static int +construct_libvirt_xml_disk_driver_qemu (guestfs_h *g, xmlTextWriterPtr xo, + const char *format, + const char *cachemode) +{ + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "driver")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "name", + BAD_CAST "qemu")); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "type", + BAD_CAST format)); + XMLERROR (-1, + xmlTextWriterWriteAttribute (xo, BAD_CAST "cache", + BAD_CAST cachemode)); + XMLERROR (-1, xmlTextWriterEndElement (xo)); + + return 0; +} + +static int +construct_libvirt_xml_disk_address (guestfs_h *g, xmlTextWriterPtr xo, + size_t drv_index) +{ + char scsi_target[64]; + + snprintf (scsi_target, sizeof scsi_target, "%zu", drv_index); + XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "address")); XMLERROR (-1, xmlTextWriterWriteAttribute (xo, BAD_CAST "type", @@ -1318,8 +1435,6 @@ construct_libvirt_xml_disk (guestfs_h *g, BAD_CAST "0")); XMLERROR (-1, xmlTextWriterEndElement (xo)); - XMLERROR (-1, xmlTextWriterEndElement (xo)); - return 0; } @@ -1398,10 +1513,6 @@ construct_libvirt_xml_appliance (guestfs_h *g, const struct libvirt_xml_params *params, xmlTextWriterPtr xo) { - char scsi_target[64]; - - snprintf (scsi_target, sizeof scsi_target, "%zu", params->appliance_index); - XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "disk")); XMLERROR (-1, xmlTextWriterWriteAttribute (xo, BAD_CAST "type", @@ -1425,46 +1536,15 @@ construct_libvirt_xml_appliance (guestfs_h *g, BAD_CAST "scsi")); XMLERROR (-1, xmlTextWriterEndElement (xo)); - XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "driver")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "name", - BAD_CAST "qemu")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "type", - BAD_CAST "qcow2")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "cache", - BAD_CAST "unsafe")); - XMLERROR (-1, xmlTextWriterEndElement (xo)); + if (construct_libvirt_xml_disk_driver_qemu (g, xo, "qcow2", "unsafe") == -1) + return -1; - XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "address")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "type", - BAD_CAST "drive")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "controller", - BAD_CAST "0")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "bus", - BAD_CAST "0")); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "target", - BAD_CAST scsi_target)); - XMLERROR (-1, - xmlTextWriterWriteAttribute (xo, BAD_CAST "unit", - BAD_CAST "0")); - XMLERROR (-1, xmlTextWriterEndElement (xo)); + if (construct_libvirt_xml_disk_address (g, xo, params->appliance_index) == -1) + return -1; XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "shareable")); XMLERROR (-1, xmlTextWriterEndElement (xo)); - /* We'd like to do this, but it's not supported by libvirt. - * See make_drive_priv for the workaround. - * - * XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST "transient")); - * XMLERROR (-1, xmlTextWriterEndElement (xo)); - */ - XMLERROR (-1, xmlTextWriterEndElement (xo)); return 0; @@ -1562,143 +1642,6 @@ ignore_errors (void *ignore, virErrorPtr ignore2) /* empty */ } -/* 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 *backing_device, - const char *format, const char *selinux_imagelabel) -{ - char *tmpfile = NULL; - CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); - int r; - - tmpfile = safe_asprintf (g, "%s/snapshot%d", g->tmpdir, ++g->unique); - - guestfs___cmd_add_arg (cmd, "qemu-img"); - guestfs___cmd_add_arg (cmd, "create"); - guestfs___cmd_add_arg (cmd, "-f"); - guestfs___cmd_add_arg (cmd, "qcow2"); - guestfs___cmd_add_arg (cmd, "-b"); - 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); - } - guestfs___cmd_add_arg (cmd, tmpfile); - r = guestfs___cmd_run (cmd); - if (r == -1) - goto error; - if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { - guestfs___external_command_failed (g, r, "qemu-img create", backing_device); - goto error; - } - -#if HAVE_LIBSELINUX - if (selinux_imagelabel) { - debug (g, "setting SELinux label on %s to %s", - tmpfile, selinux_imagelabel); - if (setfilecon (tmpfile, (security_context_t) selinux_imagelabel) == -1) - selinux_warning (g, __func__, "setfilecon", tmpfile); - } -#endif - - return tmpfile; /* caller frees */ - - error: - free (tmpfile); - - 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_drive_priv (guestfs_h *g, struct drive *drv, - const char *selinux_imagelabel) -{ - char *path; - struct drive_libvirt *drv_priv; - - if (drv->priv && drv->free_priv) - drv->free_priv (drv->priv); - - drv->priv = drv_priv = safe_calloc (g, 1, sizeof (struct drive_libvirt)); - drv->free_priv = drive_free_priv; - - switch (drv->src.protocol) { - case drive_protocol_file: - - /* Even for non-readonly paths, we need to make the paths absolute here. */ - path = realpath (drv->src.u.path, NULL); - if (path == NULL) { - perrorf (g, _("realpath: could not convert '%s' to absolute path"), - drv->src.u.path); - return -1; - } - - drv_priv->real_src.protocol = drive_protocol_file; - - if (!drv->readonly) { - drv_priv->real_src.u.path = path; - drv_priv->format = drv->format ? safe_strdup (g, drv->format) : NULL; - } - else { - drv_priv->real_src.u.path = make_qcow2_overlay (g, path, drv->format, - selinux_imagelabel); - free (path); - if (!drv_priv->real_src.u.path) - return -1; - drv_priv->format = safe_strdup (g, "qcow2"); - } - break; - - case drive_protocol_ftp: - case drive_protocol_ftps: - case drive_protocol_gluster: - case drive_protocol_http: - case drive_protocol_https: - case drive_protocol_iscsi: - case drive_protocol_nbd: - case drive_protocol_rbd: - case drive_protocol_sheepdog: - case drive_protocol_ssh: - case drive_protocol_tftp: - if (!drv->readonly) { - guestfs___copy_drive_source (g, &drv->src, &drv_priv->real_src); - drv_priv->format = drv->format ? safe_strdup (g, drv->format) : NULL; - } - else { - CLEANUP_FREE char *qemu_device = NULL; - - drv_priv->real_src.protocol = drive_protocol_file; - qemu_device = guestfs___drive_source_qemu_param (g, &drv->src); - drv_priv->real_src.u.path = make_qcow2_overlay (g, qemu_device, - drv->format, - selinux_imagelabel); - if (!drv_priv->real_src.u.path) - return -1; - drv_priv->format = safe_strdup (g, "qcow2"); - } - break; - } - - return 0; -} - -static void -drive_free_priv (void *priv) -{ - struct drive_libvirt *drv_priv = priv; - - guestfs___free_drive_source (&drv_priv->real_src); - free (drv_priv->format); - free (drv_priv); -} - static int shutdown_libvirt (guestfs_h *g, void *datav, int check_for_errors) { @@ -1805,9 +1748,6 @@ hot_add_drive_libvirt (guestfs_h *g, void *datav, return -1; } - if (make_drive_priv (g, drv, data->selinux_imagelabel) == -1) - return -1; - /* Create the XML for the new disk. */ xml = construct_libvirt_xml_hot_add_disk (g, data, drv, drv_index); if (xml == NULL) @@ -1908,6 +1848,7 @@ set_libvirt_selinux_norelabel_disks (guestfs_h *g, void *datav, int flag) static struct backend_ops backend_libvirt_ops = { .data_size = sizeof (struct backend_libvirt_data), + .create_cow_overlay = create_cow_overlay_libvirt, .launch = launch_libvirt, .shutdown = shutdown_libvirt, .max_disks = max_disks_libvirt, diff --git a/src/launch-uml.c b/src/launch-uml.c index 023d033..9b2a004 100644 --- a/src/launch-uml.c +++ b/src/launch-uml.c @@ -50,7 +50,42 @@ struct backend_uml_data { }; static void print_vmlinux_command_line (guestfs_h *g, char **argv); -static char *make_cow_overlay (guestfs_h *g, const char *original); + +/* Run uml_mkcow to create a COW overlay. */ +static char * +make_cow_overlay (guestfs_h *g, const char *original) +{ + CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); + char *overlay; + int r; + + if (guestfs___lazy_make_tmpdir (g) == -1) + return NULL; + + overlay = safe_asprintf (g, "%s/overlay%d", g->tmpdir, g->unique++); + + guestfs___cmd_add_arg (cmd, "uml_mkcow"); + guestfs___cmd_add_arg (cmd, overlay); + guestfs___cmd_add_arg (cmd, original); + r = guestfs___cmd_run (cmd); + if (r == -1) { + free (overlay); + return NULL; + } + if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { + guestfs___external_command_failed (g, r, "uml_mkcow", original); + free (overlay); + return NULL; + } + + return overlay; +} + +static char * +create_cow_overlay_uml (guestfs_h *g, void *datav, struct drive *drv) +{ + return make_cow_overlay (g, drv->src.u.path); +} /* Test for features which are not supported by the UML backend. * Possibly some of these should just be warnings, not errors. @@ -75,7 +110,7 @@ uml_supported (guestfs_h *g) error (g, _("uml backend does not support remote drives")); return false; } - if (drv->format && STRNEQ (drv->format, "raw")) { + if (drv->src.format && STRNEQ (drv->src.format, "raw")) { error (g, _("uml backend does not support non-raw-format drives")); return false; } @@ -132,20 +167,10 @@ launch_uml (guestfs_h *g, void *datav, const char *arg) return -1; has_appliance_drive = appliance != NULL; - /* Create COW overlays for any readonly drives, and for the root. - * Note that the documented syntax ubd0=cow,orig does not work since - * kernel 3.3. See: + /* Create COW overlays for the appliance. Note that the documented + * syntax ubd0=cow,orig does not work since kernel 3.3. See: * http://thread.gmane.org/gmane.linux.uml.devel/13556 */ - ITER_DRIVES (g, i, drv) { - if (drv->readonly) { - drv->priv = make_cow_overlay (g, drv->src.u.path); - if (!drv->priv) - goto cleanup0; - drv->free_priv = free; - } - } - if (has_appliance_drive) { appliance_cow = make_cow_overlay (g, appliance); if (!appliance_cow) @@ -219,10 +244,10 @@ launch_uml (guestfs_h *g, void *datav, const char *arg) /* Add the drives. */ ITER_DRIVES (g, i, drv) { - if (!drv->readonly) + if (!drv->overlay) ADD_CMDLINE_PRINTF ("ubd%zu=%s", i, drv->src.u.path); else - ADD_CMDLINE_PRINTF ("ubd%zu=%s", i, (char *) drv->priv); + ADD_CMDLINE_PRINTF ("ubd%zu=%s", i, drv->overlay); } /* Add the ext2 appliance drive (after all the drives). */ @@ -479,35 +504,6 @@ launch_uml (guestfs_h *g, void *datav, const char *arg) return -1; } -/* Run uml_mkcow to create a COW overlay. This works around a kernel - * bug in UML option parsing. - */ -static char * -make_cow_overlay (guestfs_h *g, const char *original) -{ - CLEANUP_CMD_CLOSE struct command *cmd = guestfs___new_command (g); - char *cow; - int r; - - cow = safe_asprintf (g, "%s/cow%d", g->tmpdir, g->unique++); - - guestfs___cmd_add_arg (cmd, "uml_mkcow"); - guestfs___cmd_add_arg (cmd, cow); - guestfs___cmd_add_arg (cmd, original); - r = guestfs___cmd_run (cmd); - if (r == -1) { - free (cow); - return NULL; - } - if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) { - guestfs___external_command_failed (g, r, "uml_mkcow", original); - free (cow); - return NULL; - } - - return cow; /* caller must free */ -} - /* This is called from the forked subprocess just before vmlinux runs, * so it can just print the message straight to stderr, where it will * be picked up and funnelled through the usual appliance event API. @@ -605,6 +601,7 @@ max_disks_uml (guestfs_h *g, void *datav) static struct backend_ops backend_uml_ops = { .data_size = sizeof (struct backend_uml_data), + .create_cow_overlay = create_cow_overlay_uml, .launch = launch_uml, .shutdown = shutdown_uml, .get_pid = get_pid_uml, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 238f64d..ace2e00 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -235,7 +235,10 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom, if ((doc = get_domain_xml (g, dom)) == NULL) return -1; - /* Find and pass the SELinux security label to the libvirt back end. */ + /* Find and pass the SELinux security label to the libvirt back end. + * Note this has to happen before adding the disks, since those may + * use the label. + */ if (libvirt_selinux_label (g, doc, &label, &imagelabel) == -1) return -1; if (label && imagelabel) { -- 1.8.4.2