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@xxxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list