[libvirt] [PATCH 3/5] Utilize virDomainDiskAuth for domain disk

John Ferlan jferlan at redhat.com
Fri Jun 27 15:11:40 UTC 2014


Replace the inline "auth" struct in virStorageSource with a pointer
to a virStorageAuthDefPtr and utilize between the domain_conf, qemu_conf,
and qemu_command sources for finding the auth data for a domain disk

Signed-off-by: John Ferlan <jferlan at redhat.com>
---
 src/conf/domain_conf.c    | 106 +++++++---------------------------------------
 src/libvirt_private.syms  |   1 -
 src/qemu/qemu_command.c   |  72 +++++++++++++++++++++----------
 src/qemu/qemu_conf.c      |  26 +++++++-----
 src/util/virstoragefile.c |  14 +-----
 src/util/virstoragefile.h |  10 +----
 tests/qemuargv2xmltest.c  |   1 -
 7 files changed, 81 insertions(+), 149 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7aa4f5..93f09a7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5203,7 +5203,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 {
     virDomainDiskDefPtr def;
     xmlNodePtr sourceNode = NULL;
-    xmlNodePtr cur, child;
+    xmlNodePtr cur;
     xmlNodePtr save_ctxt = ctxt->node;
     char *type = NULL;
     char *device = NULL;
@@ -5227,10 +5227,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     virStorageEncryptionPtr encryption = NULL;
     char *serial = NULL;
     char *startupPolicy = NULL;
-    char *authUsername = NULL;
-    char *authUsage = NULL;
-    char *authUUID = NULL;
-    char *usageType = NULL;
+    virStorageAuthDefPtr authdef = NULL;
     char *tray = NULL;
     char *removable = NULL;
     char *logical_block_size = NULL;
@@ -5432,65 +5429,14 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                     VIR_FREE(ready);
                 }
             } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) {
-                authUsername = virXMLPropString(cur, "username");
-                if (authUsername == NULL) {
-                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                                   _("missing username for auth"));
+                if (!(authdef = virStorageAuthDefParse(node->doc, cur)))
+                    goto error;
+                if ((auth_secret_usage =
+                     virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
+                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                                   _("invalid secret type %s"),
+                                   authdef->secrettype);
                     goto error;
-                }
-
-                def->src->auth.secretType = VIR_STORAGE_SECRET_TYPE_NONE;
-                child = cur->children;
-                while (child != NULL) {
-                    if (child->type == XML_ELEMENT_NODE &&
-                        xmlStrEqual(child->name, BAD_CAST "secret")) {
-                        usageType = virXMLPropString(child, "type");
-                        if (usageType == NULL) {
-                            virReportError(VIR_ERR_XML_ERROR, "%s",
-                                           _("missing type for secret"));
-                            goto error;
-                        }
-                        auth_secret_usage =
-                            virSecretUsageTypeFromString(usageType);
-                        if (auth_secret_usage < 0) {
-                            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                                           _("invalid secret type %s"),
-                                           usageType);
-                            goto error;
-                        }
-
-                        authUUID = virXMLPropString(child, "uuid");
-                        authUsage = virXMLPropString(child, "usage");
-
-                        if (authUUID != NULL && authUsage != NULL) {
-                            virReportError(VIR_ERR_XML_ERROR, "%s",
-                                           _("only one of uuid and usage can be specified"));
-                            goto error;
-                        }
-
-                        if (!authUUID && !authUsage) {
-                            virReportError(VIR_ERR_XML_ERROR, "%s",
-                                           _("either uuid or usage should be "
-                                             "specified for a secret"));
-                            goto error;
-                        }
-
-                        if (authUUID != NULL) {
-                            def->src->auth.secretType = VIR_STORAGE_SECRET_TYPE_UUID;
-                            if (virUUIDParse(authUUID,
-                                             def->src->auth.secret.uuid) < 0) {
-                                virReportError(VIR_ERR_XML_ERROR,
-                                               _("malformed uuid %s"),
-                                               authUUID);
-                                goto error;
-                            }
-                        } else if (authUsage != NULL) {
-                            def->src->auth.secretType = VIR_STORAGE_SECRET_TYPE_USAGE;
-                            def->src->auth.secret.usage = authUsage;
-                            authUsage = NULL;
-                        }
-                    }
-                    child = child->next;
                 }
             } else if (xmlStrEqual(cur->name, BAD_CAST "iotune")) {
                 if (virXPathULongLong("string(./iotune/total_bytes_sec)",
@@ -5944,8 +5890,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
 
     def->dst = target;
     target = NULL;
-    def->src->auth.username = authUsername;
-    authUsername = NULL;
+    def->src->auth = authdef;
+    authdef = NULL;
     def->src->driverName = driverName;
     driverName = NULL;
     def->src->encryption = encryption;
@@ -5987,10 +5933,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
     VIR_FREE(removable);
     VIR_FREE(trans);
     VIR_FREE(device);
-    VIR_FREE(authUsername);
-    VIR_FREE(usageType);
-    VIR_FREE(authUUID);
-    VIR_FREE(authUsage);
+    virStorageAuthDefFree(authdef);
     VIR_FREE(driverType);
     VIR_FREE(driverName);
     VIR_FREE(cachetag);
@@ -15066,8 +15009,6 @@ virDomainDiskDefFormat(virBufferPtr buf,
     const char *sgio = virDomainDeviceSGIOTypeToString(def->sgio);
     const char *discard = virDomainDiskDiscardTypeToString(def->discard);
 
-    char uuidstr[VIR_UUID_STRING_BUFLEN];
-
     if (!type || !def->src->type) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unexpected disk type %d"), def->src->type);
@@ -15149,26 +15090,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
         virBufferAddLit(buf, "/>\n");
     }
 
-    if (def->src->auth.username) {
-        virBufferEscapeString(buf, "<auth username='%s'>\n",
-                              def->src->auth.username);
-        virBufferAdjustIndent(buf, 2);
-        if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) {
-            virBufferAddLit(buf, "<secret type='iscsi'");
-        } else if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
-            virBufferAddLit(buf, "<secret type='ceph'");
-        }
-
-        if (def->src->auth.secretType == VIR_STORAGE_SECRET_TYPE_UUID) {
-            virUUIDFormat(def->src->auth.secret.uuid, uuidstr);
-            virBufferAsprintf(buf, " uuid='%s'/>\n", uuidstr);
-        }
-        if (def->src->auth.secretType == VIR_STORAGE_SECRET_TYPE_USAGE) {
-            virBufferEscapeString(buf, " usage='%s'/>\n",
-                                  def->src->auth.secret.usage);
-        }
-        virBufferAdjustIndent(buf, -2);
-        virBufferAddLit(buf, "</auth>\n");
+    if (def->src->auth) {
+        if (virStorageAuthDefFormat(buf, def->src->auth) < 0)
+            return -1;
     }
 
     if (virDomainDiskSourceFormat(buf, def->src, def->startupPolicy,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 17dcd67..119175b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1907,7 +1907,6 @@ virStorageNetHostDefFree;
 virStorageNetHostTransportTypeFromString;
 virStorageNetHostTransportTypeToString;
 virStorageNetProtocolTypeToString;
-virStorageSourceAuthClear;
 virStorageSourceBackingStoreClear;
 virStorageSourceClear;
 virStorageSourceFree;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 63f322a..6664547 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -45,6 +45,7 @@
 #include "domain_conf.h"
 #include "snapshot_conf.h"
 #include "storage_conf.h"
+#include "secret_conf.h"
 #include "network/bridge_driver.h"
 #include "virnetdevtap.h"
 #include "base64.h"
@@ -2469,9 +2470,7 @@ static char *
 qemuGetSecretString(virConnectPtr conn,
                     const char *scheme,
                     bool encoded,
-                    int diskSecretType,
-                    char *username,
-                    unsigned char *uuid, char *usage,
+                    virStorageAuthDefPtr authdef,
                     virSecretUsageType secretUsageType)
 {
     size_t secret_size;
@@ -2480,25 +2479,26 @@ qemuGetSecretString(virConnectPtr conn,
     char uuidStr[VIR_UUID_STRING_BUFLEN];
 
     /* look up secret */
-    switch (diskSecretType) {
+    switch (authdef->secretType) {
     case VIR_STORAGE_SECRET_TYPE_UUID:
-        sec = virSecretLookupByUUID(conn, uuid);
-        virUUIDFormat(uuid, uuidStr);
+        sec = virSecretLookupByUUID(conn, authdef->secret.uuid);
+        virUUIDFormat(authdef->secret.uuid, uuidStr);
         break;
     case VIR_STORAGE_SECRET_TYPE_USAGE:
-        sec = virSecretLookupByUsage(conn, secretUsageType, usage);
+        sec = virSecretLookupByUsage(conn, secretUsageType,
+                                     authdef->secret.usage);
         break;
     }
 
     if (!sec) {
-        if (diskSecretType == VIR_STORAGE_SECRET_TYPE_UUID) {
+        if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) {
             virReportError(VIR_ERR_NO_SECRET,
                            _("%s no secret matches uuid '%s'"),
                            scheme, uuidStr);
         } else {
             virReportError(VIR_ERR_NO_SECRET,
                            _("%s no secret matches usage value '%s'"),
-                           scheme, usage);
+                           scheme, authdef->secret.usage);
         }
         goto cleanup;
     }
@@ -2506,16 +2506,16 @@ qemuGetSecretString(virConnectPtr conn,
     secret = (char *)conn->secretDriver->secretGetValue(sec, &secret_size, 0,
                                                         VIR_SECRET_GET_VALUE_INTERNAL_CALL);
     if (!secret) {
-        if (diskSecretType == VIR_STORAGE_SECRET_TYPE_UUID) {
+        if (authdef->secretType == VIR_STORAGE_SECRET_TYPE_UUID) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("could not get value of the secret for "
                              "username '%s' using uuid '%s'"),
-                           username, uuidStr);
+                           authdef->username, uuidStr);
         } else {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("could not get value of the secret for "
                              "username '%s' using usage value '%s'"),
-                           username, usage);
+                           authdef->username, authdef->secret.usage);
         }
         goto cleanup;
     }
@@ -2619,9 +2619,22 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk)
             *e = '\0';
         }
 
-        if (STRPREFIX(p, "id=") &&
-            VIR_STRDUP(disk->src->auth.username, p + strlen("id=")) < 0)
-            goto error;
+        if (STRPREFIX(p, "id=")) {
+            const char *secrettype;
+            /* formulate a disk->src->auth */
+            if (VIR_ALLOC(disk->src->auth) < 0)
+                goto error;
+
+            if (VIR_STRDUP(disk->src->auth->username, p + strlen("id=")) < 0)
+                goto error;
+            secrettype = virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_CEPH);
+            if (VIR_STRDUP(disk->src->auth->secrettype, secrettype) < 0)
+                goto error;
+
+            /* Cannot formulate a secretType (eg, usage or uuid) given
+             * what is provided.
+             */
+        }
         if (STRPREFIX(p, "mon_host=")) {
             char *h, *sep;
 
@@ -2650,6 +2663,7 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk)
 
  error:
     VIR_FREE(options);
+    virStorageAuthDefFree(disk->src->auth);
     return -1;
 }
 
@@ -2719,12 +2733,27 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri,
     }
 
     if (uri->user) {
+        const char *secrettype;
+        /* formulate a def->src->auth */
+        if (VIR_ALLOC(def->src->auth) < 0)
+            goto error;
+
         secret = strchr(uri->user, ':');
         if (secret)
             *secret = '\0';
 
-        if (VIR_STRDUP(def->src->auth.username, uri->user) < 0)
+        if (VIR_STRDUP(def->src->auth->username, uri->user) < 0)
             goto error;
+        if (STREQ(scheme, "iscsi")) {
+            secrettype =
+                virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI);
+            if (VIR_STRDUP(def->src->auth->secrettype, secrettype) < 0)
+                goto error;
+        }
+
+        /* Cannot formulate a secretType (eg, usage or uuid) given
+         * what is provided.
+         */
     }
 
     def->src->nhosts = 1;
@@ -2738,6 +2767,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri,
  error:
     virStorageNetHostDefClear(def->src->hosts);
     VIR_FREE(def->src->hosts);
+    VIR_FREE(def->src->auth);
     goto cleanup;
 }
 
@@ -3134,14 +3164,13 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
 
     if (conn) {
         if (actualType == VIR_STORAGE_TYPE_NETWORK &&
-            src->auth.username &&
+            src->auth &&
             (src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI ||
              src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD)) {
             bool encode = false;
             int secretType = VIR_SECRET_USAGE_TYPE_ISCSI;
             const char *protocol = virStorageNetProtocolTypeToString(src->protocol);
-
-            username = src->auth.username;
+            username = src->auth->username;
 
             if (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) {
                 /* qemu requires the secret to be encoded for RBD */
@@ -3152,10 +3181,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
             if (!(secret = qemuGetSecretString(conn,
                                                protocol,
                                                encode,
-                                               src->auth.secretType,
-                                               username,
-                                               src->auth.secret.uuid,
-                                               src->auth.secret.usage,
+                                               src->auth,
                                                secretType)))
                 goto cleanup;
         }
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 8a3bdef..43af60e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1214,45 +1214,49 @@ qemuTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def,
                                 virStoragePoolDefPtr pooldef)
 {
     int ret = -1;
+    virStorageAuthDefPtr authdef;
 
     /* Only necessary when authentication set */
     if (pooldef->source.authType == VIR_STORAGE_POOL_AUTH_NONE) {
         ret = 0;
         goto cleanup;
     }
+    if (VIR_ALLOC(def->src->auth) < 0)
+        goto cleanup;
+    authdef = def->src->auth;
 
     /* Copy the authentication information from the storage pool
      * into the virDomainDiskDef
      */
     if (pooldef->source.authType == VIR_STORAGE_POOL_AUTH_CHAP) {
-        if (VIR_STRDUP(def->src->auth.username,
+        if (VIR_STRDUP(authdef->username,
                        pooldef->source.auth.chap.username) < 0)
             goto cleanup;
         if (pooldef->source.auth.chap.secret.uuidUsable) {
-            def->src->auth.secretType = VIR_STORAGE_SECRET_TYPE_UUID;
-            memcpy(def->src->auth.secret.uuid,
+            authdef->secretType = VIR_STORAGE_SECRET_TYPE_UUID;
+            memcpy(authdef->secret.uuid,
                    pooldef->source.auth.chap.secret.uuid,
                    VIR_UUID_BUFLEN);
         } else {
-            if (VIR_STRDUP(def->src->auth.secret.usage,
+            if (VIR_STRDUP(authdef->secret.usage,
                            pooldef->source.auth.chap.secret.usage) < 0)
                 goto cleanup;
-            def->src->auth.secretType = VIR_STORAGE_SECRET_TYPE_USAGE;
+            authdef->secretType = VIR_STORAGE_SECRET_TYPE_USAGE;
         }
     } else if (pooldef->source.authType == VIR_STORAGE_POOL_AUTH_CEPHX) {
-        if (VIR_STRDUP(def->src->auth.username,
+        if (VIR_STRDUP(authdef->username,
                        pooldef->source.auth.cephx.username) < 0)
             goto cleanup;
         if (pooldef->source.auth.cephx.secret.uuidUsable) {
-            def->src->auth.secretType = VIR_STORAGE_SECRET_TYPE_UUID;
-            memcpy(def->src->auth.secret.uuid,
+            authdef->secretType = VIR_STORAGE_SECRET_TYPE_UUID;
+            memcpy(authdef->secret.uuid,
                    pooldef->source.auth.cephx.secret.uuid,
                    VIR_UUID_BUFLEN);
         } else {
-            if (VIR_STRDUP(def->src->auth.secret.usage,
+            if (VIR_STRDUP(authdef->secret.usage,
                            pooldef->source.auth.cephx.secret.usage) < 0)
                 goto cleanup;
-            def->src->auth.secretType = VIR_STORAGE_SECRET_TYPE_USAGE;
+            authdef->secretType = VIR_STORAGE_SECRET_TYPE_USAGE;
         }
     }
     ret = 0;
@@ -1315,7 +1319,7 @@ qemuTranslateDiskSourcePool(virConnectPtr conn,
 
     VIR_FREE(def->src->path);
     virStorageNetHostDefFree(def->src->nhosts, def->src->hosts);
-    virStorageSourceAuthClear(def->src);
+    virStorageAuthDefFree(def->src->auth);
 
     switch ((virStoragePoolType) pooldef->type) {
     case VIR_STORAGE_POOL_DIR:
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 056e59e..93de343 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1733,18 +1733,6 @@ virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def)
 }
 
 
-void
-virStorageSourceAuthClear(virStorageSourcePtr def)
-{
-    VIR_FREE(def->auth.username);
-
-    if (def->auth.secretType == VIR_STORAGE_SECRET_TYPE_USAGE)
-        VIR_FREE(def->auth.secret.usage);
-
-    def->auth.secretType = VIR_STORAGE_SECRET_TYPE_NONE;
-}
-
-
 int
 virStorageSourceGetActualType(virStorageSourcePtr def)
 {
@@ -1802,7 +1790,7 @@ virStorageSourceClear(virStorageSourcePtr def)
     VIR_FREE(def->timestamps);
 
     virStorageNetHostDefFree(def->nhosts, def->hosts);
-    virStorageSourceAuthClear(def);
+    virStorageAuthDefFree(def->auth);
 
     virStorageSourceBackingStoreClear(def);
 }
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 383ba96..833fbe1 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -239,14 +239,7 @@ struct _virStorageSource {
     size_t nhosts;
     virStorageNetHostDefPtr hosts;
     virStorageSourcePoolDefPtr srcpool;
-    struct {
-        char *username;
-        int secretType; /* virStorageSecretType */
-        union {
-            unsigned char uuid[VIR_UUID_BUFLEN];
-            char *usage;
-        } secret;
-    } auth;
+    virStorageAuthDefPtr auth;
     virStorageEncryptionPtr encryption;
 
     char *driverName;
@@ -343,7 +336,6 @@ void virStorageNetHostDefFree(size_t nhosts, virStorageNetHostDefPtr hosts);
 virStorageNetHostDefPtr virStorageNetHostDefCopy(size_t nhosts,
                                                  virStorageNetHostDefPtr hosts);
 
-void virStorageSourceAuthClear(virStorageSourcePtr def);
 void virStorageSourcePoolDefFree(virStorageSourcePoolDefPtr def);
 void virStorageSourceClear(virStorageSourcePtr def);
 int virStorageSourceGetActualType(virStorageSourcePtr def);
diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 04d5a65..6bad7d6 100644
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -27,7 +27,6 @@ static int blankProblemElements(char *data)
         virtTestClearLineRegex("<uuid>([[:alnum:]]|-)+</uuid>", data) < 0 ||
         virtTestClearLineRegex("<memory.*>[[:digit:]]+</memory>", data) < 0 ||
         virtTestClearLineRegex("<secret.*>", data) < 0 ||
-        virtTestClearLineRegex("</auth.*>", data) < 0 ||
         virtTestClearLineRegex("<currentMemory.*>[[:digit:]]+</currentMemory>",
                                data) < 0 ||
         virtTestClearLineRegex("<readonly/>", data) < 0 ||
-- 
1.9.3




More information about the libvir-list mailing list