[libvirt] [PATCH 4/9] conf: simplify internal virSecretDef handling of usage

The public virSecret object has a single "usage_id" field but the virSecretDef object has a different 'char *' field for each usage type, but the code all assumes every usage type has a corresponding single string. Get rid of the pointless union in virSecretDef and just use "usage_id" everywhere. This doesn't impact public XML format, only the internal handling. Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- src/access/viraccessdriverpolkit.c | 8 ++--- src/conf/secret_conf.c | 74 +++++++------------------------------- src/conf/secret_conf.h | 9 +---- src/conf/virsecretobj.c | 42 +++++----------------- src/datatypes.c | 3 +- src/libvirt_private.syms | 1 - src/secret/secret_driver.c | 6 ++-- src/storage/storage_backend.c | 2 +- 8 files changed, 31 insertions(+), 114 deletions(-) diff --git a/src/access/viraccessdriverpolkit.c b/src/access/viraccessdriverpolkit.c index 0d9e0a1..246af2f 100644 --- a/src/access/viraccessdriverpolkit.c +++ b/src/access/viraccessdriverpolkit.c @@ -303,7 +303,7 @@ virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, const char *attrs[] = { "connect_driver", driverName, "secret_uuid", uuidstr, - "secret_usage_volume", secret->usage.volume, + "secret_usage_volume", secret->usage_id, NULL, }; @@ -316,7 +316,7 @@ virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, const char *attrs[] = { "connect_driver", driverName, "secret_uuid", uuidstr, - "secret_usage_ceph", secret->usage.ceph, + "secret_usage_ceph", secret->usage_id, NULL, }; @@ -329,7 +329,7 @@ virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, const char *attrs[] = { "connect_driver", driverName, "secret_uuid", uuidstr, - "secret_usage_target", secret->usage.target, + "secret_usage_target", secret->usage_id, NULL, }; @@ -342,7 +342,7 @@ virAccessDriverPolkitCheckSecret(virAccessManagerPtr manager, const char *attrs[] = { "connect_driver", driverName, "secret_uuid", uuidstr, - "secret_usage_name", secret->usage.name, + "secret_usage_name", secret->usage_id, NULL, }; diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index e662455..985bae4 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -41,31 +41,6 @@ VIR_LOG_INIT("conf.secret_conf"); VIR_ENUM_IMPL(virSecretUsage, VIR_SECRET_USAGE_TYPE_LAST, "none", "volume", "ceph", "iscsi", "tls") -const char * -virSecretUsageIDForDef(virSecretDefPtr def) -{ - switch (def->usage_type) { - case VIR_SECRET_USAGE_TYPE_NONE: - return ""; - - case VIR_SECRET_USAGE_TYPE_VOLUME: - return def->usage.volume; - - case VIR_SECRET_USAGE_TYPE_CEPH: - return def->usage.ceph; - - case VIR_SECRET_USAGE_TYPE_ISCSI: - return def->usage.target; - - case VIR_SECRET_USAGE_TYPE_TLS: - return def->usage.name; - - default: - return NULL; - } -} - - void virSecretDefFree(virSecretDefPtr def) { @@ -73,30 +48,7 @@ virSecretDefFree(virSecretDefPtr def) return; VIR_FREE(def->description); - switch (def->usage_type) { - case VIR_SECRET_USAGE_TYPE_NONE: - break; - - case VIR_SECRET_USAGE_TYPE_VOLUME: - VIR_FREE(def->usage.volume); - break; - - case VIR_SECRET_USAGE_TYPE_CEPH: - VIR_FREE(def->usage.ceph); - break; - - case VIR_SECRET_USAGE_TYPE_ISCSI: - VIR_FREE(def->usage.target); - break; - - case VIR_SECRET_USAGE_TYPE_TLS: - VIR_FREE(def->usage.name); - break; - - default: - VIR_ERROR(_("unexpected secret usage type %d"), def->usage_type); - break; - } + VIR_FREE(def->usage_id); VIR_FREE(def); } @@ -127,8 +79,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, break; case VIR_SECRET_USAGE_TYPE_VOLUME: - def->usage.volume = virXPathString("string(./usage/volume)", ctxt); - if (!def->usage.volume) { + def->usage_id = virXPathString("string(./usage/volume)", ctxt); + if (!def->usage_id) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("volume usage specified, but volume path is missing")); return -1; @@ -136,8 +88,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, break; case VIR_SECRET_USAGE_TYPE_CEPH: - def->usage.ceph = virXPathString("string(./usage/name)", ctxt); - if (!def->usage.ceph) { + def->usage_id = virXPathString("string(./usage/name)", ctxt); + if (!def->usage_id) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Ceph usage specified, but name is missing")); return -1; @@ -145,8 +97,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, break; case VIR_SECRET_USAGE_TYPE_ISCSI: - def->usage.target = virXPathString("string(./usage/target)", ctxt); - if (!def->usage.target) { + def->usage_id = virXPathString("string(./usage/target)", ctxt); + if (!def->usage_id) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("iSCSI usage specified, but target is missing")); return -1; @@ -154,8 +106,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, break; case VIR_SECRET_USAGE_TYPE_TLS: - def->usage.name = virXPathString("string(./usage/name)", ctxt); - if (!def->usage.name) { + def->usage_id = virXPathString("string(./usage/name)", ctxt); + if (!def->usage_id) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("TLS usage specified, but name is missing")); return -1; @@ -303,19 +255,19 @@ virSecretDefFormatUsage(virBufferPtr buf, break; case VIR_SECRET_USAGE_TYPE_VOLUME: - virBufferEscapeString(buf, "<volume>%s</volume>\n", def->usage.volume); + virBufferEscapeString(buf, "<volume>%s</volume>\n", def->usage_id); break; case VIR_SECRET_USAGE_TYPE_CEPH: - virBufferEscapeString(buf, "<name>%s</name>\n", def->usage.ceph); + virBufferEscapeString(buf, "<name>%s</name>\n", def->usage_id); break; case VIR_SECRET_USAGE_TYPE_ISCSI: - virBufferEscapeString(buf, "<target>%s</target>\n", def->usage.target); + virBufferEscapeString(buf, "<target>%s</target>\n", def->usage_id); break; case VIR_SECRET_USAGE_TYPE_TLS: - virBufferEscapeString(buf, "<name>%s</name>\n", def->usage.name); + virBufferEscapeString(buf, "<name>%s</name>\n", def->usage_id); break; default: diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index c34880f..e0d9465 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -36,16 +36,9 @@ struct _virSecretDef { unsigned char uuid[VIR_UUID_BUFLEN]; char *description; /* May be NULL */ int usage_type; /* virSecretUsageType */ - union { - char *volume; /* May be NULL */ - char *ceph; - char *target; - char *name; - } usage; + char *usage_id; /* May be NULL */ }; -const char *virSecretUsageIDForDef(virSecretDefPtr def); - void virSecretDefFree(virSecretDefPtr def); virSecretDefPtr virSecretDefParseString(const char *xml); virSecretDefPtr virSecretDefParseFile(const char *filename); diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 1351f18..049cab3 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -218,31 +218,9 @@ virSecretObjSearchName(const void *payload, if (secret->def->usage_type != data->usageType) goto cleanup; - switch (data->usageType) { - case VIR_SECRET_USAGE_TYPE_NONE: - /* never match this */ - break; - - case VIR_SECRET_USAGE_TYPE_VOLUME: - if (STREQ(secret->def->usage.volume, data->usageID)) - found = 1; - break; - - case VIR_SECRET_USAGE_TYPE_CEPH: - if (STREQ(secret->def->usage.ceph, data->usageID)) - found = 1; - break; - - case VIR_SECRET_USAGE_TYPE_ISCSI: - if (STREQ(secret->def->usage.target, data->usageID)) - found = 1; - break; - - case VIR_SECRET_USAGE_TYPE_TLS: - if (STREQ(secret->def->usage.name, data->usageID)) - found = 1; - break; - } + if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE && + STREQ(secret->def->usage_id, data->usageID)) + found = 1; cleanup: virObjectUnlock(secret); @@ -352,7 +330,6 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets, { virSecretObjPtr secret; virSecretObjPtr ret = NULL; - const char *newUsageID = virSecretUsageIDForDef(def); char uuidstr[VIR_UUID_STRING_BUFLEN]; char *configFile = NULL, *base64File = NULL; @@ -361,17 +338,14 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets, /* Is there a secret already matching this UUID */ if ((secret = virSecretObjListFindByUUIDLocked(secrets, def->uuid))) { - const char *oldUsageID; - virObjectLock(secret); - oldUsageID = virSecretUsageIDForDef(secret->def); - if (STRNEQ(oldUsageID, newUsageID)) { + if (STRNEQ_NULLABLE(secret->def->usage_id, def->usage_id)) { virUUIDFormat(secret->def->uuid, uuidstr); virReportError(VIR_ERR_INTERNAL_ERROR, _("a secret with UUID %s is already defined for " "use with %s"), - uuidstr, oldUsageID); + uuidstr, secret->def->usage_id); goto cleanup; } @@ -391,13 +365,13 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets, * try look for matching usage instead */ if ((secret = virSecretObjListFindByUsageLocked(secrets, def->usage_type, - newUsageID))) { + def->usage_id))) { virObjectLock(secret); virUUIDFormat(secret->def->uuid, uuidstr); virReportError(VIR_ERR_INTERNAL_ERROR, _("a secret with UUID %s already defined for " "use with %s"), - uuidstr, newUsageID); + uuidstr, def->usage_id); goto cleanup; } @@ -577,7 +551,7 @@ virSecretObjListPopulate(void *payload, if (!(secret = virGetSecret(data->conn, obj->def->uuid, obj->def->usage_type, - virSecretUsageIDForDef(obj->def)))) { + obj->def->usage_id))) { data->error = true; goto cleanup; } diff --git a/src/datatypes.c b/src/datatypes.c index c8a46b0..fe8b9c2 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -678,14 +678,13 @@ virGetSecret(virConnectPtr conn, const unsigned char *uuid, virCheckConnectGoto(conn, error); virCheckNonNullArgGoto(uuid, error); - virCheckNonNullArgGoto(usageID, error); if (!(ret = virObjectNew(virSecretClass))) return NULL; memcpy(&(ret->uuid[0]), uuid, VIR_UUID_BUFLEN); ret->usageType = usageType; - if (VIR_STRDUP(ret->usageID, usageID) < 0) + if (VIR_STRDUP(ret->usageID, usageID ? usageID : "") < 0) goto error; ret->conn = virObjectRef(conn); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bb84488..2c78ed6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -835,7 +835,6 @@ virSecretDefFormat; virSecretDefFree; virSecretDefParseFile; virSecretDefParseString; -virSecretUsageIDForDef; virSecretUsageTypeFromString; virSecretUsageTypeToString; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index b5b0bea..70e4dd7 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -170,7 +170,7 @@ secretLookupByUUID(virConnectPtr conn, ret = virGetSecret(conn, def->uuid, def->usage_type, - virSecretUsageIDForDef(def)); + def->usage_id); cleanup: virSecretObjEndAPI(&secret); @@ -201,7 +201,7 @@ secretLookupByUsage(virConnectPtr conn, ret = virGetSecret(conn, def->uuid, def->usage_type, - virSecretUsageIDForDef(def)); + def->usage_id); cleanup: virSecretObjEndAPI(&secret); @@ -259,7 +259,7 @@ secretDefineXML(virConnectPtr conn, ret = virGetSecret(conn, new_attrs->uuid, new_attrs->usage_type, - virSecretUsageIDForDef(new_attrs)); + new_attrs->usage_id); new_attrs = NULL; goto cleanup; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index f2fc038..116eb80 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -630,7 +630,7 @@ virStorageGenerateQcowEncryption(virConnectPtr conn, goto cleanup; def->usage_type = VIR_SECRET_USAGE_TYPE_VOLUME; - if (VIR_STRDUP(def->usage.volume, vol->target.path) < 0) + if (VIR_STRDUP(def->usage_id, vol->target.path) < 0) goto cleanup; xml = virSecretDefFormat(def); virSecretDefFree(def); -- 2.9.3