Libguestfs - Nov 2016 - [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

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

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

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