[libvirt] [PATCH 4/9] conf: simplify internal virSecretDef handling of usage
Daniel P. Berrange
berrange at redhat.com
Thu Jan 5 13:59:22 UTC 2017
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 at redhat.com>
---
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
More information about the libvir-list
mailing list