From 6887af392cbb155ebbb7d24692a73c13ed714cc8 Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Tue, 24 Jun 2014 09:46:23 -0400 Subject: [PATCH] Utilize virDomainDiskAuth for domain disk 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 --- src/conf/domain_conf.c | 106 ++++++-------------------------------- src/libvirt_private.syms | 1 - src/qemu/qemu_command.c | 78 +++++++++++++++++++--------- src/qemu/qemu_conf.c | 26 ++++++---- src/util/virstoragefile.c | 14 +---- src/util/virstoragefile.h | 10 +--- tests/qemuargv2xmltest.c | 1 - 7 files changed, 87 insertions(+), 149 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e1ce585cc0..54925ba1a7 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); @@ -15082,8 +15025,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); @@ -15165,26 +15106,9 @@ virDomainDiskDefFormat(virBufferPtr buf, virBufferAddLit(buf, "/>\n"); } - if (def->src->auth.username) { - virBufferEscapeString(buf, "\n", - def->src->auth.username); - virBufferAdjustIndent(buf, 2); - if (def->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) { - virBufferAddLit(buf, "src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD) { - virBufferAddLit(buf, "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, "\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 a920ef0724..8c02df17af 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1917,7 +1917,6 @@ virStorageNetHostDefFree; virStorageNetHostTransportTypeFromString; virStorageNetHostTransportTypeToString; virStorageNetProtocolTypeToString; -virStorageSourceAuthClear; virStorageSourceBackingStoreClear; virStorageSourceClear; virStorageSourceFree; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6d3cc07ef4..fb64cdad3a 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; } @@ -2590,6 +2590,7 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk) { char *options = NULL; char *p, *e, *next; + virStorageAuthDefPtr authdef = NULL; p = strchr(disk->src->path, ':'); if (p) { @@ -2619,9 +2620,24 @@ 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 authdef for disk->src->auth */ + if (VIR_ALLOC(authdef) < 0) + goto error; + + if (VIR_STRDUP(authdef->username, p + strlen("id=")) < 0) + goto error; + secrettype = virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_CEPH); + if (VIR_STRDUP(authdef->secrettype, secrettype) < 0) + goto error; + disk->src->auth = authdef; + authdef = NULL; + + /* Cannot formulate a secretType (eg, usage or uuid) given + * what is provided. + */ + } if (STRPREFIX(p, "mon_host=")) { char *h, *sep; @@ -2650,6 +2666,7 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk) error: VIR_FREE(options); + virStorageAuthDefFree(authdef); return -1; } @@ -2662,6 +2679,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, char *sock = NULL; char *volimg = NULL; char *secret = NULL; + virStorageAuthDefPtr authdef = NULL; if (VIR_ALLOC(def->src->hosts) < 0) goto error; @@ -2719,12 +2737,29 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, } if (uri->user) { + const char *secrettype; + /* formulate authdef for disk->src->auth */ + if (VIR_ALLOC(authdef) < 0) + goto error; + secret = strchr(uri->user, ':'); if (secret) *secret = '\0'; - if (VIR_STRDUP(def->src->auth.username, uri->user) < 0) + if (VIR_STRDUP(authdef->username, uri->user) < 0) goto error; + if (STREQ(scheme, "iscsi")) { + secrettype = + virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); + if (VIR_STRDUP(authdef->secrettype, secrettype) < 0) + goto error; + } + def->src->auth = authdef; + authdef = NULL; + + /* Cannot formulate a secretType (eg, usage or uuid) given + * what is provided. + */ } def->src->nhosts = 1; @@ -2738,6 +2773,7 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, error: virStorageNetHostDefClear(def->src->hosts); VIR_FREE(def->src->hosts); + virStorageAuthDefFree(authdef); goto cleanup; } @@ -3130,14 +3166,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 */ @@ -3148,10 +3183,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 8a3bdefd51..43af60ee9d 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 66be441f9d..15043827b1 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1732,18 +1732,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) { @@ -1801,7 +1789,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 383ba96cb7..833fbe1c01 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 04d5a65955..6bad7d6d19 100644 --- a/tests/qemuargv2xmltest.c +++ b/tests/qemuargv2xmltest.c @@ -27,7 +27,6 @@ static int blankProblemElements(char *data) virtTestClearLineRegex("([[:alnum:]]|-)+", data) < 0 || virtTestClearLineRegex("[[:digit:]]+", data) < 0 || virtTestClearLineRegex("", data) < 0 || - virtTestClearLineRegex("", data) < 0 || virtTestClearLineRegex("[[:digit:]]+", data) < 0 || virtTestClearLineRegex("", data) < 0 ||