Pino Toscano
2016-Nov-16 11:59 UTC
[Libguestfs] [PATCH 1/2] libvirt: un-duplicate XPath code
Move the checks for empty xmlXPathObjectPtr, and for extracting the
result string out of it, to a new helper functions.
This is just code motion, there should be no behaviour changes.
---
src/libvirt-domain.c | 122 +++++++++++++++++++++------------------------------
1 file changed, 50 insertions(+), 72 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4d4142d..baab307 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -43,6 +43,8 @@ static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr
dom);
static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, 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 char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char
*pool_nane, const char *volume_name);
+static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj);
+static char *xPathObjectGetString (xmlDocPtr doc, xmlXPathObjectPtr obj);
static void
ignore_errors (void *ignore, virErrorPtr ignore2)
@@ -513,7 +515,6 @@ for_each_disk (guestfs_h *g,
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpusername = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppool = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL;
- xmlAttrPtr attr;
int readonly;
int t;
@@ -527,31 +528,21 @@ for_each_disk (guestfs_h *g,
* Check the <disk type=..> attribute first to find out which one.
*/
xptype = xmlXPathEvalExpression (BAD_CAST "./@type", xpathCtx);
- if (xptype == NULL ||
- xptype->nodesetval == NULL ||
- xptype->nodesetval->nodeNr == 0) {
+ if (xPathObjectIsEmpty (xptype))
continue; /* no type attribute, skip it */
- }
- assert (xptype->nodesetval->nodeTab[0]);
- assert (xptype->nodesetval->nodeTab[0]->type ==
XML_ATTRIBUTE_NODE);
- attr = (xmlAttrPtr) xptype->nodesetval->nodeTab[0];
- type = (char *) xmlNodeListGetString (doc, attr->children, 1);
+ type = xPathObjectGetString (doc, xptype);
if (STREQ (type, "file")) { /* type = "file" so look
at source/@file */
xpathCtx->node = nodes->nodeTab[i];
xpfilename = xmlXPathEvalExpression (BAD_CAST
"./source/@file",
xpathCtx);
- if (xpfilename == NULL ||
- xpfilename->nodesetval == NULL ||
- xpfilename->nodesetval->nodeNr == 0)
+ if (xPathObjectIsEmpty (xpfilename))
continue; /* disk filename not found, skip this */
} else if (STREQ (type, "block")) { /* type =
"block", use source/@dev */
xpathCtx->node = nodes->nodeTab[i];
xpfilename = xmlXPathEvalExpression (BAD_CAST
"./source/@dev",
xpathCtx);
- if (xpfilename == NULL ||
- xpfilename->nodesetval == NULL ||
- xpfilename->nodesetval->nodeNr == 0)
+ if (xPathObjectIsEmpty (xpfilename))
continue; /* disk filename not found, skip this */
} else if (STREQ (type, "network")) { /* type =
"network", use source/@name */
int hi;
@@ -562,15 +553,9 @@ for_each_disk (guestfs_h *g,
/* Get the protocol (e.g. "rbd"). Required. */
xpprotocol = xmlXPathEvalExpression (BAD_CAST
"./source/@protocol",
xpathCtx);
- if (xpprotocol == NULL ||
- xpprotocol->nodesetval == NULL ||
- xpprotocol->nodesetval->nodeNr == 0)
+ if (xPathObjectIsEmpty (xpprotocol))
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);
+ protocol = xPathObjectGetString (doc, xpprotocol);
debug (g, "disk[%zu]: protocol: %s", i, protocol);
/* <source name="..."> is the path/exportname.
Optional. */
@@ -583,13 +568,8 @@ for_each_disk (guestfs_h *g,
/* <auth username="...">. Optional. */
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);
+ if (!xPathObjectIsEmpty (xpusername)) {
+ username = xPathObjectGetString (doc, xpusername);
debug (g, "disk[%zu]: username: %s", i, username);
}
@@ -641,28 +621,16 @@ for_each_disk (guestfs_h *g,
/* Get the source pool. Required. */
xppool = xmlXPathEvalExpression (BAD_CAST "./source/@pool",
xpathCtx);
- if (xppool == NULL ||
- xppool->nodesetval == NULL ||
- xppool->nodesetval->nodeNr == 0)
+ if (xPathObjectIsEmpty (xppool))
continue;
- assert (xppool->nodesetval->nodeTab[0]);
- assert (xppool->nodesetval->nodeTab[0]->type =-
XML_ATTRIBUTE_NODE);
- attr = (xmlAttrPtr) xppool->nodesetval->nodeTab[0];
- pool = (char *) xmlNodeListGetString (doc, attr->children, 1);
+ pool = xPathObjectGetString (doc, xppool);
/* Get the source volume. Required. */
xpvolume = xmlXPathEvalExpression (BAD_CAST
"./source/@volume",
xpathCtx);
- if (xpvolume == NULL ||
- xpvolume->nodesetval == NULL ||
- xpvolume->nodesetval->nodeNr == 0)
+ if (xPathObjectIsEmpty (xpvolume))
continue;
- assert (xpvolume->nodesetval->nodeTab[0]);
- assert (xpvolume->nodesetval->nodeTab[0]->type =-
XML_ATTRIBUTE_NODE);
- attr = (xmlAttrPtr) xpvolume->nodesetval->nodeTab[0];
- volume = (char *) xmlNodeListGetString (doc, attr->children, 1);
+ volume = xPathObjectGetString (doc, xpvolume);
debug (g, "disk[%zu]: pool: %s; volume: %s", i, pool,
volume);
@@ -679,13 +647,8 @@ for_each_disk (guestfs_h *g,
if (filename == NULL) {
assert (xpfilename);
assert (xpfilename->nodesetval);
- if (xpfilename->nodesetval->nodeNr > 0) {
- assert (xpfilename->nodesetval->nodeTab[0]);
- assert (xpfilename->nodesetval->nodeTab[0]->type =-
XML_ATTRIBUTE_NODE);
- attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0];
- filename = (char *) xmlNodeListGetString (doc, attr->children, 1);
- }
+ if (xpfilename->nodesetval->nodeNr > 0)
+ filename = xPathObjectGetString (doc, xpfilename);
else
/* For network protocols (eg. nbd), name may be omitted. */
filename = safe_strdup (g, "");
@@ -696,22 +659,14 @@ for_each_disk (guestfs_h *g,
/* Get the disk format (may not be set). */
xpathCtx->node = nodes->nodeTab[i];
xpformat = xmlXPathEvalExpression (BAD_CAST "./driver/@type",
xpathCtx);
- if (xpformat != NULL &&
- xpformat->nodesetval &&
- xpformat->nodesetval->nodeNr > 0) {
- assert (xpformat->nodesetval->nodeTab[0]);
- assert (xpformat->nodesetval->nodeTab[0]->type ==
XML_ATTRIBUTE_NODE);
- attr = (xmlAttrPtr) xpformat->nodesetval->nodeTab[0];
- format = (char *) xmlNodeListGetString (doc, attr->children, 1);
- }
+ if (!xPathObjectIsEmpty (xpformat))
+ format = xPathObjectGetString (doc, xpformat);
/* Get the <readonly/> flag. */
xpathCtx->node = nodes->nodeTab[i];
xpreadonly = xmlXPathEvalExpression (BAD_CAST "./readonly",
xpathCtx);
readonly = 0;
- if (xpreadonly != NULL &&
- xpreadonly->nodesetval &&
- xpreadonly->nodesetval->nodeNr > 0)
+ if (!xPathObjectIsEmpty (xpreadonly))
readonly = 1;
if (f)
@@ -774,23 +729,17 @@ connect_live (guestfs_h *g, virDomainPtr dom)
if (nodes != NULL) {
for (i = 0; i < nodes->nodeNr; ++i) {
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL;
- xmlAttrPtr attr;
/* See note in function above. */
xpathCtx->node = nodes->nodeTab[i];
/* The path is in <source path=..> attribute. */
xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path",
xpathCtx);
- if (xppath == NULL ||
- xppath->nodesetval == NULL ||
- xppath->nodesetval->nodeNr == 0) {
+ if (xPathObjectIsEmpty (xppath)) {
xmlXPathFreeObject (xppath);
continue; /* no type attribute, skip it */
}
- assert (xppath->nodesetval->nodeTab[0]);
- assert (xppath->nodesetval->nodeTab[0]->type ==
XML_ATTRIBUTE_NODE);
- attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0];
- path = (char *) xmlNodeListGetString (doc, attr->children, 1);
+ path = xPathObjectGetString (doc, xppath);
break;
}
}
@@ -888,6 +837,35 @@ filename_from_pool (guestfs_h *g, virConnectPtr conn,
return filename;
}
+/* Check that C<obj> is not empty.
+ */
+static bool
+xPathObjectIsEmpty (xmlXPathObjectPtr obj)
+{
+ return obj == NULL ||
+ obj->nodesetval == NULL ||
+ obj->nodesetval->nodeNr == 0;
+}
+
+/* Get the string value from C<obj>.
+ *
+ * C<obj> is I<required> to not be empty, i.e. that
C<xPathObjectIsEmpty>
+ * is C<false>.
+ */
+static char *
+xPathObjectGetString (xmlDocPtr doc, xmlXPathObjectPtr obj)
+{
+ xmlAttrPtr attr;
+ char *value;
+
+ assert (obj->nodesetval->nodeTab[0]);
+ assert (obj->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE);
+ attr = (xmlAttrPtr) obj->nodesetval->nodeTab[0];
+ value = (char *) xmlNodeListGetString (doc, attr->children, 1);
+
+ return value;
+}
+
#else /* no libvirt at compile time */
#define NOT_IMPL(r) \
--
2.7.4
Pino Toscano
2016-Nov-16 11:59 UTC
[Libguestfs] [PATCH 2/2] libvirt: read secrets of disks (RHBZ#1392798)
Read also the secrets associated to disks (<secret> tag within
<auth>),
so qemu can properly open them later on.
---
src/libvirt-domain.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 122 insertions(+), 8 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index baab307..696a264 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -33,6 +33,8 @@
#include <libxml/parser.h>
#include <libxml/tree.h>
+#include "base64.h"
+
#include "guestfs.h"
#include "guestfs-internal.h"
#include "guestfs-internal-actions.h"
@@ -40,7 +42,7 @@
#if defined(HAVE_LIBVIRT)
static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom);
-static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, 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 ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc,
int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly,
const char *protocol, char *const *server, const char *username, const char
*secret, void *data), void *data);
static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char
**label_rtn, char **imagelabel_rtn);
static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char
*pool_nane, const char *volume_name);
static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj);
@@ -95,8 +97,12 @@ guestfs_impl_add_domain (guestfs_h *g, const char
*domain_name,
return -1;
}
- /* Connect to libvirt, find the domain. */
- conn = guestfs_int_open_libvirt_connection (g, libvirturi, VIR_CONNECT_RO);
+ /* Connect to libvirt, find the domain. We cannot open the connection
+ * in read-only mode (VIR_CONNECT_RO), as that kind of connection
+ * is considered untrusted, and thus libvirt will prevent to read
+ * the values of secrets.
+ */
+ conn = guestfs_int_open_libvirt_connection (g, libvirturi, 0);
if (!conn) {
err = virGetLastError ();
error (g, _("could not connect to libvirt (code %d, domain %d):
%s"),
@@ -163,7 +169,7 @@ guestfs_impl_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, const char *protocol, char *const *server, const char *username,
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,
const char *secret, void *data);
static int connect_live (guestfs_h *g, virDomainPtr dom);
enum readonlydisk {
@@ -325,7 +331,7 @@ 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)
+ const char *secret, void *datavp)
{
struct add_disk_data *data = datavp;
/* Copy whole struct so we can make local changes: */
@@ -382,6 +388,10 @@ add_disk (guestfs_h *g,
optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK;
optargs.username = username;
}
+ if (secret) {
+ optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK;
+ optargs.secret = secret;
+ }
return guestfs_add_drive_opts_argv (g, filename, &optargs);
}
@@ -475,7 +485,7 @@ for_each_disk (guestfs_h *g,
const char *filename, const char *format,
int readonly,
const char *protocol, char *const *server,
- const char *username,
+ const char *username, const char *secret,
void *data),
void *data)
{
@@ -504,7 +514,7 @@ 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,
*protocol = NULL, *username = NULL;
+ CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL,
*protocol = NULL, *username = NULL, *secret = NULL;
CLEANUP_FREE_STRING_LIST char **server = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL;
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL;
@@ -517,6 +527,7 @@ for_each_disk (guestfs_h *g,
CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL;
int readonly;
int t;
+ virErrorPtr err;
/* Change the context to the current <disk> node.
* DV advises to reset this before each search since older versions of
@@ -569,8 +580,111 @@ for_each_disk (guestfs_h *g,
xpusername = xmlXPathEvalExpression (BAD_CAST
"./auth/@username",
xpathCtx);
if (!xPathObjectIsEmpty (xpusername)) {
+ CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpsecrettype = NULL;
+ CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpsecretuuid = NULL;
+ CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpsecretusage = NULL;
+ CLEANUP_FREE char *typestr = NULL;
+ unsigned char *value = NULL;
+ size_t value_size = 0;
+
username = xPathObjectGetString (doc, xpusername);
debug (g, "disk[%zu]: username: %s", i, username);
+
+ /* <secret type="...">. Mandatory given <auth>
is specified. */
+ xpsecrettype = xmlXPathEvalExpression (BAD_CAST
"./auth/secret/@type",
+ xpathCtx);
+ if (xPathObjectIsEmpty (xpsecrettype))
+ continue;
+ typestr = xPathObjectGetString (doc, xpsecrettype);
+
+ /* <secret uuid="..."> and <secret
usage="...">.
+ * At least one of them is required.
+ */
+ xpsecretuuid = xmlXPathEvalExpression (BAD_CAST
"./auth/secret/@uuid",
+ xpathCtx);
+ xpsecretusage = xmlXPathEvalExpression (BAD_CAST
"./auth/secret/@usage",
+ xpathCtx);
+ if (!xPathObjectIsEmpty (xpsecretuuid)) {
+ CLEANUP_FREE char *uuidstr = NULL;
+ virSecretPtr sec;
+
+ uuidstr = xPathObjectGetString (doc, xpsecretuuid);
+ debug (g, "disk[%zu]: secret type: %s; UUID: %s",
+ i, typestr, uuidstr);
+ sec = virSecretLookupByUUIDString (conn, uuidstr);
+ if (sec == NULL) {
+ err = virGetLastError ();
+ error (g, _("no secret with UUID '%s': %s"),
+ uuidstr, err ? err->message : "(none)");
+ continue;
+ }
+
+ value = virSecretGetValue (sec, &value_size, 0);
+ if (value == NULL) {
+ err = virGetLastError ();
+ error (g, _("cannot get the value of the secret with UUID
'%s': %s"),
+ uuidstr, err->message);
+ virSecretFree (sec);
+ continue;
+ }
+
+ virSecretFree (sec);
+ } else if (!xPathObjectIsEmpty (xpsecretusage)) {
+ virSecretUsageType usageType;
+ CLEANUP_FREE char *usagestr = NULL;
+ virSecretPtr sec;
+
+ usagestr = xPathObjectGetString (doc, xpsecretusage);
+ debug (g, "disk[%zu]: secret type: %s; usage: %s",
+ i, typestr, usagestr);
+ if (STREQ (usagestr, "none"))
+ usageType = VIR_SECRET_USAGE_TYPE_NONE;
+ else if (STREQ (usagestr, "volume"))
+ usageType = VIR_SECRET_USAGE_TYPE_VOLUME;
+ else if (STREQ (usagestr, "ceph"))
+ usageType = VIR_SECRET_USAGE_TYPE_CEPH;
+ else if (STREQ (usagestr, "iscsi"))
+ usageType = VIR_SECRET_USAGE_TYPE_ISCSI;
+ else
+ continue;
+ sec = virSecretLookupByUsage (conn, usageType, usagestr);
+ if (sec == NULL) {
+ err = virGetLastError ();
+ error (g, _("no secret for usage '%s': %s"),
+ usagestr, err->message);
+ continue;
+ }
+
+ value = virSecretGetValue (sec, &value_size, 0);
+ if (value == NULL) {
+ err = virGetLastError ();
+ error (g, _("cannot get the value of the secret with usage
'%s': %s"),
+ usagestr, err->message);
+ virSecretFree (sec);
+ continue;
+ }
+
+ virSecretFree (sec);
+ } else {
+ continue;
+ }
+
+ assert (value != NULL);
+ assert (value_size > 0);
+
+ if (STREQ (typestr, "ceph")) {
+ const size_t res = base64_encode_alloc ((const char *) value,
+ value_size, &secret);
+ free (value);
+ if (res == 0 || secret == NULL) {
+ error (g, "internal error: cannot encode the rbd secret as
base64");
+ return -1;
+ }
+ } else {
+ secret = (char *) value;
+ }
+
+ assert (secret != NULL);
}
xphost = xmlXPathEvalExpression (BAD_CAST "./source/host",
@@ -670,7 +784,7 @@ for_each_disk (guestfs_h *g,
readonly = 1;
if (f)
- t = f (g, filename, format, readonly, protocol, server, username,
data);
+ t = f (g, filename, format, readonly, protocol, server, username,
secret, data);
else
t = 0;
--
2.7.4
Richard W.M. Jones
2016-Dec-07 11:57 UTC
Re: [Libguestfs] [PATCH 1/2] libvirt: un-duplicate XPath code
On Wed, Nov 16, 2016 at 12:59:38PM +0100, Pino Toscano wrote:> Move the checks for empty xmlXPathObjectPtr, and for extracting the > result string out of it, to a new helper functions. > > This is just code motion, there should be no behaviour changes. > --- > src/libvirt-domain.c | 122 +++++++++++++++++++++------------------------------ > 1 file changed, 50 insertions(+), 72 deletions(-) > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index 4d4142d..baab307 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c > @@ -43,6 +43,8 @@ static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom); > static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, 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 char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char *pool_nane, const char *volume_name); > +static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj); > +static char *xPathObjectGetString (xmlDocPtr doc, xmlXPathObjectPtr obj);To be consistent with the rest of the code, xpath_object_is_empty etc. ACK with that changed. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com 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
Richard W.M. Jones
2016-Dec-07 11:59 UTC
Re: [Libguestfs] [PATCH 2/2] libvirt: read secrets of disks (RHBZ#1392798)
On Wed, Nov 16, 2016 at 12:59:39PM +0100, Pino Toscano wrote:> Read also the secrets associated to disks (<secret> tag within <auth>), > so qemu can properly open them later on.Looks good, ACK. Rich.> src/libvirt-domain.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 122 insertions(+), 8 deletions(-) > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > index baab307..696a264 100644 > --- a/src/libvirt-domain.c > +++ b/src/libvirt-domain.c > @@ -33,6 +33,8 @@ > #include <libxml/parser.h> > #include <libxml/tree.h> > > +#include "base64.h" > + > #include "guestfs.h" > #include "guestfs-internal.h" > #include "guestfs-internal-actions.h" > @@ -40,7 +42,7 @@ > #if defined(HAVE_LIBVIRT) > > static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom); > -static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, 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 ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, const char *secret, void *data), void *data); > static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char **label_rtn, char **imagelabel_rtn); > static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char *pool_nane, const char *volume_name); > static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj); > @@ -95,8 +97,12 @@ guestfs_impl_add_domain (guestfs_h *g, const char *domain_name, > return -1; > } > > - /* Connect to libvirt, find the domain. */ > - conn = guestfs_int_open_libvirt_connection (g, libvirturi, VIR_CONNECT_RO); > + /* Connect to libvirt, find the domain. We cannot open the connection > + * in read-only mode (VIR_CONNECT_RO), as that kind of connection > + * is considered untrusted, and thus libvirt will prevent to read > + * the values of secrets. > + */ > + conn = guestfs_int_open_libvirt_connection (g, libvirturi, 0); > if (!conn) { > err = virGetLastError (); > error (g, _("could not connect to libvirt (code %d, domain %d): %s"), > @@ -163,7 +169,7 @@ guestfs_impl_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, const char *protocol, char *const *server, const char *username, 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, const char *secret, void *data); > static int connect_live (guestfs_h *g, virDomainPtr dom); > > enum readonlydisk { > @@ -325,7 +331,7 @@ 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) > + const char *secret, void *datavp) > { > struct add_disk_data *data = datavp; > /* Copy whole struct so we can make local changes: */ > @@ -382,6 +388,10 @@ add_disk (guestfs_h *g, > optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_USERNAME_BITMASK; > optargs.username = username; > } > + if (secret) { > + optargs.bitmask |= GUESTFS_ADD_DRIVE_OPTS_SECRET_BITMASK; > + optargs.secret = secret; > + } > > return guestfs_add_drive_opts_argv (g, filename, &optargs); > } > @@ -475,7 +485,7 @@ for_each_disk (guestfs_h *g, > const char *filename, const char *format, > int readonly, > const char *protocol, char *const *server, > - const char *username, > + const char *username, const char *secret, > void *data), > void *data) > { > @@ -504,7 +514,7 @@ 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, *protocol = NULL, *username = NULL; > + CLEANUP_FREE char *type = NULL, *filename = NULL, *format = NULL, *protocol = NULL, *username = NULL, *secret = NULL; > CLEANUP_FREE_STRING_LIST char **server = NULL; > CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xptype = NULL; > CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpformat = NULL; > @@ -517,6 +527,7 @@ for_each_disk (guestfs_h *g, > CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL; > int readonly; > int t; > + virErrorPtr err; > > /* Change the context to the current <disk> node. > * DV advises to reset this before each search since older versions of > @@ -569,8 +580,111 @@ for_each_disk (guestfs_h *g, > xpusername = xmlXPathEvalExpression (BAD_CAST "./auth/@username", > xpathCtx); > if (!xPathObjectIsEmpty (xpusername)) { > + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpsecrettype = NULL; > + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpsecretuuid = NULL; > + CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpsecretusage = NULL; > + CLEANUP_FREE char *typestr = NULL; > + unsigned char *value = NULL; > + size_t value_size = 0; > + > username = xPathObjectGetString (doc, xpusername); > debug (g, "disk[%zu]: username: %s", i, username); > + > + /* <secret type="...">. Mandatory given <auth> is specified. */ > + xpsecrettype = xmlXPathEvalExpression (BAD_CAST "./auth/secret/@type", > + xpathCtx); > + if (xPathObjectIsEmpty (xpsecrettype)) > + continue; > + typestr = xPathObjectGetString (doc, xpsecrettype); > + > + /* <secret uuid="..."> and <secret usage="...">. > + * At least one of them is required. > + */ > + xpsecretuuid = xmlXPathEvalExpression (BAD_CAST "./auth/secret/@uuid", > + xpathCtx); > + xpsecretusage = xmlXPathEvalExpression (BAD_CAST "./auth/secret/@usage", > + xpathCtx); > + if (!xPathObjectIsEmpty (xpsecretuuid)) { > + CLEANUP_FREE char *uuidstr = NULL; > + virSecretPtr sec; > + > + uuidstr = xPathObjectGetString (doc, xpsecretuuid); > + debug (g, "disk[%zu]: secret type: %s; UUID: %s", > + i, typestr, uuidstr); > + sec = virSecretLookupByUUIDString (conn, uuidstr); > + if (sec == NULL) { > + err = virGetLastError (); > + error (g, _("no secret with UUID '%s': %s"), > + uuidstr, err ? err->message : "(none)"); > + continue; > + } > + > + value = virSecretGetValue (sec, &value_size, 0); > + if (value == NULL) { > + err = virGetLastError (); > + error (g, _("cannot get the value of the secret with UUID '%s': %s"), > + uuidstr, err->message); > + virSecretFree (sec); > + continue; > + } > + > + virSecretFree (sec); > + } else if (!xPathObjectIsEmpty (xpsecretusage)) { > + virSecretUsageType usageType; > + CLEANUP_FREE char *usagestr = NULL; > + virSecretPtr sec; > + > + usagestr = xPathObjectGetString (doc, xpsecretusage); > + debug (g, "disk[%zu]: secret type: %s; usage: %s", > + i, typestr, usagestr); > + if (STREQ (usagestr, "none")) > + usageType = VIR_SECRET_USAGE_TYPE_NONE; > + else if (STREQ (usagestr, "volume")) > + usageType = VIR_SECRET_USAGE_TYPE_VOLUME; > + else if (STREQ (usagestr, "ceph")) > + usageType = VIR_SECRET_USAGE_TYPE_CEPH; > + else if (STREQ (usagestr, "iscsi")) > + usageType = VIR_SECRET_USAGE_TYPE_ISCSI; > + else > + continue; > + sec = virSecretLookupByUsage (conn, usageType, usagestr); > + if (sec == NULL) { > + err = virGetLastError (); > + error (g, _("no secret for usage '%s': %s"), > + usagestr, err->message); > + continue; > + } > + > + value = virSecretGetValue (sec, &value_size, 0); > + if (value == NULL) { > + err = virGetLastError (); > + error (g, _("cannot get the value of the secret with usage '%s': %s"), > + usagestr, err->message); > + virSecretFree (sec); > + continue; > + } > + > + virSecretFree (sec); > + } else { > + continue; > + } > + > + assert (value != NULL); > + assert (value_size > 0); > + > + if (STREQ (typestr, "ceph")) { > + const size_t res = base64_encode_alloc ((const char *) value, > + value_size, &secret); > + free (value); > + if (res == 0 || secret == NULL) { > + error (g, "internal error: cannot encode the rbd secret as base64"); > + return -1; > + } > + } else { > + secret = (char *) value; > + } > + > + assert (secret != NULL); > } > > xphost = xmlXPathEvalExpression (BAD_CAST "./source/host", > @@ -670,7 +784,7 @@ for_each_disk (guestfs_h *g, > readonly = 1; > > if (f) > - t = f (g, filename, format, readonly, protocol, server, username, data); > + t = f (g, filename, format, readonly, protocol, server, username, secret, data); > else > t = 0; > > -- > 2.7.4 > > _______________________________________________ > Libguestfs mailing list > Libguestfs@redhat.com > https://www.redhat.com/mailman/listinfo/libguestfs-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Maybe Matching Threads
- [PATCH 2/2] libvirt: read secrets of disks (RHBZ#1392798)
- [PATCH] libvirt: read disk paths from pools (RHBZ#1366049)
- [PATCH v2] libvirt: read disk paths from pools (RHBZ#1366049)
- [PATCH 0/5] rbd improvements
- [PATCH 0/7] Various fixes for Ceph drives and parsing libvirt XML.