util: hash: Improve debugability of "Duplicate key" error message

If we get a user reporting this error message being shown it's pretty
useless in terms of actually debugging it since we don't know which hash
and which key are actually subject to the error.

This patch adds a new hash table callback which formats the
user-readable version of the hash key and reports it in the new message
which will look like:

"Duplicate hash table key 'blah'"

That way we will at least have an anchor point where to start the
search.

There are two special implementations of keys which are numeric so we
add specific printer functions for them.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index ef7ee80e6ae4d44f22a3f6dd7929c13018e884ce..607ba56efdc1a4f1dc1935425e159ed7fdfd42fb 100644 (file)
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1007,6 +1007,13 @@ virZPCIAddrKeyCopy(const void *name)
 }
 
 
+static char *
+virZPCIAddrKeyPrintHuman(const void *name)
+{
+    return g_strdup_printf("%u", *((unsigned int *)name));
+}
+
+
 static void
 virZPCIAddrKeyFree(void *name)
 {
@@ -1041,6 +1048,7 @@ virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
                                                        virZPCIAddrKeyCode,
                                                        virZPCIAddrKeyEqual,
                                                        virZPCIAddrKeyCopy,
+                                                       virZPCIAddrKeyPrintHuman,
                                                        virZPCIAddrKeyFree)))
             goto error;
 
@@ -1048,6 +1056,7 @@ virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
                                                        virZPCIAddrKeyCode,
                                                        virZPCIAddrKeyEqual,
                                                        virZPCIAddrKeyCopy,
+                                                       virZPCIAddrKeyPrintHuman,
                                                        virZPCIAddrKeyFree)))
             goto error;
     }

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 87ed9f4565012617d7f6be5eb1cb3176e901375b..dff2f6fd3a352307eb97c0a6c7eaea477c0e7c79 100644 (file)
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2502,6 +2502,13 @@ virCgroupPidCopy(const void *name)
 }
 
 
+static char *
+virCgroupPidPrintHuman(const void *name)
+{
+    return g_strdup_printf("%ld", (const long)name);
+}
+
+
 int
 virCgroupKillRecursiveInternal(virCgroupPtr group,
                                int signum,
@@ -2587,6 +2594,7 @@ virCgroupKillRecursive(virCgroupPtr group, int signum)
                                              virCgroupPidCode,
                                              virCgroupPidEqual,
                                              virCgroupPidCopy,
+                                             virCgroupPidPrintHuman,
                                              NULL);
 
     VIR_DEBUG("group=%p path=%s signum=%d", group, group->path, signum);

diff --git a/src/util/virhash.c b/src/util/virhash.c
index 313ca57a544b8a0a47e62f252ab4f4dd398a4f16..edf11e8b7a82e520c47ce39b079e77599bf108a1 100644 (file)
--- a/src/util/virhash.c
+++ b/src/util/virhash.c
@@ -59,6 +59,7 @@ struct _virHashTable {
     virHashKeyCode keyCode;
     virHashKeyEqual keyEqual;
     virHashKeyCopy keyCopy;
+    virHashKeyPrintHuman keyPrint;
     virHashKeyFree keyFree;
 };
 
@@ -98,6 +99,14 @@ static void *virHashStrCopy(const void *name)
     return ret;
 }
 
+
+static char *
+virHashStrPrintHuman(const void *name)
+{
+    return g_strdup(name);
+}
+
+
 static void virHashStrFree(void *name)
 {
     VIR_FREE(name);
@@ -136,6 +145,7 @@ virHashTablePtr virHashCreateFull(ssize_t size,
                                   virHashKeyCode keyCode,
                                   virHashKeyEqual keyEqual,
                                   virHashKeyCopy keyCopy,
+                                  virHashKeyPrintHuman keyPrint,
                                   virHashKeyFree keyFree)
 {
     virHashTablePtr table = NULL;
@@ -153,6 +163,7 @@ virHashTablePtr virHashCreateFull(ssize_t size,
     table->keyCode = keyCode;
     table->keyEqual = keyEqual;
     table->keyCopy = keyCopy;
+    table->keyPrint = keyPrint;
     table->keyFree = keyFree;
 
     if (VIR_ALLOC_N(table->table, size) < 0) {
@@ -180,6 +191,7 @@ virHashNew(virHashDataFree dataFree)
                              virHashStrCode,
                              virHashStrEqual,
                              virHashStrCopy,
+                             virHashStrPrintHuman,
                              virHashStrFree);
 }
 
@@ -200,6 +212,7 @@ virHashTablePtr virHashCreate(ssize_t size, virHashDataFree dataFree)
                              virHashStrCode,
                              virHashStrEqual,
                              virHashStrCopy,
+                             virHashStrPrintHuman,
                              virHashStrFree);
 }
 
@@ -353,8 +366,13 @@ virHashAddOrUpdateEntry(virHashTablePtr table, const void *name,
                 entry->payload = userdata;
                 return 0;
             } else {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("Duplicate key"));
+                g_autofree char *keystr = NULL;
+
+                if (table->keyPrint)
+                    keystr = table->keyPrint(name);
+
+                virReportError(VIR_ERR_INTERNAL_ERROR,
+                               _("Duplicate hash table key '%s'"), NULLSTR(keystr));
                 return -1;
             }
         }

diff --git a/src/util/virhash.h b/src/util/virhash.h
index 6318c0b3cd4a43abbecc1f7d99ffeb096e36dd9a..08f99d8a3d063d90e63d33f1c5508ead27e51ed9 100644 (file)
--- a/src/util/virhash.h
+++ b/src/util/virhash.h
@@ -86,6 +86,18 @@ typedef bool (*virHashKeyEqual)(const void *namea, const void *nameb);
  * Returns a newly allocated copy of @name
  */
 typedef void *(*virHashKeyCopy)(const void *name);
+/**
+ * virHashKeyPrintHuman:
+ * @name: the hash key
+ *
+ * Get a human readable version of the key for error messages. Caller
+ * will free the returned string.
+ *
+ * Returns a string representation of the key for use in error messages. Caller
+ * promises to always free the returned string.
+ */
+typedef char *(*virHashKeyPrintHuman) (const void *name);
+
 /**
  * virHashKeyFree:
  * @name: the hash key
@@ -108,6 +120,7 @@ virHashTablePtr virHashCreateFull(ssize_t size,
                                   virHashKeyCode keyCode,
                                   virHashKeyEqual keyEqual,
                                   virHashKeyCopy keyCopy,
+                                  virHashKeyPrintHuman keyPrint,
                                   virHashKeyFree keyFree);
 void virHashFree(virHashTablePtr table);
 ssize_t virHashSize(const virHashTable *table);