This series improves ceph rbd support in libguestfs. It uses the servers list, adds support for a custom username, and starts to add support for custom secret.
Mike Kelly
2013-May-07 14:50 UTC
[Libguestfs] [PATCH 1/5] rbd: send mon_host for rbd drives
This is how the servers are passed to kvm. --- src/drives.c | 37 +++++++++++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/src/drives.c b/src/drives.c index 0e62ca8..0a6d956 100644 --- a/src/drives.c +++ b/src/drives.c @@ -1086,12 +1086,37 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) return ret; } - case drive_protocol_rbd: - /* XXX Although libvirt allows multiple hosts to be specified, - * it's unclear how these are ever passed to Ceph. Perhaps via - * environment variables? - */ - return safe_asprintf (g, "rbd:%s", src->u.exportname); + case drive_protocol_rbd: { + /* build the list of all the mon hosts */ + CLEANUP_FREE char *mon_host = NULL; + size_t n = 0; + for (int 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, sizeof (char *) * n); + n = 0; + for (int i = 0; i < src->nr_servers; i++) { + for (int 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++] = ':'; + CLEANUP_FREE char *port = safe_asprintf (g, "%d", src->servers[i].port); + for (int 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'; + + return safe_asprintf (g, "rbd:%s:mon_host=%s", src->u.exportname, mon_host); + } case drive_protocol_sheepdog: if (src->nr_servers == 0) -- 1.7.9.5
Mike Kelly
2013-May-07 14:50 UTC
[Libguestfs] [PATCH 2/5] libvirt: initial support for networked disks
--- src/libvirt-domain.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 5 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0ec0cad..8e94541 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -40,7 +40,7 @@ #if defined(HAVE_LIBVIRT) static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom); -static ssize_t for_each_disk (guestfs_h *g, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, void *data), void *data); +static ssize_t for_each_disk (guestfs_h *g, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, void *data), void *data); static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char **label_rtn, char **imagelabel_rtn); static void @@ -139,7 +139,7 @@ guestfs__add_domain (guestfs_h *g, const char *domain_name, return r; } -static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly, void *data); +static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, void *data); static int connect_live (guestfs_h *g, virDomainPtr dom); enum readonlydisk { @@ -268,6 +268,7 @@ guestfs___add_libvirt_dom (guestfs_h *g, virDomainPtr dom, static int add_disk (guestfs_h *g, const char *filename, const char *format, int readonly_in_xml, + const char *protocol, char *const *server, const char *username, void *datavp) { struct add_disk_data *data = datavp; @@ -313,6 +314,18 @@ add_disk (guestfs_h *g, optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_FORMAT_BITMASK; optargs.format = format; } + if (protocol) { + optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_PROTOCOL_BITMASK; + optargs.protocol = protocol; + } + if (server) { + optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SERVER_BITMASK; + optargs.server = server; + } + if (username) { + optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK; + optargs.username = username; + } return guestfs__add_drive_opts (g, filename, &optargs); } @@ -404,6 +417,8 @@ for_each_disk (guestfs_h *g, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, + const char *protocol, char *const *server, + const char *username, void *data), void *data) { @@ -432,11 +447,14 @@ for_each_disk (guestfs_h *g, if (nodes != NULL) { nr_nodes = nodes->nodeNr; for (i = 0; i < nr_nodes; ++i) { - CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL; + CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL, *protocol = NULL, *username = NULL; + CLEANUP_FREE_STRING_LIST char **server = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpreadonly = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpfilename = NULL; + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpprotocol = NULL; + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xphost = NULL; xmlAttrPtr attr; int readonly; int t; @@ -479,8 +497,66 @@ for_each_disk (guestfs_h *g, xpfilename->nodesetval->nodeNr == 0) { continue; /* disk filename not found, skip this */ } + } else if (STREQ (type, "network")) { /* type = "network", use source/@name */ + debug(g, _("disk[%zu]: network device"), i); + xpathCtx->node = nodes->nodeTab[i]; + xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@name", + xpathCtx); + if (xpfilename == NULL || + xpfilename->nodesetval == NULL || + xpfilename->nodesetval->nodeNr == 0) { + continue; + } + + xpprotocol = xmlXPathEvalExpression (BAD_CAST "./source/@protocol", + xpathCtx); + /* Get the protocol (e.g. "rbd"). */ + if (xpprotocol == NULL || + xpprotocol->nodesetval == NULL || + xpprotocol->nodesetval->nodeNr == 0) { + continue; + } + assert (xpprotocol->nodesetval->nodeTab[0]); + assert (xpprotocol->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); + attr = (xmlAttrPtr) xpprotocol->nodesetval->nodeTab[0]; + protocol = (char *) xmlNodeListGetString (doc, attr->children, 1); + debug(g, _("disk[%zu]: protocol: %s"), i, protocol); + + xphost = xmlXPathEvalExpression (BAD_CAST "./source/host", + xpathCtx); + if (xphost == NULL || + xphost->nodesetval == NULL || + xphost->nodesetval->nodeNr == 0) { + continue; + } + /* This gives us a list of <host> elements, which each have a + * 'name' and 'port' attribute which we want to put into a + * string, joined by a ':'. + */ + server = safe_malloc (g, sizeof (char *) * (xphost->nodesetval->nodeNr + 1)); + for (int hi = 0; hi < xphost->nodesetval->nodeNr ; hi++) { + xmlChar *name, *port; + xmlNodePtr h = xphost->nodesetval->nodeTab[hi]; + assert (h); + assert (h->type == XML_ELEMENT_NODE); + name = xmlGetProp(h, "name"); + assert(name); + port = xmlGetProp(h, "port"); + assert(port); + debug(g, _("disk[%zu]: host: %s:%s"), i, name, port); + if (asprintf(&server[hi], "%s:%s", name, port) == -1) { + perror("asprintf"); + return -1; + } + } + server[xphost->nodesetval->nodeNr] = NULL; + + /* + * TODO: usernames, secrets: ./auth/secret/@type, + * ./auth/secret/@usage || ./auth/secret/@uuid + */ } else - continue; /* type <> "file" or "block", skip it */ + continue; /* type <> "file", "block", or "network", skip it */ assert (xpfilename); assert (xpfilename->nodesetval); @@ -488,6 +564,7 @@ for_each_disk (guestfs_h *g, assert (xpfilename->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0]; filename = (char *) xmlNodeListGetString (doc, attr->children, 1); + debug(g, _("disk[%zu]: filename: %s"), i, filename); /* Get the disk format (may not be set). */ xpathCtx->node = nodes->nodeTab[i]; @@ -511,7 +588,7 @@ for_each_disk (guestfs_h *g, readonly = 1; if (f) - t = f (g, filename, format, readonly, data); + t = f (g, filename, format, readonly, protocol, server, username, data); else t = 0; -- 1.7.9.5
Mike Kelly
2013-May-07 14:50 UTC
[Libguestfs] [PATCH 3/5] rbd: support usernames, for cephx authentication
--- generator/actions.ml | 13 +++++++------ src/drives.c | 18 +++++++++++------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/generator/actions.ml b/generator/actions.ml index 0bb04be..daea6b6 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -1352,6 +1352,7 @@ See also: L<guestfs(3)/NETWORK BLOCK DEVICE>. Connect to the Ceph (librbd/RBD) server. The C<server> parameter must also be supplied - see below. +The C<username> parameter may be supplied. See below. See also: L<guestfs(3)/CEPH>. @@ -1401,13 +1402,13 @@ for the protocol is used (see C</etc/services>). =item C<username> -For the C<ssh> protocol only, this specifies the remote username. +For the C<ssh> and C<rbd> protocols only, this specifies the remote username. -If not given, then the local username is used. But note this sometimes -may give unexpected results, for example if using the libvirt backend -and if the libvirt backend is configured to start the qemu appliance -as a special user such as C<qemu.qemu>. If in doubt, specify the -remote username you want. +If not given, then the local username is used for C<ssh>, and 'admin' is used +for C<rbd>. But note this sometimes may give unexpected results, for example +if using the libvirt backend and if the libvirt backend is configured to start +the qemu appliance as a special user such as C<qemu.qemu>. If in doubt, +specify the remote username you want. =back" }; diff --git a/src/drives.c b/src/drives.c index 0a6d956..a510e1e 100644 --- a/src/drives.c +++ b/src/drives.c @@ -221,11 +221,6 @@ create_drive_rbd (guestfs_h *g, { size_t i; - if (username != NULL) { - error (g, _("rbd: you cannot specify a username with this protocol")); - return NULL; - } - if (nr_servers == 0) { error (g, _("rbd: you must specify one or more servers")); return NULL; @@ -1088,7 +1083,8 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) case drive_protocol_rbd: { /* build the list of all the mon hosts */ - CLEANUP_FREE char *mon_host = NULL; + CLEANUP_FREE char *mon_host = NULL, *username = NULL; + char *auth; size_t n = 0; for (int i = 0; i < src->nr_servers; i++) { n += strlen (src->servers[i].u.hostname); @@ -1115,7 +1111,15 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) } mon_host[n] = '\0'; - return safe_asprintf (g, "rbd:%s:mon_host=%s", src->u.exportname, mon_host); + if (src->username) + username = safe_asprintf (g, ":id=%s", src->username); + if (username) + auth = ":auth_supported=cephx\\;none"; + else + auth = ":auth_supported=none"; + + return safe_asprintf (g, "rbd:%s:mon_host=%s%s%s", src->u.exportname, mon_host, + username ? username : "", auth); } case drive_protocol_sheepdog: -- 1.7.9.5
Mike Kelly
2013-May-07 14:50 UTC
[Libguestfs] [PATCH 4/5] libvirt: extract usernames for network disks
--- src/libvirt-domain.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 8e94541..e56a98e 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -455,6 +455,7 @@ for_each_disk (guestfs_h *g, CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpfilename = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpprotocol = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xphost = NULL; + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpusername = NULL; xmlAttrPtr attr; int readonly; int t; @@ -522,6 +523,18 @@ for_each_disk (guestfs_h *g, protocol = (char *) xmlNodeListGetString (doc, attr->children, 1); debug(g, _("disk[%zu]: protocol: %s"), i, protocol); + xpusername = xmlXPathEvalExpression (BAD_CAST "./auth/@username", + xpathCtx); + if (xpusername != NULL && + xpusername->nodesetval != NULL && + xpusername->nodesetval->nodeNr != 0) { + assert (xpusername->nodesetval->nodeTab[0]); + assert (xpusername->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); + attr = (xmlAttrPtr) xpusername->nodesetval->nodeTab[0]; + username = (char *) xmlNodeListGetString (doc, attr->children, 1); + debug(g, _("disk[%zu]: username: %s"), i, username); + } + xphost = xmlXPathEvalExpression (BAD_CAST "./source/host", xpathCtx); if (xphost == NULL || @@ -552,7 +565,7 @@ for_each_disk (guestfs_h *g, server[xphost->nodesetval->nodeNr] = NULL; /* - * TODO: usernames, secrets: ./auth/secret/@type, + * TODO: secrets: ./auth/secret/@type, * ./auth/secret/@usage || ./auth/secret/@uuid */ } else -- 1.7.9.5
Mike Kelly
2013-May-07 14:50 UTC
[Libguestfs] [PATCH 5/5] rbd: Support using a custom secret
--- generator/actions.ml | 20 ++++++++--- src/drives.c | 86 +++++++++++++++++++++++++++++++++++++----------- src/guestfs-internal.h | 2 ++ src/guestfs.pod | 5 ++- src/launch-libvirt.c | 3 ++ 5 files changed, 90 insertions(+), 26 deletions(-) diff --git a/generator/actions.ml b/generator/actions.ml index daea6b6..b0b4be7 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"; OString "protocol"; OStringList "server"; OString "username"]; + style = RErr, [String "filename"], [OBool "readonly"; OString "format"; OString "iface"; OString "name"; OString "label"; OString "protocol"; OStringList "server"; OString "username"; OString "secret"]; once_had_no_optargs = true; blocking = false; fish_alias = ["add"]; @@ -1353,6 +1353,7 @@ See also: L<guestfs(3)/NETWORK BLOCK DEVICE>. Connect to the Ceph (librbd/RBD) server. The C<server> parameter must also be supplied - see below. The C<username> parameter may be supplied. See below. +The C<secret> parameter may be supplied. See below. See also: L<guestfs(3)/CEPH>. @@ -1404,12 +1405,21 @@ for the protocol is used (see C</etc/services>). For the C<ssh> and C<rbd> protocols only, this specifies the remote username. -If not given, then the local username is used for C<ssh>, and 'admin' is used -for C<rbd>. But note this sometimes may give unexpected results, for example -if using the libvirt backend and if the libvirt backend is configured to start -the qemu appliance as a special user such as C<qemu.qemu>. If in doubt, +If not given, then the local username is used for C<ssh>, and no authentication +is attempted for ceph. But note this sometimes may give unexpected results, for +example if using the libvirt backend and if the libvirt backend is configured to +start the qemu appliance as a special user such as C<qemu.qemu>. If in doubt, specify the remote username you want. +=item C<secret> + +For the C<rbd> protocol only, this specifies the 'secret' to use when +connecting to the remote device. + +If not given, then a secret matching the given username will be looked up in the +default keychain locations, or if no username is given, then no authentication +will be used. + =back" }; { defaults with diff --git a/src/drives.c b/src/drives.c index a510e1e..1429050 100644 --- a/src/drives.c +++ b/src/drives.c @@ -107,7 +107,8 @@ static struct drive * create_drive_non_file (guestfs_h *g, enum drive_protocol protocol, struct drive_server *servers, size_t nr_servers, - const char *exportname, const char *username, + const char *exportname, + const char *username, const char *secret, bool readonly, const char *format, const char *iface, const char *name, const char *disk_label, @@ -120,6 +121,7 @@ create_drive_non_file (guestfs_h *g, drv->src.nr_servers = nr_servers; 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->readonly = readonly; drv->format = format ? safe_strdup (g, format) : NULL; @@ -136,7 +138,8 @@ create_drive_non_file (guestfs_h *g, static struct drive * create_drive_gluster (guestfs_h *g, struct drive_server *servers, size_t nr_servers, - const char *exportname, const char *username, + const char *exportname, + const char *username, const char *secret, bool readonly, const char *format, const char *iface, const char *name, const char *disk_label, @@ -146,6 +149,11 @@ create_drive_gluster (guestfs_h *g, error (g, _("gluster: you cannot specify a username with this protocol")); return NULL; } + if (secret != NULL) { + error (g, _("gluster: you cannot specify a secret with this protocol")); + return NULL; + } + if (nr_servers != 1) { error (g, _("gluster: you must specify exactly one server")); @@ -165,7 +173,8 @@ create_drive_gluster (guestfs_h *g, } return create_drive_non_file (g, drive_protocol_gluster, - servers, nr_servers, exportname, username, + servers, nr_servers, exportname, + username, secret, readonly, format, iface, name, disk_label, use_cache_none); } @@ -185,7 +194,8 @@ nbd_port (void) static struct drive * create_drive_nbd (guestfs_h *g, struct drive_server *servers, size_t nr_servers, - const char *exportname, const char *username, + const char *exportname, + const char *username, const char *secret, bool readonly, const char *format, const char *iface, const char *name, const char *disk_label, @@ -195,6 +205,10 @@ create_drive_nbd (guestfs_h *g, error (g, _("nbd: you cannot specify a username with this protocol")); return NULL; } + if (secret != NULL) { + error (g, _("nbd: you cannot specify a secret with this protocol")); + return NULL; + } if (nr_servers != 1) { error (g, _("nbd: you must specify exactly one server")); @@ -205,7 +219,8 @@ create_drive_nbd (guestfs_h *g, servers[0].port = nbd_port (); return create_drive_non_file (g, drive_protocol_nbd, - servers, nr_servers, exportname, username, + servers, nr_servers, exportname, + username, secret, readonly, format, iface, name, disk_label, use_cache_none); } @@ -213,7 +228,8 @@ create_drive_nbd (guestfs_h *g, static struct drive * create_drive_rbd (guestfs_h *g, struct drive_server *servers, size_t nr_servers, - const char *exportname, const char *username, + const char *exportname, + const char *username, const char *secret, bool readonly, const char *format, const char *iface, const char *name, const char *disk_label, @@ -244,7 +260,8 @@ create_drive_rbd (guestfs_h *g, } return create_drive_non_file (g, drive_protocol_rbd, - servers, nr_servers, exportname, username, + servers, nr_servers, exportname, + username, secret, readonly, format, iface, name, disk_label, use_cache_none); } @@ -252,7 +269,8 @@ create_drive_rbd (guestfs_h *g, static struct drive * create_drive_sheepdog (guestfs_h *g, struct drive_server *servers, size_t nr_servers, - const char *exportname, const char *username, + const char *exportname, + const char *username, const char *secret, bool readonly, const char *format, const char *iface, const char *name, const char *disk_label, @@ -264,6 +282,10 @@ create_drive_sheepdog (guestfs_h *g, error (g, _("sheepdog: you cannot specify a username with this protocol")); return NULL; } + if (secret != NULL) { + error (g, _("sheepdog: you cannot specify a secret with this protocol")); + return NULL; + } for (i = 0; i < nr_servers; ++i) { if (servers[i].transport != drive_transport_none && @@ -283,7 +305,8 @@ create_drive_sheepdog (guestfs_h *g, } return create_drive_non_file (g, drive_protocol_sheepdog, - servers, nr_servers, exportname, username, + servers, nr_servers, exportname, + username, secret, readonly, format, iface, name, disk_label, use_cache_none); } @@ -291,12 +314,18 @@ create_drive_sheepdog (guestfs_h *g, static struct drive * create_drive_ssh (guestfs_h *g, struct drive_server *servers, size_t nr_servers, - const char *exportname, const char *username, + const char *exportname, + const char *username, const char *secret, bool readonly, const char *format, const char *iface, const char *name, const char *disk_label, bool use_cache_none) { + if (secret != NULL) { + error (g, _("ssh: you cannot specify a secret with this protocol")); + return NULL; + } + if (nr_servers != 1) { error (g, _("ssh: you must specify exactly one server")); return NULL; @@ -319,7 +348,8 @@ create_drive_ssh (guestfs_h *g, } return create_drive_non_file (g, drive_protocol_ssh, - servers, nr_servers, exportname, username, + servers, nr_servers, exportname, + username, secret, readonly, format, iface, name, disk_label, use_cache_none); } @@ -690,6 +720,7 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, size_t nr_servers = 0; struct drive_server *servers = NULL; const char *username; + const char *secret; int use_cache_none; struct drive *drv; size_t i, drv_index; @@ -720,6 +751,8 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, } username = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK ? optargs->username : NULL; + secret = optargs->bitmask & GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK + ? optargs->secret : NULL; if (format && !valid_format_iface (format)) { error (g, _("%s parameter is empty or contains disallowed characters"), @@ -750,6 +783,11 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, free_drive_servers (servers, nr_servers); return -1; } + if (secret != NULL) { + error (g, _("you cannot specify a secret with file-backed disks")); + free_drive_servers (servers, nr_servers); + return -1; + } if (STREQ (filename, "/dev/null")) drv = create_drive_dev_null (g, readonly, format, iface, name, @@ -775,27 +813,32 @@ guestfs__add_drive_opts (guestfs_h *g, const char *filename, } } else if (STREQ (protocol, "gluster")) { - drv = create_drive_gluster (g, servers, nr_servers, filename, username, + drv = create_drive_gluster (g, servers, nr_servers, filename, + username, secret, readonly, format, iface, name, disk_label, false); } else if (STREQ (protocol, "nbd")) { - drv = create_drive_nbd (g, servers, nr_servers, filename, username, + drv = create_drive_nbd (g, servers, nr_servers, filename, + username, secret, readonly, format, iface, name, disk_label, false); } else if (STREQ (protocol, "rbd")) { - drv = create_drive_rbd (g, servers, nr_servers, filename, username, + drv = create_drive_rbd (g, servers, nr_servers, filename, + username, secret, readonly, format, iface, name, disk_label, false); } else if (STREQ (protocol, "sheepdog")) { - drv = create_drive_sheepdog (g, servers, nr_servers, filename, username, + drv = create_drive_sheepdog (g, servers, nr_servers, filename, + username, secret, readonly, format, iface, name, disk_label, false); } else if (STREQ (protocol, "ssh")) { - drv = create_drive_ssh (g, servers, nr_servers, filename, username, + drv = create_drive_ssh (g, servers, nr_servers, filename, + username, secret, readonly, format, iface, name, disk_label, false); } @@ -1083,7 +1126,7 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) case drive_protocol_rbd: { /* build the list of all the mon hosts */ - CLEANUP_FREE char *mon_host = NULL, *username = NULL; + CLEANUP_FREE char *mon_host = NULL, *username = NULL, *secret = NULL; char *auth; size_t n = 0; for (int i = 0; i < src->nr_servers; i++) { @@ -1113,13 +1156,15 @@ guestfs___drive_source_qemu_param (guestfs_h *g, const struct drive_source *src) if (src->username) username = safe_asprintf (g, ":id=%s", src->username); - if (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"; - return safe_asprintf (g, "rbd:%s:mon_host=%s%s%s", src->u.exportname, mon_host, - username ? username : "", auth); + return safe_asprintf (g, "rbd:%s:mon_host=%s%s%s%s", src->u.exportname, mon_host, + username ? username : "", auth, secret ? secret : ""); } case drive_protocol_sheepdog: @@ -1155,6 +1200,7 @@ guestfs___free_drive_source (struct drive_source *src) if (src) { free (src->u.path); free (src->username); + free (src->secret); free_drive_servers (src->servers, src->nr_servers); } } diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index e13c498..6a58f2f 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -161,6 +161,8 @@ struct drive_source { /* Optional username (may be NULL if not specified). */ char *username; + /* Optional secret (may be NULL if not specified). */ + char *secret; }; struct drive { diff --git a/src/guestfs.pod b/src/guestfs.pod index ee9cc2a..99374c2 100644 --- a/src/guestfs.pod +++ b/src/guestfs.pod @@ -652,11 +652,14 @@ L</guestfs_add_drive_opts> like this: GUESTFS_ADD_DRIVE_OPTS_FORMAT, "raw", GUESTFS_ADD_DRIVE_OPTS_PROTOCOL, "rbd", GUESTFS_ADD_DRIVE_OPTS_SERVER, servers, + GUESTFS_ADD_DRIVE_OPTS_USERNAME, "rbduser", + GUESTFS_ADD_DRIVE_OPTS_SECRET, "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA==", -1); C<servers> (the C<server> parameter) is a list of one or more Ceph servers. The server string is documented in -L</guestfs_add_drive_opts>. +L</guestfs_add_drive_opts>. The C<username> and C<secret> parameters are +also optional, and if not given, then no authentication will be used. =head3 GLUSTER diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c index c2bd81c..e33ddc6 100644 --- a/src/launch-libvirt.c +++ b/src/launch-libvirt.c @@ -1144,6 +1144,9 @@ construct_libvirt_xml_disk (guestfs_h *g, 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 + */ XMLERROR (-1, xmlTextWriterEndElement (xo)); } } -- 1.7.9.5
On Tue, May 07, 2013 at 10:50:07AM -0400, Mike Kelly wrote:> This series improves ceph rbd support in libguestfs. It uses the servers > list, adds support for a custom username, and starts to add support for > custom secret.I will push these, but I am making a few changes, mainly of style: - Put variables at the top of scope. - Spaces before parentheses. - Use 'size_t' instead of 'int' for counting in arrays and strings (as a general rule, using 'int' in modern C code is almost always wrong). - const correctness and casting in a few places (use: ./configure --enable-gcc-warnings) - Will test that everything still works under valgrind (but not the rbd changes themselves, since I don't have a Ceph server around). One worry I have is whether quoting is required for the server name(s), export name, username and secret. Thanks, Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
On Thu, May 09, 2013 at 11:23:55AM -0400, Mike Kelly wrote:> On Wed, May 8, 2013 at 6:53 AM, Richard W.M. Jones <rjones at redhat.com> wrote: > > One worry I have is whether quoting is required for the server > > name(s), export name, username and secret. > > Well. I think the main things we had to quote were ':' and ';', but > none of those are valid in a hostname. Username also probably doesn't > contain anything special, and secret is a base64-encoded string. I > confirmed that even with the string ending in '==', it was parsed just > fine by qemu, at least in my limited manual testing. > > If you can suggest a way to be more robust this, though, then I can > try to work that into a future patch series.The quoting problem happens when someone writes a program which takes (eg) a hostname string from the user and passes it unmodified to the guestfs API. It's an issue if this string can cause unexpected [even malicious/exploitable] things to happen when passed unquoted on the qemu command line. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Seemingly Similar Threads
- [PATCH 0/7] Various fixes for Ceph drives and parsing libvirt XML.
- [PATCH 0/3] lib: Don't assert fail if port is missing in XML (RHBZ#1370424).
- [PATCH] libvirt: read disk paths from pools (RHBZ#1366049)
- [PATCH v2] libvirt: read disk paths from pools (RHBZ#1366049)
- [PATCH 1/2] libvirt: un-duplicate XPath code