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@redhat.com>

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,
                 };
 

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) {
+    VIR_FREE(def->usage_id);
-    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);
 }
 
@@ -127,8 +79,8 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt,
         break;
 
     case VIR_SECRET_USAGE_TYPE_VOLUME:
-        def->usage.volume = virXPathString("string(./usage/volume)", ctxt);
+        def->usage_id = virXPathString("string(./usage/volume)", ctxt);
-        if (!def->usage.volume) {
+        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);
+        def->usage_id = virXPathString("string(./usage/name)", ctxt);
-        if (!def->usage.ceph) {
+        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);
+        def->usage_id = virXPathString("string(./usage/target)", ctxt);
-        if (!def->usage.target) {
+        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);
+        def->usage_id = virXPathString("string(./usage/name)", ctxt);
-        if (!def->usage.name) {
+        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:

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 *usage_id; /* May be NULL */
-        char *volume;               /* May be NULL */
-        char *ceph;
-        char *target;
-        char *name;
-    } usage;
 };
 
-const char *virSecretUsageIDForDef(virSecretDefPtr def);
-
 void virSecretDefFree(virSecretDefPtr def);
 virSecretDefPtr virSecretDefParseString(const char *xml);
 virSecretDefPtr virSecretDefParseFile(const char *filename);

src/conf/virsecretobj.c

@@ -218,31 +218,9 @@ virSecretObjSearchName(const void *payload,
     if (secret->def->usage_type != data->usageType)
         goto cleanup;
 
-    switch (data->usageType) {
+    if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE &&
-    case VIR_SECRET_USAGE_TYPE_NONE:
+        STREQ(secret->def->usage_id, data->usageID))
-    /* never match this */
+        found = 1;
-        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;
-    }
 
  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_NULLABLE(secret->def->usage_id, def->usage_id)) {
-        if (STRNEQ(oldUsageID, newUsageID)) {
             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;
     }

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);

src/libvirt_private.syms

@@ -838,7 +838,6 @@ virSecretDefFormat;
 virSecretDefFree;
 virSecretDefParseFile;
 virSecretDefParseString;
-virSecretUsageIDForDef;
 virSecretUsageTypeFromString;
 virSecretUsageTypeToString;
 

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;
 

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);